[llvm] [RISCV] Support constant hoisting of immediate store values (PR #96073)
Alex Bradbury via llvm-commits
llvm-commits at lists.llvm.org
Wed Jul 17 07:07:48 PDT 2024
https://github.com/asb updated https://github.com/llvm/llvm-project/pull/96073
>From f5d85f37fcbe976818fef589d890ce65a1ab4842 Mon Sep 17 00:00:00 2001
From: Alex Bradbury <asb at igalia.com>
Date: Wed, 19 Jun 2024 14:52:53 +0100
Subject: [PATCH 1/5] [RISCV] Support constant hoisting of immediate store
values
Previously getIntImmInstCost only calculated the cost of materialising
the argument of a store if it was the address. This means
ConstantHoisting's transformation wouldn't kick in for cases like
storing two values that require multiple instructions to materialise but
where one can be cheaply generated from the other (e.g. by an addition).
---
.../Target/RISCV/MCTargetDesc/RISCVMatInt.cpp | 6 +++--
.../Target/RISCV/MCTargetDesc/RISCVMatInt.h | 7 +++++-
.../Target/RISCV/RISCVTargetTransformInfo.cpp | 23 +++++++++++++++----
llvm/test/CodeGen/RISCV/pr64935.ll | 5 +---
.../ConstantHoisting/RISCV/immediates.ll | 7 +++---
5 files changed, 33 insertions(+), 15 deletions(-)
diff --git a/llvm/lib/Target/RISCV/MCTargetDesc/RISCVMatInt.cpp b/llvm/lib/Target/RISCV/MCTargetDesc/RISCVMatInt.cpp
index 0a857eb96935e..3c927b7413136 100644
--- a/llvm/lib/Target/RISCV/MCTargetDesc/RISCVMatInt.cpp
+++ b/llvm/lib/Target/RISCV/MCTargetDesc/RISCVMatInt.cpp
@@ -499,7 +499,7 @@ InstSeq generateTwoRegInstSeq(int64_t Val, const MCSubtargetInfo &STI,
}
int getIntMatCost(const APInt &Val, unsigned Size, const MCSubtargetInfo &STI,
- bool CompressionCost) {
+ bool CompressionCost, bool FreeZeroes) {
bool IsRV64 = STI.hasFeature(RISCV::Feature64Bit);
bool HasRVC = CompressionCost && (STI.hasFeature(RISCV::FeatureStdExtC) ||
STI.hasFeature(RISCV::FeatureStdExtZca));
@@ -510,10 +510,12 @@ int getIntMatCost(const APInt &Val, unsigned Size, const MCSubtargetInfo &STI,
int Cost = 0;
for (unsigned ShiftVal = 0; ShiftVal < Size; ShiftVal += PlatRegSize) {
APInt Chunk = Val.ashr(ShiftVal).sextOrTrunc(PlatRegSize);
+ if (FreeZeroes && Chunk == 0)
+ continue;
InstSeq MatSeq = generateInstSeq(Chunk.getSExtValue(), STI);
Cost += getInstSeqCost(MatSeq, HasRVC);
}
- return std::max(1, Cost);
+ return std::max(FreeZeroes ? 0 : 1, Cost);
}
OpndKind Inst::getOpndKind() const {
diff --git a/llvm/lib/Target/RISCV/MCTargetDesc/RISCVMatInt.h b/llvm/lib/Target/RISCV/MCTargetDesc/RISCVMatInt.h
index e87e0f3256470..ae94f3778b217 100644
--- a/llvm/lib/Target/RISCV/MCTargetDesc/RISCVMatInt.h
+++ b/llvm/lib/Target/RISCV/MCTargetDesc/RISCVMatInt.h
@@ -71,8 +71,13 @@ InstSeq generateTwoRegInstSeq(int64_t Val, const MCSubtargetInfo &STI,
// If CompressionCost is true it will use a different cost calculation if RVC is
// enabled. This should be used to compare two different sequences to determine
// which is more compressible.
+//
+// If FreeZeroes is true, it will be assumed free to materialize any
+// XLen-sized chunks that are 0. This is appropriate to use in instances when
+// the zero register can be used, e.g. when estimating the cost of
+// materializing a value used by a particular operation.
int getIntMatCost(const APInt &Val, unsigned Size, const MCSubtargetInfo &STI,
- bool CompressionCost = false);
+ bool CompressionCost = false, bool FreeZeroes = false);
} // namespace RISCVMatInt
} // namespace llvm
#endif
diff --git a/llvm/lib/Target/RISCV/RISCVTargetTransformInfo.cpp b/llvm/lib/Target/RISCV/RISCVTargetTransformInfo.cpp
index d603138773de4..c426acc5140fd 100644
--- a/llvm/lib/Target/RISCV/RISCVTargetTransformInfo.cpp
+++ b/llvm/lib/Target/RISCV/RISCVTargetTransformInfo.cpp
@@ -120,7 +120,9 @@ InstructionCost RISCVTTIImpl::getIntImmCost(const APInt &Imm, Type *Ty,
// Otherwise, we check how many instructions it will take to materialise.
const DataLayout &DL = getDataLayout();
- return RISCVMatInt::getIntMatCost(Imm, DL.getTypeSizeInBits(Ty), *getST());
+ return RISCVMatInt::getIntMatCost(Imm, DL.getTypeSizeInBits(Ty), *getST(),
+ /*CompressionCost=*/false,
+ /*FreeZeroes=*/true);
}
// Look for patterns of shift followed by AND that can be turned into a pair of
@@ -172,11 +174,22 @@ InstructionCost RISCVTTIImpl::getIntImmCostInst(unsigned Opcode, unsigned Idx,
// split up large offsets in GEP into better parts than ConstantHoisting
// can.
return TTI::TCC_Free;
- case Instruction::Store:
- // If the address is a constant, use the materialization cost.
- if (Idx == 1)
+ case Instruction::Store: {
+ // Use the materialization cost regardless of if it's the address or the
+ // value that is constant, except for if the store is misaligned and
+ // misaligned accesses are not legal (experience shows constant hoisting
+ // can sometimes be harmful in such cases).
+ if (Idx == 1 || !Inst)
return getIntImmCost(Imm, Ty, CostKind);
- return TTI::TCC_Free;
+
+ StoreInst *ST = cast<StoreInst>(Inst);
+ if (!getTLI()->allowsMemoryAccessForAlignment(
+ Ty->getContext(), DL, getTLI()->getValueType(DL, Ty),
+ ST->getPointerAddressSpace(), ST->getAlign()))
+ return TTI::TCC_Free;
+
+ return getIntImmCost(Imm, Ty, CostKind);
+ }
case Instruction::Load:
// If the address is a constant, use the materialization cost.
return getIntImmCost(Imm, Ty, CostKind);
diff --git a/llvm/test/CodeGen/RISCV/pr64935.ll b/llvm/test/CodeGen/RISCV/pr64935.ll
index 60be5fa6c994e..b712db0dc99d6 100644
--- a/llvm/test/CodeGen/RISCV/pr64935.ll
+++ b/llvm/test/CodeGen/RISCV/pr64935.ll
@@ -4,10 +4,7 @@
define i1 @f() {
; CHECK-LABEL: f:
; CHECK: # %bb.0:
-; CHECK-NEXT: lui a0, 524288
-; CHECK-NEXT: not a0, a0
-; CHECK-NEXT: sltiu a0, a0, 2
-; CHECK-NEXT: xori a0, a0, 1
+; CHECK-NEXT: li a0, 1
; CHECK-NEXT: ret
%B25 = shl i64 4294967296, -9223372036854775808
%B13 = sub i64 -1, -9223372036854775808
diff --git a/llvm/test/Transforms/ConstantHoisting/RISCV/immediates.ll b/llvm/test/Transforms/ConstantHoisting/RISCV/immediates.ll
index 8f57df6edb2c0..29df749e10059 100644
--- a/llvm/test/Transforms/ConstantHoisting/RISCV/immediates.ll
+++ b/llvm/test/Transforms/ConstantHoisting/RISCV/immediates.ll
@@ -211,11 +211,12 @@ exit:
; Check that we use a common base for immediates needed by a store if the
; constants require more than 1 instruction.
-; TODO: This doesn't trigger currently.
define void @test20(ptr %p1, ptr %p2) {
; CHECK-LABEL: test20
-; CHECK: store i32 15111111, ptr %p1
-; CHECK: store i32 15111112, ptr %p2
+; CHECK: %const = bitcast i32 15111111 to i32
+; CHECK: store i32 %const, ptr %p1, align 4
+; CHECK: %const_mat = add i32 %const, 1
+; CHECK: store i32 %const_mat, ptr %p2, align 4
store i32 15111111, ptr %p1, align 4
store i32 15111112, ptr %p2, align 4
ret void
>From 7197feae3c6670643729544fa01a32df34a3c263 Mon Sep 17 00:00:00 2001
From: Alex Bradbury <asb at igalia.com>
Date: Wed, 10 Jul 2024 15:00:36 +0100
Subject: [PATCH 2/5] Add additional tests
---
.../ConstantHoisting/RISCV/immediates.ll | 51 +++++++++++++++++++
1 file changed, 51 insertions(+)
diff --git a/llvm/test/Transforms/ConstantHoisting/RISCV/immediates.ll b/llvm/test/Transforms/ConstantHoisting/RISCV/immediates.ll
index 29df749e10059..5953a7800483e 100644
--- a/llvm/test/Transforms/ConstantHoisting/RISCV/immediates.ll
+++ b/llvm/test/Transforms/ConstantHoisting/RISCV/immediates.ll
@@ -221,3 +221,54 @@ define void @test20(ptr %p1, ptr %p2) {
store i32 15111112, ptr %p2, align 4
ret void
}
+
+define void @test21(ptr %p1, ptr %p2) {
+; CHECK-LABEL: define void @test21(
+; CHECK-SAME: ptr [[P1:%.*]], ptr [[P2:%.*]]) {
+; CHECK-NEXT: store i32 15111111, ptr [[P1]], align 1
+; CHECK-NEXT: store i32 15111112, ptr [[P2]], align 1
+; CHECK-NEXT: ret void
+;
+ store i32 15111111, ptr %p1, align 1
+ store i32 15111112, ptr %p2, align 1
+ ret void
+}
+
+; 0 immediates shouldn't be hoisted.
+define void @test22(ptr %p1, ptr %p2) {
+; CHECK-LABEL: define void @test22(
+; CHECK-SAME: ptr [[P1:%.*]], ptr [[P2:%.*]]) {
+; CHECK-NEXT: store i128 0, ptr [[P1]], align 8
+; CHECK-NEXT: store i128 -1, ptr [[P2]], align 8
+; CHECK-NEXT: ret void
+;
+ store i64 0, ptr %p1, align 8
+ store i64 -1, ptr %p2, align 8
+ ret void
+}
+
+; 0 immediates shouldn't be hoisted.
+define void @test23(ptr %p1, ptr %p2) {
+; CHECK-LABEL: define void @test23(
+; CHECK-SAME: ptr [[P1:%.*]], ptr [[P2:%.*]]) {
+; CHECK-NEXT: store i127 0, ptr [[P1]], align 8
+; CHECK-NEXT: store i127 -1, ptr [[P2]], align 8
+; CHECK-NEXT: ret void
+;
+ store i127 0, ptr %p1, align 8
+ store i127 -1, ptr %p2, align 8
+ ret void
+}
+
+; Hoisting doesn't happen for types that aren't legal.
+define void @test24(ptr %p1, ptr %p2) {
+; CHECK-LABEL: define void @test24(
+; CHECK-SAME: ptr [[P1:%.*]], ptr [[P2:%.*]]) {
+; CHECK-NEXT: store i128 15111111, ptr [[P1]], align 4
+; CHECK-NEXT: store i128 15111112, ptr [[P2]], align 4
+; CHECK-NEXT: ret void
+;
+ store i128 15111111, ptr %p1, align 4
+ store i128 15111112, ptr %p2, align 4
+ ret void
+}
>From 4e0d1a95523a8c1559f0f03584a6e17e671b48ca Mon Sep 17 00:00:00 2001
From: Alex Bradbury <asb at igalia.com>
Date: Wed, 10 Jul 2024 15:01:24 +0100
Subject: [PATCH 3/5] Adopt suggestion from Craig
---
llvm/lib/Target/RISCV/MCTargetDesc/RISCVMatInt.cpp | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/llvm/lib/Target/RISCV/MCTargetDesc/RISCVMatInt.cpp b/llvm/lib/Target/RISCV/MCTargetDesc/RISCVMatInt.cpp
index 3c927b7413136..26725cf7decbe 100644
--- a/llvm/lib/Target/RISCV/MCTargetDesc/RISCVMatInt.cpp
+++ b/llvm/lib/Target/RISCV/MCTargetDesc/RISCVMatInt.cpp
@@ -510,7 +510,7 @@ int getIntMatCost(const APInt &Val, unsigned Size, const MCSubtargetInfo &STI,
int Cost = 0;
for (unsigned ShiftVal = 0; ShiftVal < Size; ShiftVal += PlatRegSize) {
APInt Chunk = Val.ashr(ShiftVal).sextOrTrunc(PlatRegSize);
- if (FreeZeroes && Chunk == 0)
+ if (FreeZeroes && Chunk.getSExtValue() == 0)
continue;
InstSeq MatSeq = generateInstSeq(Chunk.getSExtValue(), STI);
Cost += getInstSeqCost(MatSeq, HasRVC);
>From a341494d2723113f2adfa06e0279ed41bd1de1c8 Mon Sep 17 00:00:00 2001
From: Alex Bradbury <asb at igalia.com>
Date: Wed, 17 Jul 2024 13:48:11 +0100
Subject: [PATCH 4/5] Fix typo in tests
---
llvm/test/Transforms/ConstantHoisting/RISCV/immediates.ll | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/llvm/test/Transforms/ConstantHoisting/RISCV/immediates.ll b/llvm/test/Transforms/ConstantHoisting/RISCV/immediates.ll
index 5953a7800483e..329281e7dc301 100644
--- a/llvm/test/Transforms/ConstantHoisting/RISCV/immediates.ll
+++ b/llvm/test/Transforms/ConstantHoisting/RISCV/immediates.ll
@@ -238,8 +238,8 @@ define void @test21(ptr %p1, ptr %p2) {
define void @test22(ptr %p1, ptr %p2) {
; CHECK-LABEL: define void @test22(
; CHECK-SAME: ptr [[P1:%.*]], ptr [[P2:%.*]]) {
-; CHECK-NEXT: store i128 0, ptr [[P1]], align 8
-; CHECK-NEXT: store i128 -1, ptr [[P2]], align 8
+; CHECK-NEXT: store i64 0, ptr [[P1]], align 8
+; CHECK-NEXT: store i64 -1, ptr [[P2]], align 8
; CHECK-NEXT: ret void
;
store i64 0, ptr %p1, align 8
>From 74e05dcf3a55032de59dce7ceb5af8dbe80462f4 Mon Sep 17 00:00:00 2001
From: Alex Bradbury <asb at igalia.com>
Date: Wed, 17 Jul 2024 15:04:56 +0100
Subject: [PATCH 5/5] Make it so FreeZeroes is only used for loada instruction
arguments, for now
---
.../Target/RISCV/RISCVTargetTransformInfo.cpp | 24 ++++++++++++-------
llvm/test/CodeGen/RISCV/pr64935.ll | 5 +++-
2 files changed, 20 insertions(+), 9 deletions(-)
diff --git a/llvm/lib/Target/RISCV/RISCVTargetTransformInfo.cpp b/llvm/lib/Target/RISCV/RISCVTargetTransformInfo.cpp
index c426acc5140fd..f9eef60f77b7a 100644
--- a/llvm/lib/Target/RISCV/RISCVTargetTransformInfo.cpp
+++ b/llvm/lib/Target/RISCV/RISCVTargetTransformInfo.cpp
@@ -109,8 +109,11 @@ RISCVTTIImpl::getRISCVInstructionCost(ArrayRef<unsigned> OpCodes, MVT VT,
return Cost;
}
-InstructionCost RISCVTTIImpl::getIntImmCost(const APInt &Imm, Type *Ty,
- TTI::TargetCostKind CostKind) {
+static InstructionCost getIntImmCostImpl(const DataLayout &DL,
+ const RISCVSubtarget *ST,
+ const APInt &Imm, Type *Ty,
+ TTI::TargetCostKind CostKind,
+ bool FreeZeroes) {
assert(Ty->isIntegerTy() &&
"getIntImmCost can only estimate cost of materialising integers");
@@ -119,10 +122,13 @@ InstructionCost RISCVTTIImpl::getIntImmCost(const APInt &Imm, Type *Ty,
return TTI::TCC_Free;
// Otherwise, we check how many instructions it will take to materialise.
- const DataLayout &DL = getDataLayout();
- return RISCVMatInt::getIntMatCost(Imm, DL.getTypeSizeInBits(Ty), *getST(),
- /*CompressionCost=*/false,
- /*FreeZeroes=*/true);
+ return RISCVMatInt::getIntMatCost(Imm, DL.getTypeSizeInBits(Ty), *ST,
+ /*CompressionCost=*/false, FreeZeroes);
+}
+
+InstructionCost RISCVTTIImpl::getIntImmCost(const APInt &Imm, Type *Ty,
+ TTI::TargetCostKind CostKind) {
+ return getIntImmCostImpl(getDataLayout(), getST(), Imm, Ty, CostKind, false);
}
// Look for patterns of shift followed by AND that can be turned into a pair of
@@ -180,7 +186,8 @@ InstructionCost RISCVTTIImpl::getIntImmCostInst(unsigned Opcode, unsigned Idx,
// misaligned accesses are not legal (experience shows constant hoisting
// can sometimes be harmful in such cases).
if (Idx == 1 || !Inst)
- return getIntImmCost(Imm, Ty, CostKind);
+ return getIntImmCostImpl(getDataLayout(), getST(), Imm, Ty, CostKind,
+ /*FreeZeroes=*/true);
StoreInst *ST = cast<StoreInst>(Inst);
if (!getTLI()->allowsMemoryAccessForAlignment(
@@ -188,7 +195,8 @@ InstructionCost RISCVTTIImpl::getIntImmCostInst(unsigned Opcode, unsigned Idx,
ST->getPointerAddressSpace(), ST->getAlign()))
return TTI::TCC_Free;
- return getIntImmCost(Imm, Ty, CostKind);
+ return getIntImmCostImpl(getDataLayout(), getST(), Imm, Ty, CostKind,
+ /*FreeZeroes=*/true);
}
case Instruction::Load:
// If the address is a constant, use the materialization cost.
diff --git a/llvm/test/CodeGen/RISCV/pr64935.ll b/llvm/test/CodeGen/RISCV/pr64935.ll
index b712db0dc99d6..60be5fa6c994e 100644
--- a/llvm/test/CodeGen/RISCV/pr64935.ll
+++ b/llvm/test/CodeGen/RISCV/pr64935.ll
@@ -4,7 +4,10 @@
define i1 @f() {
; CHECK-LABEL: f:
; CHECK: # %bb.0:
-; CHECK-NEXT: li a0, 1
+; CHECK-NEXT: lui a0, 524288
+; CHECK-NEXT: not a0, a0
+; CHECK-NEXT: sltiu a0, a0, 2
+; CHECK-NEXT: xori a0, a0, 1
; CHECK-NEXT: ret
%B25 = shl i64 4294967296, -9223372036854775808
%B13 = sub i64 -1, -9223372036854775808
More information about the llvm-commits
mailing list