Skip to content

Conversation

@topperc
Copy link
Collaborator

@topperc topperc commented Jun 5, 2025

This code incorrectly inherited a ShAmt <= 32 check from an earlier pattern.

@llvmbot
Copy link
Member

llvmbot commented Jun 5, 2025

@llvm/pr-subscribers-backend-risc-v

Author: Craig Topper (topperc)

Changes

This code incorrectly inherited a ShAmt &lt;= 32 check from an earlier pattern.


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

2 Files Affected:

  • (modified) llvm/lib/Target/RISCV/RISCVISelDAGToDAG.cpp (+2-2)
  • (modified) llvm/test/CodeGen/RISCV/and-shl.ll (+18)
diff --git a/llvm/lib/Target/RISCV/RISCVISelDAGToDAG.cpp b/llvm/lib/Target/RISCV/RISCVISelDAGToDAG.cpp
index 4dd53fdd0213d..a5cdf9f79f57a 100644
--- a/llvm/lib/Target/RISCV/RISCVISelDAGToDAG.cpp
+++ b/llvm/lib/Target/RISCV/RISCVISelDAGToDAG.cpp
@@ -1051,11 +1051,11 @@ void RISCVDAGToDAGISel::Select(SDNode *Node) {
     unsigned ShAmt = N1C->getZExtValue();
     uint64_t Mask = N0.getConstantOperandVal(1);
 
-    if (ShAmt <= 32 && isShiftedMask_64(Mask)) {
+    if (isShiftedMask_64(Mask)) {
       unsigned XLen = Subtarget->getXLen();
       unsigned LeadingZeros = XLen - llvm::bit_width(Mask);
       unsigned TrailingZeros = llvm::countr_zero(Mask);
-      if (TrailingZeros > 0 && LeadingZeros == 32) {
+      if (ShAmt <= 32 && TrailingZeros > 0 && LeadingZeros == 32) {
         // Optimize (shl (and X, C2), C) -> (slli (srliw X, C3), C3+C)
         // where C2 has 32 leading zeros and C3 trailing zeros.
         SDNode *SRLIW = CurDAG->getMachineNode(
diff --git a/llvm/test/CodeGen/RISCV/and-shl.ll b/llvm/test/CodeGen/RISCV/and-shl.ll
index c3cb5d8e2e37d..26b4a90d88fba 100644
--- a/llvm/test/CodeGen/RISCV/and-shl.ll
+++ b/llvm/test/CodeGen/RISCV/and-shl.ll
@@ -77,3 +77,21 @@ define i32 @and_0xfff_shl_2_multi_use(i32 %x) {
   %r = add i32 %a, %s
   ret i32 %r
 }
+
+define i64 @and_0xfff_shl_33(i64 %x) {
+; RV32I-LABEL: and_0xfff_shl_33:
+; RV32I:       # %bb.0:
+; RV32I-NEXT:    slli a0, a0, 20
+; RV32I-NEXT:    srli a1, a0, 19
+; RV32I-NEXT:    li a0, 0
+; RV32I-NEXT:    ret
+;
+; RV64I-LABEL: and_0xfff_shl_33:
+; RV64I:       # %bb.0:
+; RV64I-NEXT:    slli a0, a0, 52
+; RV64I-NEXT:    srli a0, a0, 19
+; RV64I-NEXT:    ret
+  %a = and i64 %x, 4095
+  %s = shl i64 %a, 33
+  ret i64 %s
+}

@pfusik
Copy link
Contributor

pfusik commented Jun 5, 2025

LGTM, modulo the commit message. I think you are relaxing the (shl (and X, C2), C) -> (srli (slli X, C4), C4-C) pattern.

@topperc
Copy link
Collaborator Author

topperc commented Jun 5, 2025

LGTM, modulo the commit message. I think you are relaxing the (shl (and X, C2), C) -> (srli (slli X, C4), C4-C) pattern.

You're right. I copied the wrong comment while I was writing the message

@topperc topperc changed the title [RISCV] Remove artificial restriction on ShAmt from (shl (and X, C2), C) -> (slli (srliw X, C3), C3+C) isel. [RISCV] Remove artificial restriction on ShAmt from (shl (and X, C2), C) -> (srli (slli X, C4), C4-C) isel. Jun 5, 2025
@topperc topperc merged commit a23bd17 into llvm:main Jun 6, 2025
13 checks passed
@topperc topperc deleted the pr/shamt branch June 6, 2025 00:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants