Skip to content

[PartiallyInlineLibCalls] Emit missed- and passed-optimization remarks when partially inlining sqrt #123966

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open

[PartiallyInlineLibCalls] Emit missed- and passed-optimization remarks when partially inlining sqrt #123966

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
12e8f4a
Add passed and missed opt-remarks to PartiallyInlineLibCalls
Jan 22, 2025
fb45df1
Messages now start with lowercase
TiborGY May 17, 2025
ce81d5c
Move analysis remark about fast sqrt isn next to the condition that c…
TiborGY May 18, 2025
1285745
Use FileCheck's --input-file option instead of cat|
TiborGY May 18, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
42 changes: 38 additions & 4 deletions llvm/lib/Transforms/Scalar/PartiallyInlineLibCalls.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,14 @@ static bool optimizeSQRT(CallInst *Call, Function *CalledFunc,
// dst = phi(v0, v1)
//

ORE->emit([&]() {
return OptimizationRemarkMissed(DEBUG_TYPE, "BranchInserted",
Call->getDebugLoc(), &CurrBB)
<< "branch to library sqrt fn had to be inserted to satisfy the "
"current target's requirement for math functions to set errno on "
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This isn't a property of the target. I also don't see why these two remarks are emitted under the same exact conditions

Copy link
Contributor Author

@TiborGY TiborGY May 18, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, target may not be the best word for it indeed, but the docs are using a similar language:

On some targets, math library functions never set errno, and so -fno-math-errno is the default. This includes most BSD-derived systems, including Darwin.

In guess I could say target triplet instead of target?

PS. I have created two remarks so I can split them into an optimization passed/missed pair. In my mental model passed means "LLVM did some profitable transformation" and missed means "something is inhibiting an optimization that could have been profitable" or "LLVM thinks something is not profitable". In this case, applying the partial inlining with the branch is simultaneously both: profitable but could have been better if setting errno wasn't a requirement.
Separating the two aspects enables the user to filter the remark output and e.g. look at only the missed optimization remarks. I often use Compiler Explorer to look at what LLVM says about a piece of code, and I find the ability to filter by remark type useful.

"invalid inputs";
});

Type *Ty = Call->getType();
IRBuilder<> Builder(Call->getNextNode());

Expand Down Expand Up @@ -125,11 +133,29 @@ static bool runPartiallyInlineLibCalls(Function &F, TargetLibraryInfo *TLI,
if (!Call || !(CalledFunc = Call->getCalledFunction()))
continue;

if (Call->isNoBuiltin() || Call->isStrictFP())
if (Call->isNoBuiltin())
continue;

if (Call->isMustTailCall())
if (Call->isStrictFP()) {
ORE->emit([&]() {
return OptimizationRemarkMissed(DEBUG_TYPE, "StrictFloat",
Call->getDebugLoc(), &*CurrBB)
<< "could not consider library function for partial inlining:"
" strict FP exception behavior is active";
});
continue;
}
// Partially inlining a libcall that has the musttail attribute leads to
// broken LLVM IR, triggering an assertion in the IR verifier.
// Work around that by forgoing this optimization for musttail calls.
if (Call->isMustTailCall()) {
ORE->emit([&]() {
return OptimizationRemarkMissed(DEBUG_TYPE, "MustTailCall",
Call->getDebugLoc(), &*CurrBB)
<< "could not consider library function for partial inlining:"
" must tail call";
});
continue;
}

// Skip if function either has local linkage or is not a known library
// function.
Expand All @@ -143,8 +169,16 @@ static bool runPartiallyInlineLibCalls(Function &F, TargetLibraryInfo *TLI,
case LibFunc_sqrt:
if (TTI->haveFastSqrt(Call->getType()) &&
optimizeSQRT(Call, CalledFunc, *CurrBB, BB, TTI,
DTU ? &*DTU : nullptr, ORE))
DTU ? &*DTU : nullptr, ORE)) {
ORE->emit([&]() {
return OptimizationRemark(DEBUG_TYPE, "SqrtPartiallyInlined",
Call->getDebugLoc(), &*CurrBB)
<< "partially inlined call to sqrt function despite having "
"to use errno for error handling: target has fast sqrt "
"instruction";
});
break;
}
continue;
default:
continue;
Expand Down
6 changes: 5 additions & 1 deletion llvm/test/Transforms/PartiallyInlineLibCalls/X86/good-prototype.ll
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,8 +1,12 @@
; NOTE: Assertions have been autogenerated by utils/update_test_checks.py
; RUN: opt -S -passes=partially-inline-libcalls -mtriple=x86_64-unknown-linux-gnu < %s | FileCheck %s
; RUN: opt -S -passes=partially-inline-libcalls -mtriple=x86_64-unknown-linux-gnu -pass-remarks=partially-inline-libcalls \
; RUN: -pass-remarks-missed=partially-inline-libcalls 2>%t < %s | FileCheck %s
; RUN: FileCheck %s -check-prefix=CHECK-REMARK --input-file=%t

define float @f(float %val) {
; CHECK-LABEL: @f(
; CHECK-REMARK: partially inlined call to sqrt function despite having to use errno for error handling: target has fast sqrt instruction
; CHECK-REMARK: branch to library sqrt fn had to be inserted to satisfy the current target's requirement for math functions to set errno on invalid inputs
; CHECK-NEXT: entry:
; CHECK-NEXT: [[RES:%.*]] = tail call float @sqrtf(float [[VAL:%.*]]) #[[READNONE:.*]]
; CHECK-NEXT: [[TMP0:%.*]] = fcmp oge float [[VAL]], 0.000000e+00
Expand Down
4 changes: 3 additions & 1 deletion llvm/test/Transforms/PartiallyInlineLibCalls/X86/musttail.ll
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,8 +1,10 @@
; NOTE: Assertions have been autogenerated by utils/update_test_checks.py
; RUN: opt -S -passes=partially-inline-libcalls -mtriple=x86_64-unknown-linux-gnu < %s | FileCheck %s
; RUN: opt -S -passes=partially-inline-libcalls -mtriple=x86_64-unknown-linux-gnu -pass-remarks-missed=partially-inline-libcalls 2>%t < %s | FileCheck %s
; RUN: FileCheck %s -check-prefix=CHECK-REMARK --input-file=%t

define double @foo(double %x) {
; CHECK-LABEL: @foo(
; CHECK-REMARK: could not consider library function for partial inlining: must tail call
; CHECK-NEXT: [[R:%.*]] = musttail call double @sqrt(double [[X:%.*]])
; CHECK-NEXT: ret double [[R]]
;
Expand Down
4 changes: 3 additions & 1 deletion llvm/test/Transforms/PartiallyInlineLibCalls/strictfp.ll
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,9 @@
; RUN: opt -S -passes=partially-inline-libcalls -mtriple=x86_64-unknown-linux-gnu < %s | FileCheck %s
; RUN: opt -S -passes=partially-inline-libcalls -mtriple=x86_64-unknown-linux-gnu -pass-remarks-missed=partially-inline-libcalls 2>%t < %s | FileCheck %s
; RUN: FileCheck %s -check-prefix=CHECK-REMARK --input-file=%t

define float @f(float %val) strictfp {
; CHECK-LABEL: @f
; CHECK-REMARK: could not consider library function for partial inlining: strict FP exception behavior is active
; CHECK: call{{.*}}@sqrtf
; CHECK-NOT: call{{.*}}@sqrtf
%res = tail call float @sqrtf(float %val) strictfp
Expand Down