[TLI] Add basic support for fdim libcall #108702
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Signed-off-by: Kushal Pal <kushalpal109@gmail.com>
|
@llvm/pr-subscribers-llvm-analysis Author: None (braw-lee) Changesfirst PR to fix #108695 Full diff: https://github.com/llvm/llvm-project/pull/108702.diff 6 Files Affected:
diff --git a/llvm/include/llvm/Analysis/TargetLibraryInfo.def b/llvm/include/llvm/Analysis/TargetLibraryInfo.def
index 5914324b286c05..a18952f038cc31 100644
--- a/llvm/include/llvm/Analysis/TargetLibraryInfo.def
+++ b/llvm/include/llvm/Analysis/TargetLibraryInfo.def
@@ -2067,6 +2067,21 @@ TLI_DEFINE_ENUM_INTERNAL(remquol)
TLI_DEFINE_STRING_INTERNAL("remquol")
TLI_DEFINE_SIG_INTERNAL(LDbl, LDbl, LDbl, Ptr)
+/// double fdim(double x, double y);
+TLI_DEFINE_ENUM_INTERNAL(fdim)
+TLI_DEFINE_STRING_INTERNAL("fdim")
+TLI_DEFINE_SIG_INTERNAL(Dbl, Dbl, Dbl)
+
+/// float fdimf(float x, float y);
+TLI_DEFINE_ENUM_INTERNAL(fdimf)
+TLI_DEFINE_STRING_INTERNAL("fdimf")
+TLI_DEFINE_SIG_INTERNAL(Flt, Flt, Flt)
+
+/// long double fdiml(long double x, long double y);
+TLI_DEFINE_ENUM_INTERNAL(fdiml)
+TLI_DEFINE_STRING_INTERNAL("fdiml")
+TLI_DEFINE_SIG_INTERNAL(LDbl, LDbl, LDbl)
+
/// int remove(const char *path);
TLI_DEFINE_ENUM_INTERNAL(remove)
TLI_DEFINE_STRING_INTERNAL("remove")
diff --git a/llvm/lib/Analysis/TargetLibraryInfo.cpp b/llvm/lib/Analysis/TargetLibraryInfo.cpp
index 47413239f3c6cc..1785d77bca985c 100644
--- a/llvm/lib/Analysis/TargetLibraryInfo.cpp
+++ b/llvm/lib/Analysis/TargetLibraryInfo.cpp
@@ -306,6 +306,7 @@ static void initializeLibCalls(TargetLibraryInfoImpl &TLI, const Triple &T,
TLI.setUnavailable(LibFunc_powf);
TLI.setUnavailable(LibFunc_remainderf);
TLI.setUnavailable(LibFunc_remquof);
+ TLI.setUnavailable(LibFunc_fdimf);
TLI.setUnavailable(LibFunc_sinf);
TLI.setUnavailable(LibFunc_sinhf);
TLI.setUnavailable(LibFunc_sqrtf);
@@ -337,6 +338,7 @@ static void initializeLibCalls(TargetLibraryInfoImpl &TLI, const Triple &T,
TLI.setUnavailable(LibFunc_powl);
TLI.setUnavailable(LibFunc_remainderl);
TLI.setUnavailable(LibFunc_remquol);
+ TLI.setUnavailable(LibFunc_fdiml);
TLI.setUnavailable(LibFunc_sinl);
TLI.setUnavailable(LibFunc_sinhl);
TLI.setUnavailable(LibFunc_sqrtl);
diff --git a/llvm/lib/Transforms/Utils/BuildLibCalls.cpp b/llvm/lib/Transforms/Utils/BuildLibCalls.cpp
index b0da19813f0a4b..abe0d8c3d2e999 100644
--- a/llvm/lib/Transforms/Utils/BuildLibCalls.cpp
+++ b/llvm/lib/Transforms/Utils/BuildLibCalls.cpp
@@ -1191,6 +1191,9 @@ bool llvm::inferNonMandatoryLibFuncAttrs(Function &F,
case LibFunc_fabs:
case LibFunc_fabsf:
case LibFunc_fabsl:
+ case LibFunc_fdim:
+ case LibFunc_fdiml:
+ case LibFunc_fdimf:
case LibFunc_ffs:
case LibFunc_ffsl:
case LibFunc_ffsll:
diff --git a/llvm/test/Transforms/InferFunctionAttrs/annotate.ll b/llvm/test/Transforms/InferFunctionAttrs/annotate.ll
index bc0d7a509e1f5d..267a288345dc10 100644
--- a/llvm/test/Transforms/InferFunctionAttrs/annotate.ll
+++ b/llvm/test/Transforms/InferFunctionAttrs/annotate.ll
@@ -830,6 +830,16 @@ declare float @remquof(float, float, ptr)
; CHECK: declare x86_fp80 @remquol(x86_fp80, x86_fp80, ptr nocapture) [[NOFREE_NOUNWIND_WILLRETURN_WRITEONLY]]
declare x86_fp80 @remquol(x86_fp80, x86_fp80, ptr)
+
+; CHECK: declare double @fdim(double, double) [[NOFREE_NOUNWIND_WILLRETURN_WRITEONLY]]
+declare double @fdim(double, double)
+
+; CHECK: declare float @fdimf(float, float) [[NOFREE_NOUNWIND_WILLRETURN_WRITEONLY]]
+declare float @fdimf(float, float)
+
+; CHECK: declare x86_fp80 @fdiml(x86_fp80, x86_fp80) [[NOFREE_NOUNWIND_WILLRETURN_WRITEONLY]]
+declare x86_fp80 @fdiml(x86_fp80, x86_fp80)
+
; CHECK: declare noundef i32 @rename(ptr nocapture noundef readonly, ptr nocapture noundef readonly) [[NOFREE_NOUNWIND]]
declare i32 @rename(ptr, ptr)
diff --git a/llvm/test/tools/llvm-tli-checker/ps4-tli-check.yaml b/llvm/test/tools/llvm-tli-checker/ps4-tli-check.yaml
index 47aeb0ad8fdef9..767a962402b783 100644
--- a/llvm/test/tools/llvm-tli-checker/ps4-tli-check.yaml
+++ b/llvm/test/tools/llvm-tli-checker/ps4-tli-check.yaml
@@ -34,7 +34,7 @@
#
# CHECK: << Total TLI yes SDK no: 18
# CHECK: >> Total TLI no SDK yes: 0
-# CHECK: == Total TLI yes SDK yes: 250
+# CHECK: == Total TLI yes SDK yes: 253
#
# WRONG_DETAIL: << TLI yes SDK no : '_ZdaPv' aka operator delete[](void*)
# WRONG_DETAIL: >> TLI no SDK yes: '_ZdaPvj' aka operator delete[](void*, unsigned int)
@@ -48,14 +48,14 @@
# WRONG_DETAIL: << TLI yes SDK no : 'fminimum_numl'
# WRONG_SUMMARY: << Total TLI yes SDK no: 19{{$}}
# WRONG_SUMMARY: >> Total TLI no SDK yes: 1{{$}}
-# WRONG_SUMMARY: == Total TLI yes SDK yes: 249
+# WRONG_SUMMARY: == Total TLI yes SDK yes: 252
#
## The -COUNT suffix doesn't care if there are too many matches, so check
## the exact count first; the two directives should add up to that.
## Yes, this means additions to TLI will fail this test, but the argument
## to -COUNT can't be an expression.
-# AVAIL: TLI knows 501 symbols, 268 available
-# AVAIL-COUNT-268: {{^}} available
+# AVAIL: TLI knows 504 symbols, 271 available
+# AVAIL-COUNT-271: {{^}} available
# AVAIL-NOT: {{^}} available
# UNAVAIL-COUNT-233: not available
# UNAVAIL-NOT: not available
@@ -814,6 +814,18 @@ DynamicSymbols:
Type: STT_FUNC
Section: .text
Binding: STB_GLOBAL
+ - Name: fdim
+ Type: STT_FUNC
+ Section: .text
+ Binding: STB_GLOBAL
+ - Name: fdimf
+ Type: STT_FUNC
+ Section: .text
+ Binding: STB_GLOBAL
+ - Name: fdiml
+ Type: STT_FUNC
+ Section: .text
+ Binding: STB_GLOBAL
- Name: rewind
Type: STT_FUNC
Section: .text
diff --git a/llvm/unittests/Analysis/TargetLibraryInfoTest.cpp b/llvm/unittests/Analysis/TargetLibraryInfoTest.cpp
index c081c44ed35d00..ed6a91ce611acf 100644
--- a/llvm/unittests/Analysis/TargetLibraryInfoTest.cpp
+++ b/llvm/unittests/Analysis/TargetLibraryInfoTest.cpp
@@ -317,6 +317,9 @@ TEST_F(TargetLibraryInfoTest, ValidProto) {
"declare double @remquo(double, double, ptr)\n"
"declare float @remquof(float, float, ptr)\n"
"declare x86_fp80 @remquol(x86_fp80, x86_fp80, ptr)\n"
+ "declare double @fdim(double, double)\n"
+ "declare float @fdimf(float, float)\n"
+ "declare x86_fp80 @fdiml(x86_fp80, x86_fp80)\n"
"declare i32 @rename(i8*, i8*)\n"
"declare void @rewind(%struct*)\n"
"declare double @rint(double)\n"
|
|
@llvm/pr-subscribers-llvm-transforms Author: None (braw-lee) Changesfirst PR to fix #108695 Full diff: https://github.com/llvm/llvm-project/pull/108702.diff 6 Files Affected:
diff --git a/llvm/include/llvm/Analysis/TargetLibraryInfo.def b/llvm/include/llvm/Analysis/TargetLibraryInfo.def
index 5914324b286c05..a18952f038cc31 100644
--- a/llvm/include/llvm/Analysis/TargetLibraryInfo.def
+++ b/llvm/include/llvm/Analysis/TargetLibraryInfo.def
@@ -2067,6 +2067,21 @@ TLI_DEFINE_ENUM_INTERNAL(remquol)
TLI_DEFINE_STRING_INTERNAL("remquol")
TLI_DEFINE_SIG_INTERNAL(LDbl, LDbl, LDbl, Ptr)
+/// double fdim(double x, double y);
+TLI_DEFINE_ENUM_INTERNAL(fdim)
+TLI_DEFINE_STRING_INTERNAL("fdim")
+TLI_DEFINE_SIG_INTERNAL(Dbl, Dbl, Dbl)
+
+/// float fdimf(float x, float y);
+TLI_DEFINE_ENUM_INTERNAL(fdimf)
+TLI_DEFINE_STRING_INTERNAL("fdimf")
+TLI_DEFINE_SIG_INTERNAL(Flt, Flt, Flt)
+
+/// long double fdiml(long double x, long double y);
+TLI_DEFINE_ENUM_INTERNAL(fdiml)
+TLI_DEFINE_STRING_INTERNAL("fdiml")
+TLI_DEFINE_SIG_INTERNAL(LDbl, LDbl, LDbl)
+
/// int remove(const char *path);
TLI_DEFINE_ENUM_INTERNAL(remove)
TLI_DEFINE_STRING_INTERNAL("remove")
diff --git a/llvm/lib/Analysis/TargetLibraryInfo.cpp b/llvm/lib/Analysis/TargetLibraryInfo.cpp
index 47413239f3c6cc..1785d77bca985c 100644
--- a/llvm/lib/Analysis/TargetLibraryInfo.cpp
+++ b/llvm/lib/Analysis/TargetLibraryInfo.cpp
@@ -306,6 +306,7 @@ static void initializeLibCalls(TargetLibraryInfoImpl &TLI, const Triple &T,
TLI.setUnavailable(LibFunc_powf);
TLI.setUnavailable(LibFunc_remainderf);
TLI.setUnavailable(LibFunc_remquof);
+ TLI.setUnavailable(LibFunc_fdimf);
TLI.setUnavailable(LibFunc_sinf);
TLI.setUnavailable(LibFunc_sinhf);
TLI.setUnavailable(LibFunc_sqrtf);
@@ -337,6 +338,7 @@ static void initializeLibCalls(TargetLibraryInfoImpl &TLI, const Triple &T,
TLI.setUnavailable(LibFunc_powl);
TLI.setUnavailable(LibFunc_remainderl);
TLI.setUnavailable(LibFunc_remquol);
+ TLI.setUnavailable(LibFunc_fdiml);
TLI.setUnavailable(LibFunc_sinl);
TLI.setUnavailable(LibFunc_sinhl);
TLI.setUnavailable(LibFunc_sqrtl);
diff --git a/llvm/lib/Transforms/Utils/BuildLibCalls.cpp b/llvm/lib/Transforms/Utils/BuildLibCalls.cpp
index b0da19813f0a4b..abe0d8c3d2e999 100644
--- a/llvm/lib/Transforms/Utils/BuildLibCalls.cpp
+++ b/llvm/lib/Transforms/Utils/BuildLibCalls.cpp
@@ -1191,6 +1191,9 @@ bool llvm::inferNonMandatoryLibFuncAttrs(Function &F,
case LibFunc_fabs:
case LibFunc_fabsf:
case LibFunc_fabsl:
+ case LibFunc_fdim:
+ case LibFunc_fdiml:
+ case LibFunc_fdimf:
case LibFunc_ffs:
case LibFunc_ffsl:
case LibFunc_ffsll:
diff --git a/llvm/test/Transforms/InferFunctionAttrs/annotate.ll b/llvm/test/Transforms/InferFunctionAttrs/annotate.ll
index bc0d7a509e1f5d..267a288345dc10 100644
--- a/llvm/test/Transforms/InferFunctionAttrs/annotate.ll
+++ b/llvm/test/Transforms/InferFunctionAttrs/annotate.ll
@@ -830,6 +830,16 @@ declare float @remquof(float, float, ptr)
; CHECK: declare x86_fp80 @remquol(x86_fp80, x86_fp80, ptr nocapture) [[NOFREE_NOUNWIND_WILLRETURN_WRITEONLY]]
declare x86_fp80 @remquol(x86_fp80, x86_fp80, ptr)
+
+; CHECK: declare double @fdim(double, double) [[NOFREE_NOUNWIND_WILLRETURN_WRITEONLY]]
+declare double @fdim(double, double)
+
+; CHECK: declare float @fdimf(float, float) [[NOFREE_NOUNWIND_WILLRETURN_WRITEONLY]]
+declare float @fdimf(float, float)
+
+; CHECK: declare x86_fp80 @fdiml(x86_fp80, x86_fp80) [[NOFREE_NOUNWIND_WILLRETURN_WRITEONLY]]
+declare x86_fp80 @fdiml(x86_fp80, x86_fp80)
+
; CHECK: declare noundef i32 @rename(ptr nocapture noundef readonly, ptr nocapture noundef readonly) [[NOFREE_NOUNWIND]]
declare i32 @rename(ptr, ptr)
diff --git a/llvm/test/tools/llvm-tli-checker/ps4-tli-check.yaml b/llvm/test/tools/llvm-tli-checker/ps4-tli-check.yaml
index 47aeb0ad8fdef9..767a962402b783 100644
--- a/llvm/test/tools/llvm-tli-checker/ps4-tli-check.yaml
+++ b/llvm/test/tools/llvm-tli-checker/ps4-tli-check.yaml
@@ -34,7 +34,7 @@
#
# CHECK: << Total TLI yes SDK no: 18
# CHECK: >> Total TLI no SDK yes: 0
-# CHECK: == Total TLI yes SDK yes: 250
+# CHECK: == Total TLI yes SDK yes: 253
#
# WRONG_DETAIL: << TLI yes SDK no : '_ZdaPv' aka operator delete[](void*)
# WRONG_DETAIL: >> TLI no SDK yes: '_ZdaPvj' aka operator delete[](void*, unsigned int)
@@ -48,14 +48,14 @@
# WRONG_DETAIL: << TLI yes SDK no : 'fminimum_numl'
# WRONG_SUMMARY: << Total TLI yes SDK no: 19{{$}}
# WRONG_SUMMARY: >> Total TLI no SDK yes: 1{{$}}
-# WRONG_SUMMARY: == Total TLI yes SDK yes: 249
+# WRONG_SUMMARY: == Total TLI yes SDK yes: 252
#
## The -COUNT suffix doesn't care if there are too many matches, so check
## the exact count first; the two directives should add up to that.
## Yes, this means additions to TLI will fail this test, but the argument
## to -COUNT can't be an expression.
-# AVAIL: TLI knows 501 symbols, 268 available
-# AVAIL-COUNT-268: {{^}} available
+# AVAIL: TLI knows 504 symbols, 271 available
+# AVAIL-COUNT-271: {{^}} available
# AVAIL-NOT: {{^}} available
# UNAVAIL-COUNT-233: not available
# UNAVAIL-NOT: not available
@@ -814,6 +814,18 @@ DynamicSymbols:
Type: STT_FUNC
Section: .text
Binding: STB_GLOBAL
+ - Name: fdim
+ Type: STT_FUNC
+ Section: .text
+ Binding: STB_GLOBAL
+ - Name: fdimf
+ Type: STT_FUNC
+ Section: .text
+ Binding: STB_GLOBAL
+ - Name: fdiml
+ Type: STT_FUNC
+ Section: .text
+ Binding: STB_GLOBAL
- Name: rewind
Type: STT_FUNC
Section: .text
diff --git a/llvm/unittests/Analysis/TargetLibraryInfoTest.cpp b/llvm/unittests/Analysis/TargetLibraryInfoTest.cpp
index c081c44ed35d00..ed6a91ce611acf 100644
--- a/llvm/unittests/Analysis/TargetLibraryInfoTest.cpp
+++ b/llvm/unittests/Analysis/TargetLibraryInfoTest.cpp
@@ -317,6 +317,9 @@ TEST_F(TargetLibraryInfoTest, ValidProto) {
"declare double @remquo(double, double, ptr)\n"
"declare float @remquof(float, float, ptr)\n"
"declare x86_fp80 @remquol(x86_fp80, x86_fp80, ptr)\n"
+ "declare double @fdim(double, double)\n"
+ "declare float @fdimf(float, float)\n"
+ "declare x86_fp80 @fdiml(x86_fp80, x86_fp80)\n"
"declare i32 @rename(i8*, i8*)\n"
"declare void @rewind(%struct*)\n"
"declare double @rint(double)\n"
|
arsenm
reviewed
Sep 15, 2024
| @@ -337,6 +338,7 @@ static void initializeLibCalls(TargetLibraryInfoImpl &TLI, const Triple &T, | |||
| TLI.setUnavailable(LibFunc_powl); | |||
| TLI.setUnavailable(LibFunc_remainderl); | |||
| TLI.setUnavailable(LibFunc_remquol); | |||
| TLI.setUnavailable(LibFunc_fdiml); | |||
There was a problem hiding this comment.
Don't believe this is tested
There was a problem hiding this comment.
this line marks the long double library function unavailable for Win32
sry but i am not sure what you want to me to do here
There was a problem hiding this comment.
Add a test that shows fdiml is not recognized as a libcall on windows
There was a problem hiding this comment.
i added it because there's a comment above stating long double functions are not supported by Win32
are there any previous such tests that i can refer to?
There was a problem hiding this comment.
There seem to be 3 ways it's done. The most direct way has an example in TargetLibraryInfoTest.cpp in TLITestAarch64.
There's also an underutilized looking direct test in llvm/test/tools/llvm-tli-checker, which currently only covers ps4.
The indirect way most TLI tests do is to add a negative instcombine test for some transform, e.g. test/Transforms/InstCombine/memcmp-1.ll
There was a problem hiding this comment.
tried to add a test but its failing
mingw32 is generating fdimf
i feel like this was not the correct way to test
arsenm
reviewed
Sep 15, 2024
| ; CHECK-LABEL: define fp128 @fdim_fp128( | ||
| ; MINGW32-NOT: fp128 @fdiml | ||
| ; | ||
| %1 = call fp128 @fdiml(fp128 %x, fp128 %y) |
There was a problem hiding this comment.
I don't understand this test; what positive transform would happen? This is an ordinary, unoptimizable call?
There was a problem hiding this comment.
yh, unoptimizable call
tbh i thought windows will not generate fdiml and that would be enough
i will try the other 2 ways, but i think i will need some help, sry, i am new to the codebase
There was a problem hiding this comment.
In your constant folding patch, this wouldn't constant fold on windows. so you could test it there I suppose
There was a problem hiding this comment.
alright, I will just remove the 2nd commit from this PR
braw-lee
force-pushed
the
fdim_support
branch
from
2ec09a7 to
46f031f
Compare
arsenm
approved these changes
Sep 20, 2024
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/160/builds/5432 Here is the relevant piece of the build log for the reference |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/144/builds/7501 Here is the relevant piece of the build log for the reference |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/140/builds/7018 Here is the relevant piece of the build log for the reference |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/12/builds/6211 Here is the relevant piece of the build log for the reference |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/180/builds/5430 Here is the relevant piece of the build log for the reference |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/3/builds/4914 Here is the relevant piece of the build log for the reference |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/174/builds/5458 Here is the relevant piece of the build log for the reference |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/46/builds/5229 Here is the relevant piece of the build log for the reference |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/175/builds/5548 Here is the relevant piece of the build log for the reference |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/185/builds/5530 Here is the relevant piece of the build log for the reference |
|
Fixed by 2c6e5ef |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/73/builds/5835 Here is the relevant piece of the build log for the reference |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/11/builds/5321 Here is the relevant piece of the build log for the reference |
thank you! |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/137/builds/5592 Here is the relevant piece of the build log for the reference |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/95/builds/4056 Here is the relevant piece of the build log for the reference |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/33/builds/3366 Here is the relevant piece of the build log for the reference |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/108/builds/3946 Here is the relevant piece of the build log for the reference |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/168/builds/3544 Here is the relevant piece of the build log for the reference |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/153/builds/9506 Here is the relevant piece of the build log for the reference |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/56/builds/7896 Here is the relevant piece of the build log for the reference |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/60/builds/8130 Here is the relevant piece of the build log for the reference |
arsenm
pushed a commit
that referenced
this pull request
Oct 10, 2024
DanielCChen
pushed a commit
to DanielCChen/llvm-project
that referenced
this pull request
Oct 16, 2024
2nd PR to fix llvm#108695 based on llvm#108702 --------- Signed-off-by: Kushal Pal <kushalpal109@gmail.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
first PR to fix #108695