[llvm] 0081671 - [DAGCombiner][RISCV] Make foldBinOpIntoSelect work correctly with opaque constants.

Craig Topper via llvm-commits llvm-commits at lists.llvm.org
Sat Oct 22 19:13:54 PDT 2022


Author: Craig Topper
Date: 2022-10-22T19:10:33-07:00
New Revision: 00816714f96505c59c236f3f0fd2eb815c57f674

URL: https://github.com/llvm/llvm-project/commit/00816714f96505c59c236f3f0fd2eb815c57f674
DIFF: https://github.com/llvm/llvm-project/commit/00816714f96505c59c236f3f0fd2eb815c57f674.diff

LOG: [DAGCombiner][RISCV] Make foldBinOpIntoSelect work correctly with opaque constants.

The CanFoldNonConst doesn't work correctly with opaque constants
because getNode won't constant fold constants if one is opaque. Even
if the operation is AND/OR. This can lead to infinite loops.

This patch does the folding manually in the DAGCombine. Alternatively,
we could improve getNode but that seemed likely to have bigger impact
and possibly increase compile time for the additional checks. We wouldn't
want to directly constant fold because we need to preserve the opaque flag.

Fixes PR58511.

Reviewed By: spatel

Differential Revision: https://reviews.llvm.org/D136472

Added: 
    llvm/test/CodeGen/RISCV/pr58511.ll

Modified: 
    llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp

Removed: 
    


################################################################################
diff  --git a/llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp b/llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp
index 38cd6b5e666a..a4c04b525bdf 100644
--- a/llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp
+++ b/llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp
@@ -2271,8 +2271,8 @@ SDValue DAGCombiner::foldBinOpIntoSelect(SDNode *BO) {
   // or X, (select Cond, -1, 0) --> select Cond, -1, X
   bool CanFoldNonConst =
       (BinOpcode == ISD::AND || BinOpcode == ISD::OR) &&
-      (isNullOrNullSplat(CT) || isAllOnesOrAllOnesSplat(CT)) &&
-      (isNullOrNullSplat(CF) || isAllOnesOrAllOnesSplat(CF));
+      ((isNullOrNullSplat(CT) && isAllOnesOrAllOnesSplat(CF)) ||
+       (isNullOrNullSplat(CF) && isAllOnesOrAllOnesSplat(CT)));
 
   SDValue CBO = BO->getOperand(SelOpNo ^ 1);
   if (!CanFoldNonConst &&
@@ -2280,23 +2280,41 @@ SDValue DAGCombiner::foldBinOpIntoSelect(SDNode *BO) {
       !DAG.isConstantFPBuildVectorOrConstantFP(CBO))
     return SDValue();
 
-  // We have a select-of-constants followed by a binary operator with a
-  // constant. Eliminate the binop by pulling the constant math into the select.
-  // Example: add (select Cond, CT, CF), CBO --> select Cond, CT + CBO, CF + CBO
   SDLoc DL(Sel);
-  SDValue NewCT = SelOpNo ? DAG.getNode(BinOpcode, DL, VT, CBO, CT)
-                          : DAG.getNode(BinOpcode, DL, VT, CT, CBO);
-  if (!CanFoldNonConst && !NewCT.isUndef() &&
-      !isConstantOrConstantVector(NewCT, true) &&
-      !DAG.isConstantFPBuildVectorOrConstantFP(NewCT))
-    return SDValue();
+  SDValue NewCT, NewCF;
 
-  SDValue NewCF = SelOpNo ? DAG.getNode(BinOpcode, DL, VT, CBO, CF)
-                          : DAG.getNode(BinOpcode, DL, VT, CF, CBO);
-  if (!CanFoldNonConst && !NewCF.isUndef() &&
-      !isConstantOrConstantVector(NewCF, true) &&
-      !DAG.isConstantFPBuildVectorOrConstantFP(NewCF))
-    return SDValue();
+  if (CanFoldNonConst) {
+    // If CBO is an opaque constant, we can't rely on getNode to constant fold.
+    if ((BinOpcode == ISD::AND && isNullOrNullSplat(CT)) ||
+        (BinOpcode == ISD::OR && isAllOnesOrAllOnesSplat(CT)))
+      NewCT = CT;
+    else
+      NewCT = CBO;
+
+    if ((BinOpcode == ISD::AND && isNullOrNullSplat(CF)) ||
+        (BinOpcode == ISD::OR && isAllOnesOrAllOnesSplat(CF)))
+      NewCF = CF;
+    else
+      NewCF = CBO;
+  } else {
+    // We have a select-of-constants followed by a binary operator with a
+    // constant. Eliminate the binop by pulling the constant math into the
+    // select. Example: add (select Cond, CT, CF), CBO --> select Cond, CT +
+    // CBO, CF + CBO
+    NewCT = SelOpNo ? DAG.getNode(BinOpcode, DL, VT, CBO, CT)
+                    : DAG.getNode(BinOpcode, DL, VT, CT, CBO);
+    if (!CanFoldNonConst && !NewCT.isUndef() &&
+        !isConstantOrConstantVector(NewCT, true) &&
+        !DAG.isConstantFPBuildVectorOrConstantFP(NewCT))
+      return SDValue();
+
+    NewCF = SelOpNo ? DAG.getNode(BinOpcode, DL, VT, CBO, CF)
+                    : DAG.getNode(BinOpcode, DL, VT, CF, CBO);
+    if (!CanFoldNonConst && !NewCF.isUndef() &&
+        !isConstantOrConstantVector(NewCF, true) &&
+        !DAG.isConstantFPBuildVectorOrConstantFP(NewCF))
+      return SDValue();
+  }
 
   SDValue SelectOp = DAG.getSelect(DL, VT, Sel.getOperand(0), NewCT, NewCF);
   SelectOp->setFlags(BO->getFlags());

diff  --git a/llvm/test/CodeGen/RISCV/pr58511.ll b/llvm/test/CodeGen/RISCV/pr58511.ll
new file mode 100644
index 000000000000..ed520af4645e
--- /dev/null
+++ b/llvm/test/CodeGen/RISCV/pr58511.ll
@@ -0,0 +1,83 @@
+; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py
+; RUN: llc < %s -mtriple=riscv64 | FileCheck %s
+
+define i32 @f(i1 %0, i32 %1, ptr %2) {
+; CHECK-LABEL: f:
+; CHECK:       # %bb.0: # %BB
+; CHECK-NEXT:    slliw a3, a1, 11
+; CHECK-NEXT:    slliw a1, a1, 12
+; CHECK-NEXT:    subw a1, a1, a3
+; CHECK-NEXT:    andi a0, a0, 1
+; CHECK-NEXT:    neg a0, a0
+; CHECK-NEXT:    lui a3, 1
+; CHECK-NEXT:    addiw a3, a3, -2048
+; CHECK-NEXT:    or a0, a0, a3
+; CHECK-NEXT:    sw a1, 0(a2)
+; CHECK-NEXT:    ret
+BB:
+  %I = select i1 %0, i32 -1, i32 0
+  %I1 = mul i32 %1, 2048
+  %I2 = or i32 2048, %I
+  store i32 %I1, ptr %2
+  ret i32 %I2
+}
+
+define i32 @g(i1 %0, i32 %1, ptr %2) {
+; CHECK-LABEL: g:
+; CHECK:       # %bb.0: # %BB
+; CHECK-NEXT:    slliw a3, a1, 11
+; CHECK-NEXT:    slliw a1, a1, 12
+; CHECK-NEXT:    subw a1, a1, a3
+; CHECK-NEXT:    andi a0, a0, 1
+; CHECK-NEXT:    addi a0, a0, -1
+; CHECK-NEXT:    lui a3, 1
+; CHECK-NEXT:    addiw a3, a3, -2048
+; CHECK-NEXT:    or a0, a0, a3
+; CHECK-NEXT:    sw a1, 0(a2)
+; CHECK-NEXT:    ret
+BB:
+  %I = select i1 %0, i32 0, i32 -1
+  %I1 = mul i32 %1, 2048
+  %I2 = or i32 2048, %I
+  store i32 %I1, ptr %2
+  ret i32 %I2
+}
+
+define i32 @h(i1 %0, i32 %1, ptr %2) {
+; CHECK-LABEL: h:
+; CHECK:       # %bb.0: # %BB
+; CHECK-NEXT:    slliw a3, a1, 11
+; CHECK-NEXT:    slliw a1, a1, 12
+; CHECK-NEXT:    subw a1, a1, a3
+; CHECK-NEXT:    andi a0, a0, 1
+; CHECK-NEXT:    slli a0, a0, 11
+; CHECK-NEXT:    sw a1, 0(a2)
+; CHECK-NEXT:    ret
+BB:
+  %I = select i1 %0, i32 -1, i32 0
+  %I1 = mul i32 %1, 2048
+  %I2 = and i32 2048, %I
+  store i32 %I1, ptr %2
+  ret i32 %I2
+}
+
+define i32 @i(i1 %0, i32 %1, ptr %2) {
+; CHECK-LABEL: i:
+; CHECK:       # %bb.0: # %BB
+; CHECK-NEXT:    andi a0, a0, 1
+; CHECK-NEXT:    slliw a3, a1, 11
+; CHECK-NEXT:    slliw a1, a1, 12
+; CHECK-NEXT:    subw a1, a1, a3
+; CHECK-NEXT:    addi a0, a0, -1
+; CHECK-NEXT:    lui a3, 1
+; CHECK-NEXT:    addiw a3, a3, -2048
+; CHECK-NEXT:    and a0, a0, a3
+; CHECK-NEXT:    sw a1, 0(a2)
+; CHECK-NEXT:    ret
+BB:
+  %I = select i1 %0, i32 0, i32 -1
+  %I1 = mul i32 %1, 2048
+  %I2 = and i32 2048, %I
+  store i32 %I1, ptr %2
+  ret i32 %I2
+}


        


More information about the llvm-commits mailing list