-
Notifications
You must be signed in to change notification settings - Fork 10.8k
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.
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
[Inliner] No longer honor nobuiltin attributes #89434
base: main
Are you sure you want to change the base?
Conversation
Summary: Previously, we refused to inline functions between a callee that had the `"no-builtins"` attribute and a caller that did not. This was done to prevent a bug where libcalls (such as strlen) would be infinitely inlined into the caller and then turned back into the libcall. This solution did not cover all cases, as we can do IPO modifications on the imported libcalls without strictly inlining them that is also invalid. Additionally, this prevented *all* LTO inlining between modules compiled with `-fno-builtin` and those that did not, which heavily pessimized GPU performance when going through the LTO pipeline. The underlying problem should be solved in a related patch llvm#89431. This instead appends the `nobuiltin` attribute to every function in the module once we import a known libcall. This should prevent the behavior that this original patch was trying to prevent, so we can now permit inlining. Depends on: llvm#89431
@llvm/pr-subscribers-llvm-transforms @llvm/pr-subscribers-llvm-analysis Author: Joseph Huber (jhuber6) ChangesSummary: This solution did not cover all cases, as we can do IPO modifications on The underlying problem should be solved in a related patch Depends on: #89431 Full diff: https://github.com/llvm/llvm-project/pull/89434.diff 3 Files Affected:
diff --git a/llvm/include/llvm/Analysis/TargetLibraryInfo.h b/llvm/include/llvm/Analysis/TargetLibraryInfo.h
index 46f31f918e7b61..4b0ffbfd9684e5 100644
--- a/llvm/include/llvm/Analysis/TargetLibraryInfo.h
+++ b/llvm/include/llvm/Analysis/TargetLibraryInfo.h
@@ -324,21 +324,6 @@ class TargetLibraryInfo {
return *this;
}
- /// Determine whether a callee with the given TLI can be inlined into
- /// caller with this TLI, based on 'nobuiltin' attributes. When requested,
- /// allow inlining into a caller with a superset of the callee's nobuiltin
- /// attributes, which is conservatively correct.
- bool areInlineCompatible(const TargetLibraryInfo &CalleeTLI,
- bool AllowCallerSuperset) const {
- if (!AllowCallerSuperset)
- return OverrideAsUnavailable == CalleeTLI.OverrideAsUnavailable;
- BitVector B = OverrideAsUnavailable;
- B |= CalleeTLI.OverrideAsUnavailable;
- // We can inline if the union of the caller and callee's nobuiltin
- // attributes is no stricter than the caller's nobuiltin attributes.
- return B == OverrideAsUnavailable;
- }
-
/// Return true if the function type FTy is valid for the library function
/// F, regardless of whether the function is available.
bool isValidProtoForLibFunc(const FunctionType &FTy, LibFunc F,
diff --git a/llvm/lib/Analysis/InlineCost.cpp b/llvm/lib/Analysis/InlineCost.cpp
index c75460f44c1d9f..89eed2bcd0c54c 100644
--- a/llvm/lib/Analysis/InlineCost.cpp
+++ b/llvm/lib/Analysis/InlineCost.cpp
@@ -164,11 +164,6 @@ static cl::opt<bool> OptComputeFullInlineCost(
cl::desc("Compute the full inline cost of a call site even when the cost "
"exceeds the threshold."));
-static cl::opt<bool> InlineCallerSupersetNoBuiltin(
- "inline-caller-superset-nobuiltin", cl::Hidden, cl::init(true),
- cl::desc("Allow inlining when caller has a superset of callee's nobuiltin "
- "attributes."));
-
static cl::opt<bool> DisableGEPConstOperand(
"disable-gep-const-evaluation", cl::Hidden, cl::init(false),
cl::desc("Disables evaluation of GetElementPtr with constant operands"));
@@ -2887,8 +2882,6 @@ static bool functionsHaveCompatibleAttributes(
auto CalleeTLI = GetTLI(*Callee);
return (IgnoreTTIInlineCompatible ||
TTI.areInlineCompatible(Caller, Callee)) &&
- GetTLI(*Caller).areInlineCompatible(CalleeTLI,
- InlineCallerSupersetNoBuiltin) &&
AttributeFuncs::areInlineCompatible(*Caller, *Callee);
}
diff --git a/llvm/test/Transforms/Inline/inline-no-builtin-compatible.ll b/llvm/test/Transforms/Inline/inline-no-builtin-compatible.ll
deleted file mode 100644
index b9c0bc98c8499a..00000000000000
--- a/llvm/test/Transforms/Inline/inline-no-builtin-compatible.ll
+++ /dev/null
@@ -1,94 +0,0 @@
-; Test to ensure no inlining is allowed into a caller with fewer nobuiltin attributes.
-; RUN: opt < %s -mtriple=x86_64-unknown-linux-gnu -S -passes=inline | FileCheck %s
-; RUN: opt < %s -mtriple=x86_64-unknown-linux-gnu -S -passes='cgscc(inline)' | FileCheck %s
-
-; Make sure we don't inline callees into a caller with a superset of the
-; no builtin attributes when -inline-caller-superset-nobuiltin=false.
-; RUN: opt < %s -inline-caller-superset-nobuiltin=false -mtriple=x86_64-unknown-linux-gnu -S -passes='cgscc(inline)' | FileCheck %s --check-prefix=NOSUPERSET
-
-target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
-target triple = "x86_64-unknown-linux-gnu"
-
-define i32 @allbuiltins() {
-entry:
- %call = call i32 (...) @externalfunc()
- ret i32 %call
-; CHECK-LABEL: allbuiltins
-; CHECK: call i32 (...) @externalfunc()
-}
-declare i32 @externalfunc(...)
-
-; We can inline a function that allows all builtins into one with a single
-; nobuiltin.
-define i32 @nobuiltinmemcpy() #0 {
-entry:
- %call = call i32 @allbuiltins()
- ret i32 %call
-; CHECK-LABEL: nobuiltinmemcpy
-; CHECK-NOT: call i32 @allbuiltins()
-; NOSUPERSET-LABEL: nobuiltinmemcpy
-; NOSUPERSET: call i32 @allbuiltins()
-}
-
-; We can inline a function that allows all builtins into one with all
-; nobuiltins.
-define i32 @nobuiltins() #1 {
-entry:
- %call = call i32 @allbuiltins()
- ret i32 %call
-; CHECK-LABEL: nobuiltins
-; CHECK-NOT: call i32 @allbuiltins()
-; NOSUPERSET-LABEL: nobuiltins
-; NOSUPERSET: call i32 @allbuiltins()
-}
-
-; We can inline a function with a single nobuiltin into one with all nobuiltins.
-define i32 @nobuiltins2() #1 {
-entry:
- %call = call i32 @nobuiltinmemcpy()
- ret i32 %call
-; CHECK-LABEL: nobuiltins2
-; CHECK-NOT: call i32 @nobuiltinmemcpy()
-; NOSUPERSET-LABEL: nobuiltins2
-; NOSUPERSET: call i32 @nobuiltinmemcpy()
-}
-
-; We can't inline a function with any given nobuiltin into one that allows all
-; builtins.
-define i32 @allbuiltins2() {
-entry:
- %call = call i32 @nobuiltinmemcpy()
- ret i32 %call
-; CHECK-LABEL: allbuiltins2
-; CHECK: call i32 @nobuiltinmemcpy()
-; NOSUPERSET-LABEL: allbuiltins2
-; NOSUPERSET: call i32 @nobuiltinmemcpy()
-}
-
-; We can't inline a function with all nobuiltins into one that allows all
-; builtins.
-define i32 @allbuiltins3() {
-entry:
- %call = call i32 @nobuiltins()
- ret i32 %call
-; CHECK-LABEL: allbuiltins3
-; CHECK: call i32 @nobuiltins()
-; NOSUPERSET-LABEL: allbuiltins3
-; NOSUPERSET: call i32 @nobuiltins()
-}
-
-; We can't inline a function with a specific nobuiltin into one with a
-; different specific nobuiltin.
-define i32 @nobuiltinmemset() #2 {
-entry:
- %call = call i32 @nobuiltinmemcpy()
- ret i32 %call
-; CHECK-LABEL: nobuiltinmemset
-; CHECK: call i32 @nobuiltinmemcpy()
-; NOSUPERSET-LABEL: nobuiltinmemset
-; NOSUPERSET: call i32 @nobuiltinmemcpy()
-}
-
-attributes #0 = { "no-builtin-memcpy" }
-attributes #1 = { "no-builtins" }
-attributes #2 = { "no-builtin-memset" }
|
Summary:
Previously, we refused to inline functions between a callee that had the
"no-builtins"
attribute and a caller that did not. This was done toprevent a bug where libcalls (such as strlen) would be infinitely
inlined into the caller and then turned back into the libcall.
This solution did not cover all cases, as we can do IPO modifications on
the imported libcalls without strictly inlining them that is also
invalid. Additionally, this prevented all LTO inlining between modules
compiled with
-fno-builtin
and those that did not, which heavilypessimized GPU performance when going through the LTO pipeline.
The underlying problem should be solved in a related patch
#89431. This instead appends
the
nobuiltin
attribute to every function in the module once we importa known libcall. This should prevent the behavior that this original
patch was trying to prevent, so we can now permit inlining.
Depends on: #89431