-
Notifications
You must be signed in to change notification settings - Fork 15.4k
[RISCV] Correct RISCVTTIImpl::getIntImmCostInst for Zba #128174
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
Conversation
|
@llvm/pr-subscribers-llvm-transforms @llvm/pr-subscribers-backend-risc-v Author: Gergely Futo (futog) Changeszext.w is only available on RV64. Full diff: https://github.com/llvm/llvm-project/pull/128174.diff 2 Files Affected:
diff --git a/llvm/lib/Target/RISCV/RISCVTargetTransformInfo.cpp b/llvm/lib/Target/RISCV/RISCVTargetTransformInfo.cpp
index 79e3b9ee09744..bf921dff7154d 100644
--- a/llvm/lib/Target/RISCV/RISCVTargetTransformInfo.cpp
+++ b/llvm/lib/Target/RISCV/RISCVTargetTransformInfo.cpp
@@ -207,7 +207,7 @@ InstructionCost RISCVTTIImpl::getIntImmCostInst(unsigned Opcode, unsigned Idx,
if (Imm == UINT64_C(0xffff) && ST->hasStdExtZbb())
return TTI::TCC_Free;
// zext.w
- if (Imm == UINT64_C(0xffffffff) && ST->hasStdExtZba())
+ if (ST->isRV64() && Imm == UINT64_C(0xffffffff) && ST->hasStdExtZba())
return TTI::TCC_Free;
// bclri
if (ST->hasStdExtZbs() && (~Imm).isPowerOf2())
diff --git a/llvm/test/Transforms/ConstantHoisting/RISCV/immediates.ll b/llvm/test/Transforms/ConstantHoisting/RISCV/immediates.ll
index 329281e7dc301..41fa6dc15fa25 100644
--- a/llvm/test/Transforms/ConstantHoisting/RISCV/immediates.ll
+++ b/llvm/test/Transforms/ConstantHoisting/RISCV/immediates.ll
@@ -1,5 +1,5 @@
-; RUN: opt -mtriple=riscv32-unknown-elf -S -passes=consthoist < %s | FileCheck %s
-; RUN: opt -mtriple=riscv64-unknown-elf -S -passes=consthoist < %s | FileCheck %s
+; RUN: opt -mtriple=riscv32-unknown-elf -S -passes=consthoist < %s | FileCheck %s -check-prefixes=CHECK,RV32I
+; RUN: opt -mtriple=riscv64-unknown-elf -S -passes=consthoist < %s | FileCheck %s -check-prefixes=CHECK,RV64I
; Check that we don't hoist immediates with small values.
define i64 @test1(i64 %a) nounwind {
@@ -64,10 +64,13 @@ define i64 @test7(i64 %a) nounwind {
ret i64 %2
}
-; Check that we don't hoist zext.w with Zba.
+; Check that we don't hoist zext.w with Zba on riscv64-unknown-elf.
define i64 @test8(i64 %a) nounwind "target-features"="+zba" {
-; CHECK-LABEL: test8
-; CHECK: and i64 %a, 4294967295
+; RV32I-LABEL: test8
+; RVI32: %const = bitcast i64 4294967295 to i64
+
+; RV64I-LABEL: test8
+; RV64I: and i64 %a, 4294967295
%1 = and i64 %a, 4294967295
%2 = and i64 %1, 4294967295
ret i64 %2
|
|
The description is true but won't the AND get deleted by SelectionDAG after type legalization splits it? If the constant gets hoisted that can't happen. |
Yes it should be deleted. However, this has nothing to do with the |
yes we should return TCC_Free for RV32. |
zext.w is only available on RV64. We also never hoist UINT64_C(0xffffffff) on RV32, since the AND is deleted by SelectionDAG after type legalization splits it.
topperc
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
zext.w is only available on RV64. We also never hoist UINT64_C(0xffffffff) on RV32, since the AND is deleted by SelectionDAG after type legalization splits it.
zext.w is only available on RV64.