[RISCV] Select (and (sra x, c2), c1) as (srli (srai x, c2-c3), c3). #101868

topperc · 2024-08-04T07:36:25Z

If c1 is a mask with c3 leading zeros and c3 is larger than c2.

Fixes regression reported in #101751.

If c1 is a mask with c3 leading zeros and c3 is larger than c2. Fixes regression reported in llvm#101751.

llvmbot · 2024-08-04T07:36:55Z

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

Author: Craig Topper (topperc)

Changes

If c1 is a mask with c3 leading zeros and c3 is larger than c2.

Fixes regression reported in #101751.

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

3 Files Affected:

(modified) llvm/lib/Target/RISCV/RISCVISelDAGToDAG.cpp (+25)
(modified) llvm/test/CodeGen/RISCV/lack-of-signed-truncation-check.ll (+10-12)
(modified) llvm/test/CodeGen/RISCV/signed-truncation-check.ll (+10-12)

diff --git a/llvm/lib/Target/RISCV/RISCVISelDAGToDAG.cpp b/llvm/lib/Target/RISCV/RISCVISelDAGToDAG.cpp
index 31f48a6ac24d7..f1f345d65854e 100644
--- a/llvm/lib/Target/RISCV/RISCVISelDAGToDAG.cpp
+++ b/llvm/lib/Target/RISCV/RISCVISelDAGToDAG.cpp
@@ -1449,6 +1449,31 @@ void RISCVDAGToDAGISel::Select(SDNode *Node) {
       }
     }
 
+    // Turn (and (srl x, c2), c1) -> (srli (srai x, c2-c3)) if c1 is a mask with
+    // c3 leading zeros and c2 is larger than c3.
+    if (N0.getOpcode() == ISD::SRA && isa<ConstantSDNode>(N0.getOperand(1)) &&
+        N0.hasOneUse()) {
+      unsigned C2 = N0.getConstantOperandVal(1);
+      unsigned XLen = Subtarget->getXLen();
+      assert((C2 > 0 && C2 < XLen) && "Unexpected shift amount!");
+
+      SDValue X = N0.getOperand(0);
+
+      if (isMask_64(C1)) {
+        unsigned Leading = XLen - llvm::bit_width(C1);
+        if (C2 > Leading) {
+          SDNode *SRAI = CurDAG->getMachineNode(
+              RISCV::SRAI, DL, VT, X,
+              CurDAG->getTargetConstant(C2 - Leading, DL, VT));
+          SDNode *SRLI = CurDAG->getMachineNode(
+              RISCV::SRLI, DL, VT, SDValue(SRAI, 0),
+              CurDAG->getTargetConstant(Leading, DL, VT));
+          ReplaceNode(Node, SRLI);
+          return;
+        }
+      }
+    }
+
     // If C1 masks off the upper bits only (but can't be formed as an
     // ANDI), use an unsigned bitfield extract (e.g., th.extu), if
     // available.
diff --git a/llvm/test/CodeGen/RISCV/lack-of-signed-truncation-check.ll b/llvm/test/CodeGen/RISCV/lack-of-signed-truncation-check.ll
index 6e3a50542939f..c7ba0e501fa44 100644
--- a/llvm/test/CodeGen/RISCV/lack-of-signed-truncation-check.ll
+++ b/llvm/test/CodeGen/RISCV/lack-of-signed-truncation-check.ll
@@ -24,25 +24,23 @@
 define i1 @shifts_necmp_i16_i8(i16 %x) nounwind {
 ; RV32I-LABEL: shifts_necmp_i16_i8:
 ; RV32I:       # %bb.0:
-; RV32I-NEXT:    lui a1, 16
-; RV32I-NEXT:    addi a1, a1, -1
-; RV32I-NEXT:    and a2, a0, a1
+; RV32I-NEXT:    slli a1, a0, 16
+; RV32I-NEXT:    srli a1, a1, 16
 ; RV32I-NEXT:    slli a0, a0, 24
-; RV32I-NEXT:    srai a0, a0, 24
-; RV32I-NEXT:    and a0, a0, a1
-; RV32I-NEXT:    xor a0, a0, a2
+; RV32I-NEXT:    srai a0, a0, 8
+; RV32I-NEXT:    srli a0, a0, 16
+; RV32I-NEXT:    xor a0, a0, a1
 ; RV32I-NEXT:    snez a0, a0
 ; RV32I-NEXT:    ret
 ;
 ; RV64I-LABEL: shifts_necmp_i16_i8:
 ; RV64I:       # %bb.0:
-; RV64I-NEXT:    lui a1, 16
-; RV64I-NEXT:    addiw a1, a1, -1
-; RV64I-NEXT:    and a2, a0, a1
+; RV64I-NEXT:    slli a1, a0, 48
+; RV64I-NEXT:    srli a1, a1, 48
 ; RV64I-NEXT:    slli a0, a0, 56
-; RV64I-NEXT:    srai a0, a0, 56
-; RV64I-NEXT:    and a0, a0, a1
-; RV64I-NEXT:    xor a0, a0, a2
+; RV64I-NEXT:    srai a0, a0, 8
+; RV64I-NEXT:    srli a0, a0, 48
+; RV64I-NEXT:    xor a0, a0, a1
 ; RV64I-NEXT:    snez a0, a0
 ; RV64I-NEXT:    ret
 ;
diff --git a/llvm/test/CodeGen/RISCV/signed-truncation-check.ll b/llvm/test/CodeGen/RISCV/signed-truncation-check.ll
index de36bcdb91060..54b85fab757ca 100644
--- a/llvm/test/CodeGen/RISCV/signed-truncation-check.ll
+++ b/llvm/test/CodeGen/RISCV/signed-truncation-check.ll
@@ -24,25 +24,23 @@
 define i1 @shifts_eqcmp_i16_i8(i16 %x) nounwind {
 ; RV32I-LABEL: shifts_eqcmp_i16_i8:
 ; RV32I:       # %bb.0:
-; RV32I-NEXT:    lui a1, 16
-; RV32I-NEXT:    addi a1, a1, -1
-; RV32I-NEXT:    and a2, a0, a1
+; RV32I-NEXT:    slli a1, a0, 16
+; RV32I-NEXT:    srli a1, a1, 16
 ; RV32I-NEXT:    slli a0, a0, 24
-; RV32I-NEXT:    srai a0, a0, 24
-; RV32I-NEXT:    and a0, a0, a1
-; RV32I-NEXT:    xor a0, a0, a2
+; RV32I-NEXT:    srai a0, a0, 8
+; RV32I-NEXT:    srli a0, a0, 16
+; RV32I-NEXT:    xor a0, a0, a1
 ; RV32I-NEXT:    seqz a0, a0
 ; RV32I-NEXT:    ret
 ;
 ; RV64I-LABEL: shifts_eqcmp_i16_i8:
 ; RV64I:       # %bb.0:
-; RV64I-NEXT:    lui a1, 16
-; RV64I-NEXT:    addiw a1, a1, -1
-; RV64I-NEXT:    and a2, a0, a1
+; RV64I-NEXT:    slli a1, a0, 48
+; RV64I-NEXT:    srli a1, a1, 48
 ; RV64I-NEXT:    slli a0, a0, 56
-; RV64I-NEXT:    srai a0, a0, 56
-; RV64I-NEXT:    and a0, a0, a1
-; RV64I-NEXT:    xor a0, a0, a2
+; RV64I-NEXT:    srai a0, a0, 8
+; RV64I-NEXT:    srli a0, a0, 48
+; RV64I-NEXT:    xor a0, a0, a1
 ; RV64I-NEXT:    seqz a0, a0
 ; RV64I-NEXT:    ret
 ;

dtcxzyw · 2024-08-04T07:39:08Z

llvm/lib/Target/RISCV/RISCVISelDAGToDAG.cpp

@@ -1449,6 +1449,31 @@ void RISCVDAGToDAGISel::Select(SDNode *Node) {
      }
    }

+    // Turn (and (srl x, c2), c1) -> (srli (srai x, c2-c3)) if c1 is a mask with


Suggested change

// Turn (and (srl x, c2), c1) -> (srli (srai x, c2-c3)) if c1 is a mask with

// Turn (and (sra x, c2), c1) -> (srli (srai x, c2-c3), c3) if c1 is a mask with

PR Link: llvm/llvm-project#101868

dtcxzyw

LGTM.

Please add the following regression test (extracted from quickjs): https://godbolt.org/z/n4GsEPh4d

define i64 @func000000000000000a(i32 signext %0, i32 signext %1) nounwind {
entry:
  %3 = add i32 %0, %1
  %4 = icmp sgt i32 %3, -1
  %5 = select i1 %4, i64 0, i64 7
  ret i64 %5
}

Before:

func000000000000000a:                   # @func000000000000000a
        add     a0, a0, a1
        sraiw   a0, a0, 31
        andi    a0, a0, 7
        ret

After:

func000000000000000a:                   # @func000000000000000a
        add     a0, a0, a1
        slli    a0, a0, 32
        srai    a0, a0, 2
        srli    a0, a0, 61
        ret

I wouldn't like to block this patch since the net effect looks good. We can fix this minor regression to avoid stacking too many patches :)

dtcxzyw · 2024-08-04T22:13:53Z

llvm/lib/Target/RISCV/RISCVISelDAGToDAG.cpp

@@ -1449,6 +1449,31 @@ void RISCVDAGToDAGISel::Select(SDNode *Node) {
      }
    }

+    // Turn (and (sra x, c2), c1) -> (srli (srai x, c2-c3)) if c1 is a mask with


Suggested change

// Turn (and (sra x, c2), c1) -> (srli (srai x, c2-c3)) if c1 is a mask with

// Turn (and (sra x, c2), c1) -> (srli (srai x, c2-c3), c3) if c1 is a mask with

…l in some cases. (#102034) If x is a shl by 32 and c1 is an simm12, we would prefer to use a SRAIW+ANDI. This prevents selecting the slli to a separate slli instruction. Fixes regression from #101868

[RISCV] Select (and (srl x, c2), c1) as (srli (srai x, c2-c3)).

4907e1c

If c1 is a mask with c3 leading zeros and c3 is larger than c2. Fixes regression reported in llvm#101751.

topperc requested review from preames, dtcxzyw and wangpc-pp August 4, 2024 07:36

llvmbot added the backend:RISC-V label Aug 4, 2024

dtcxzyw reviewed Aug 4, 2024

View reviewed changes

dtcxzyw added a commit to dtcxzyw/llvm-codegen-benchmark that referenced this pull request Aug 4, 2024

pre-commit: test PR101868

e010409

PR Link: llvm/llvm-project#101868

dtcxzyw mentioned this pull request Aug 4, 2024

pre-commit: test PR101868 dtcxzyw/llvm-codegen-benchmark#96

Closed

dtcxzyw approved these changes Aug 4, 2024

View reviewed changes

fixup! Fix typo in comment

9cd3a54

dtcxzyw changed the title ~~[RISCV] Select (and (srl x, c2), c1) as (srli (srai x, c2-c3)).~~ [RISCV] Select (and (srl x, c2), c1) as (srli (srai x, c2-c3), c3). Aug 4, 2024

dtcxzyw reviewed Aug 4, 2024

View reviewed changes

dtcxzyw changed the title ~~[RISCV] Select (and (srl x, c2), c1) as (srli (srai x, c2-c3), c3).~~ [RISCV] Select (and (sra x, c2), c1) as (srli (srai x, c2-c3), c3). Aug 4, 2024

topperc added 2 commits August 4, 2024 16:25

fixup! Fix missing argument in comment

e53cf52

fixup! Add test case for regression.

779b938

topperc merged commit 5bc99fb into llvm:main Aug 5, 2024
7 checks passed

topperc deleted the pr/srai-srli branch August 5, 2024 05:35

topperc added a commit to topperc/llvm-project that referenced this pull request Aug 5, 2024

fixup! update tests after llvm#101868

f894a7f

dtcxzyw mentioned this pull request Aug 5, 2024

Update diff August 5th 2024, 6:52:49 am dtcxzyw/llvm-codegen-benchmark#97

Open

topperc mentioned this pull request Aug 5, 2024

[RISCV] Limit (and (sra x, c2), c1) -> (srli (srai x, c2-c3), c3) isel in some cases. #102034

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

[RISCV] Select (and (sra x, c2), c1) as (srli (srai x, c2-c3), c3). #101868

[RISCV] Select (and (sra x, c2), c1) as (srli (srai x, c2-c3), c3). #101868

Uh oh!

topperc commented Aug 4, 2024

Uh oh!

llvmbot commented Aug 4, 2024

Uh oh!

dtcxzyw Aug 4, 2024

Uh oh!

dtcxzyw left a comment

Uh oh!

dtcxzyw Aug 4, 2024

Uh oh!

Uh oh!

Uh oh!

	// Turn (and (srl x, c2), c1) -> (srli (srai x, c2-c3)) if c1 is a mask with
	// Turn (and (sra x, c2), c1) -> (srli (srai x, c2-c3), c3) if c1 is a mask with

[RISCV] Select (and (sra x, c2), c1) as (srli (srai x, c2-c3), c3). #101868

[RISCV] Select (and (sra x, c2), c1) as (srli (srai x, c2-c3), c3). #101868

Uh oh!

Conversation

topperc commented Aug 4, 2024

Uh oh!

llvmbot commented Aug 4, 2024

Uh oh!

dtcxzyw Aug 4, 2024

Choose a reason for hiding this comment

Uh oh!

dtcxzyw left a comment

Choose a reason for hiding this comment

Uh oh!

dtcxzyw Aug 4, 2024

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!