[SystemZ] Remove incorrect areInlineCompatible hook #152494
Conversation
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.
|
@llvm/pr-subscribers-backend-systemz Author: Nikita Popov (nikic) ChangesThis reverts #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. Full diff: https://github.com/llvm/llvm-project/pull/152494.diff 3 Files Affected:
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()
}
|
|
@llvm/pr-subscribers-llvm-transforms Author: Nikita Popov (nikic) ChangesThis reverts #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. Full diff: https://github.com/llvm/llvm-project/pull/152494.diff 3 Files Affected:
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()
}
|
|
Probably worth noting that this is not entirely correct for inlining |
There was a problem hiding this comment.
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.
Ah, I see. In that case I agree that we should revert that change for now.
|
/cherry-pick 18e4f77 |
|
/pull-request #152660 |
This reverts #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.