Skip to content

release/21.x: [SystemZ] Remove incorrect areInlineCompatible hook (#152494) #152660

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

Merged
merged 1 commit into from
Aug 11, 2025

Conversation

llvmbot
Copy link
Member

@llvmbot llvmbot commented Aug 8, 2025

Backport 18e4f77

Requested by: @nikic

@llvmbot
Copy link
Member Author

llvmbot commented Aug 8, 2025

@uweigand What do you think about merging this PR to the release branch?

@llvmbot
Copy link
Member Author

llvmbot commented Aug 8, 2025

@llvm/pr-subscribers-llvm-transforms

@llvm/pr-subscribers-backend-systemz

Author: None (llvmbot)

Changes

Backport 18e4f77

Requested by: @nikic


Full diff: https://github.com/llvm/llvm-project/pull/152660.diff

3 Files Affected:

  • (modified) llvm/lib/Target/SystemZ/SystemZTargetTransformInfo.cpp (-14)
  • (modified) llvm/lib/Target/SystemZ/SystemZTargetTransformInfo.h (-3)
  • (modified) llvm/test/Transforms/Inline/SystemZ/inline-target-attr.ll (+9-9)
diff --git a/llvm/lib/Target/SystemZ/SystemZTargetTransformInfo.cpp b/llvm/lib/Target/SystemZ/SystemZTargetTransformInfo.cpp
index f32c9bd2bdea1..2611c291abaa6 100644
--- a/llvm/lib/Target/SystemZ/SystemZTargetTransformInfo.cpp
+++ b/llvm/lib/Target/SystemZ/SystemZTargetTransformInfo.cpp
@@ -436,20 +436,6 @@ bool SystemZTTIImpl::isLSRCostLess(
              C2.ScaleCost, C2.SetupCost);
 }
 
-bool SystemZTTIImpl::areInlineCompatible(const Function *Caller,
-                                         const Function *Callee) const {
-  const TargetMachine &TM = getTLI()->getTargetMachine();
-
-  const FeatureBitset &CallerBits =
-      TM.getSubtargetImpl(*Caller)->getFeatureBits();
-  const FeatureBitset &CalleeBits =
-      TM.getSubtargetImpl(*Callee)->getFeatureBits();
-
-  // Support only equal feature bitsets. Restriction should be relaxed in the
-  // future to allow inlining when callee's bits are subset of the caller's.
-  return CallerBits == CalleeBits;
-}
-
 unsigned SystemZTTIImpl::getNumberOfRegisters(unsigned ClassID) const {
   bool Vector = (ClassID == 1);
   if (!Vector)
diff --git a/llvm/lib/Target/SystemZ/SystemZTargetTransformInfo.h b/llvm/lib/Target/SystemZ/SystemZTargetTransformInfo.h
index dc5736e8af009..fc681dec1859a 100644
--- a/llvm/lib/Target/SystemZ/SystemZTargetTransformInfo.h
+++ b/llvm/lib/Target/SystemZ/SystemZTargetTransformInfo.h
@@ -65,9 +65,6 @@ class SystemZTTIImpl final : public BasicTTIImplBase<SystemZTTIImpl> {
   bool isLSRCostLess(const TargetTransformInfo::LSRCost &C1,
                      const TargetTransformInfo::LSRCost &C2) const override;
 
-  bool areInlineCompatible(const Function *Caller,
-                           const Function *Callee) const override;
-
   /// @}
 
   /// \name Vector TTI Implementations
diff --git a/llvm/test/Transforms/Inline/SystemZ/inline-target-attr.ll b/llvm/test/Transforms/Inline/SystemZ/inline-target-attr.ll
index b5c4f42655bb4..71b463b2d2b00 100644
--- a/llvm/test/Transforms/Inline/SystemZ/inline-target-attr.ll
+++ b/llvm/test/Transforms/Inline/SystemZ/inline-target-attr.ll
@@ -12,28 +12,28 @@ entry:
 
 declare i32 @baz(...) #0
 
-define i32 @bar() #1 {
+define i32 @features_subset() #1 {
 entry:
   %call = call i32 @foo()
   ret i32 %call
-; CHECK-LABEL: bar
-; CHECK: call i32 @foo()
+; CHECK-LABEL: features_subset
+; CHECK: call i32 (...) @baz()
 }
 
-define i32 @qux() #0 {
+define i32 @features_equal() #0 {
 entry:
   %call = call i32 @foo()
   ret i32 %call
-; CHECK-LABEL: qux
+; CHECK-LABEL: features_equal
 ; CHECK: call i32 (...) @baz()
 }
 
-define i32 @quux() #2 {
+define i32 @features_different() #2 {
 entry:
-  %call = call i32 @bar()
+  %call = call i32 @foo()
   ret i32 %call
-; CHECK-LABEL: quux
-; CHECK: call i32 @bar()
+; CHECK-LABEL: features_different
+; CHECK: call i32 @foo()
 }
 
 

@nikic nikic moved this from Needs Triage to Needs Review in LLVM Release Status Aug 8, 2025
Copy link
Member

@uweigand uweigand left a comment

Choose a reason for hiding this comment

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

LGTM, thanks!

@github-project-automation github-project-automation bot moved this from Needs Review to Needs Merge in LLVM Release Status Aug 8, 2025
@tru tru merged commit 7be867f into llvm:release/21.x Aug 11, 2025
@github-project-automation github-project-automation bot moved this from Needs Merge to Done in LLVM Release Status Aug 11, 2025
@tru tru deleted the issue152494 branch August 11, 2025 12:08
This reverts llvm#132976.

The PR incorrectly claimed that this makes inlining more liberal,
referencing the string comparison in TargetTransformInfoImpl.h.

However, the implementation that actually applies is the one in
BasicTTIImpl.h, which performs a feature subset comparison. As such,
this regressed inlining, most concerningly of functions without +vector
into functions with +vector.

Revert the change to restore the previous behavior.

(cherry picked from commit 18e4f77)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Development

Successfully merging this pull request may close these issues.

4 participants