[RISCV] Combine (ADDI (ADDI X, C1), C2) -> (ADDI X, C1+C2)

[pfusik]

No description provided.

[llvmbot]

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

Author: Piotr Fusik (pfusik)

Changes

---

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

2 Files Affected:

• (modified) llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp (+1-1)
• (modified) llvm/lib/Target/RISCV/RISCVISelLowering.cpp (+13-3)

diff --git a/llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp b/llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp
index 430e47451fd49..56e273c8d6e81 100644
--- a/llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp
+++ b/llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp
@@ -4019,7 +4019,7 @@ void SelectionDAGBuilder::visitBitCast(const User &I) {
   // constant integer as an opaque constant.
   else if(ConstantInt *C = dyn_cast<ConstantInt>(I.getOperand(0)))
     setValue(&I, DAG.getConstant(C->getValue(), dl, DestVT, /*isTarget=*/false,
-                                 /*isOpaque*/true));
+                                 /*isOpaque*/false));
   else
     setValue(&I, N);            // noop cast.
 }
diff --git a/llvm/lib/Target/RISCV/RISCVISelLowering.cpp b/llvm/lib/Target/RISCV/RISCVISelLowering.cpp
index 3ab08f990c289..7d77535fbfb42 100644
--- a/llvm/lib/Target/RISCV/RISCVISelLowering.cpp
+++ b/llvm/lib/Target/RISCV/RISCVISelLowering.cpp
@@ -9421,7 +9421,17 @@ SDValue RISCVTargetLowering::lowerSELECT(SDValue Op, SelectionDAG &DAG) const {
       // Efficient only if the constant and its negation fit into `ADDI`
       // Prefer Add/Sub over Xor since can be compressed for small immediates
       if (isInt<12>(RawConstVal)) {
-        SDValue SubOp = DAG.getNode(ISD::SUB, DL, VT, RegV, ConstVal);
+        SDValue SubOp;
+        using namespace llvm::SDPatternMatch;
+        SDValue ShAmt;
+        if (sd_match(RegV, m_OneUse(m_Not(m_OneUse(m_Shl(m_AllOnes(), m_Value(ShAmt))))))) {
+          SDValue One = DAG.getConstant(1, DL, VT);
+          SDValue Shl = DAG.getNode(ISD::SHL, DL, VT, One, ShAmt);
+          SDValue SubAmt = DAG.getConstant(1 + RawConstVal, DL, VT);
+          SubOp = DAG.getNode(ISD::SUB, DL, VT, Shl, SubAmt);
+        } else {
+          SubOp = DAG.getNode(ISD::SUB, DL, VT, RegV, ConstVal);
+        }
         SDValue CMOV =
             DAG.getNode(IsCZERO_NEZ ? RISCVISD::CZERO_NEZ : RISCVISD::CZERO_EQZ,
                         DL, VT, SubOp, CondV);
@@ -16257,8 +16267,8 @@ static SDValue performXORCombine(SDNode *N, SelectionDAG &DAG,
     SDValue Op0 = DAG.getNode(ISD::ANY_EXTEND, DL, MVT::i64, N0.getOperand(0));
     SDValue Op1 = DAG.getNode(ISD::ZERO_EXTEND, DL, MVT::i64, N0.getOperand(1));
     SDValue Shl = DAG.getNode(ISD::SHL, DL, MVT::i64, Op0, Op1);
-    SDValue And = DAG.getNOT(DL, Shl, MVT::i64);
-    return DAG.getNode(ISD::TRUNCATE, DL, MVT::i32, And);
+    SDValue Not = DAG.getNOT(DL, Shl, MVT::i64);
+    return DAG.getNode(ISD::TRUNCATE, DL, MVT::i32, Not);
   }
 
   // fold (xor (sllw 1, x), -1) -> (rolw ~1, x)

[pfusik]

I found three patterns:

1. e.g. 23456b036ecbc7d0
   This can be addressed by the change in RISCVISelLowering.cpp.

2. e.g. d1e7a5db560b32a5
   Similar, but the inner ADDI is ADDIW until riscv-opt-w-instrs. How to handle?

3. e.g. 011a43fe5b60424c
   This is because of an opaque constant. What are opaque constants? I cannot find any documentation.

[github-actions]

⚠️ C/C++ code formatter, clang-format found issues in your code. ⚠️

You can test this locally with the following command:

git-clang-format --diff origin/main HEAD --extensions cpp -- llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp llvm/lib/Target/RISCV/RISCVISelLowering.cpp

⚠️
The reproduction instructions above might return results for more than one PR
in a stack if you are using a stacked PR workflow. You can limit the results by
changing origin/main to the base branch/commit you want to compare against.
⚠️

View the diff from clang-format here.

diff --git a/llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp b/llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp
index 56e273c8d..e2d9adecd 100644
--- a/llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp
+++ b/llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp
@@ -4019,7 +4019,7 @@ void SelectionDAGBuilder::visitBitCast(const User &I) {
   // constant integer as an opaque constant.
   else if(ConstantInt *C = dyn_cast<ConstantInt>(I.getOperand(0)))
     setValue(&I, DAG.getConstant(C->getValue(), dl, DestVT, /*isTarget=*/false,
-                                 /*isOpaque*/false));
+                                 /*isOpaque*/ false));
   else
     setValue(&I, N);            // noop cast.
 }
diff --git a/llvm/lib/Target/RISCV/RISCVISelLowering.cpp b/llvm/lib/Target/RISCV/RISCVISelLowering.cpp
index 7d77535fb..323352eb5 100644
--- a/llvm/lib/Target/RISCV/RISCVISelLowering.cpp
+++ b/llvm/lib/Target/RISCV/RISCVISelLowering.cpp
@@ -9424,7 +9424,8 @@ SDValue RISCVTargetLowering::lowerSELECT(SDValue Op, SelectionDAG &DAG) const {
         SDValue SubOp;
         using namespace llvm::SDPatternMatch;
         SDValue ShAmt;
-        if (sd_match(RegV, m_OneUse(m_Not(m_OneUse(m_Shl(m_AllOnes(), m_Value(ShAmt))))))) {
+        if (sd_match(RegV, m_OneUse(m_Not(m_OneUse(
+                               m_Shl(m_AllOnes(), m_Value(ShAmt))))))) {
           SDValue One = DAG.getConstant(1, DL, VT);
           SDValue Shl = DAG.getNode(ISD::SHL, DL, VT, One, ShAmt);
           SDValue SubAmt = DAG.getConstant(1 + RawConstVal, DL, VT);

[topperc]

I found three patterns:

1. e.g. 23456b036ecbc7d0
   This can be addressed by the change in RISCVISelLowering.cpp.
2. e.g. d1e7a5db560b32a5
   Similar, but the inner ADDI is ADDIW until riscv-opt-w-instrs. How to handle?
3. e.g. 011a43fe5b60424c
   This is because of an opaque constant. What are opaque constants? I cannot find any documentation.

Opaque constants are constants that have been hoisted by the ConstantHoisting pass. https://github.com/llvm/llvm-project/blob/main/llvm/lib/Transforms/Scalar/ConstantHoisting.cpp

That pass uses the getIntImmCostInst TTI hook to decide what constants are expensive and can't be folded.

[pfusik]

Why did we have an opaque constant here?

[topperc]

The ConstantHoisting pass adds a NOP bitcast to mark the constants that have been hoisted. bitcasts of constants normally should be optimized out before the codegen pipeline starts. So the only cases that SelectionDAG should see come from the ConstantHoisting pass.