[PATCH] D107860: [RISCV] Add test cases showing bad materialization for stores of immediates. NFC

Craig Topper via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Tue Aug 10 12:35:47 PDT 2021


craig.topper created this revision.
craig.topper added reviewers: asb, luismarques, jrtc27, frasercrmck, evandro, HsiangKai, khchen, arcbbb.
Herald added subscribers: StephenFan, vkmr, apazos, sameer.abuasal, steven.zhang, s.egerton, Jim, benna, psnobl, jocewei, PkmX, the_o, brucehoult, MartinMosbeck, rogfer01, edward-jones, zzheng, shiva0217, kito-cheng, niosHD, sabuasal, simoncook, johnrusso, rbar.
craig.topper requested review of this revision.
Herald added a subscriber: MaskRay.
Herald added a project: LLVM.

DAGCombiner::visitStore can call GetDemandedBits which will remove
upper bits from immediates. The upper bits are important for good
materialization of negative constants on RISCV. GetDemandedBits is a
different mechanism than SimplifyDemandedBits so
TargetShrinkDemandedConstant can't block it.

As far as I know this behavior is unique to stores.

I think we can fix this in isel using a concept similar to D107658 <https://reviews.llvm.org/D107658>.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D107860

Files:
  llvm/test/CodeGen/RISCV/imm.ll


Index: llvm/test/CodeGen/RISCV/imm.ll
===================================================================
--- llvm/test/CodeGen/RISCV/imm.ll
+++ llvm/test/CodeGen/RISCV/imm.ll
@@ -480,3 +480,40 @@
 ; RV64I-NEXT:    ret
   ret i64 -1152921504301427080 ; 0xF000_0000_1234_5678
 }
+
+; FIXME: This should use a single ADDI for the immediate.
+define void @imm_store_i16_neg1(i16* %p) nounwind {
+; RV32I-LABEL: imm_store_i16_neg1:
+; RV32I:       # %bb.0:
+; RV32I-NEXT:    lui a1, 16
+; RV32I-NEXT:    addi a1, a1, -1
+; RV32I-NEXT:    sh a1, 0(a0)
+; RV32I-NEXT:    ret
+;
+; RV64I-LABEL: imm_store_i16_neg1:
+; RV64I:       # %bb.0:
+; RV64I-NEXT:    lui a1, 16
+; RV64I-NEXT:    addiw a1, a1, -1
+; RV64I-NEXT:    sh a1, 0(a0)
+; RV64I-NEXT:    ret
+  store i16 -1, i16* %p
+  ret void
+}
+
+; FIXME: This should use a single ADDI for the immediate.
+define void @imm_store_i32_neg1(i32* %p) nounwind {
+; RV32I-LABEL: imm_store_i32_neg1:
+; RV32I:       # %bb.0:
+; RV32I-NEXT:    addi a1, zero, -1
+; RV32I-NEXT:    sw a1, 0(a0)
+; RV32I-NEXT:    ret
+;
+; RV64I-LABEL: imm_store_i32_neg1:
+; RV64I:       # %bb.0:
+; RV64I-NEXT:    addi a1, zero, -1
+; RV64I-NEXT:    srli a1, a1, 32
+; RV64I-NEXT:    sw a1, 0(a0)
+; RV64I-NEXT:    ret
+  store i32 -1, i32* %p
+  ret void
+}


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D107860.365586.patch
Type: text/x-patch
Size: 1281 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20210810/62db1fe0/attachment.bin>


More information about the llvm-commits mailing list