[llvm] [Inliner] No longer honor nobuiltin attributes (PR #89434)
via llvm-commits
llvm-commits at lists.llvm.org
Fri Apr 19 11:28:14 PDT 2024
llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-llvm-transforms
@llvm/pr-subscribers-llvm-analysis
Author: Joseph Huber (jhuber6)
<details>
<summary>Changes</summary>
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
https://github.com/llvm/llvm-project/pull/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: https://github.com/llvm/llvm-project/pull/89431
---
Full diff: https://github.com/llvm/llvm-project/pull/89434.diff
3 Files Affected:
- (modified) llvm/include/llvm/Analysis/TargetLibraryInfo.h (-15)
- (modified) llvm/lib/Analysis/InlineCost.cpp (-7)
- (removed) llvm/test/Transforms/Inline/inline-no-builtin-compatible.ll (-94)
``````````diff
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" }
``````````
</details>
https://github.com/llvm/llvm-project/pull/89434
More information about the llvm-commits
mailing list