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

[craig.topper]

Summary

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.

Still need to add the test, but wanted to get a check that this was
the right direction.

Fixes PR58511.

[craig.topper]

Add test

[spatel]

LGTM

llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp

2288

Add a note here or to the text above to explain that we don't re-use an opaque 0/1 because that could inf-loop.

[craig.topper]

llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp

2288

The 0/-1 weren’t opaque. I was just being paranoid about using 0/-1 that might be mixed with undef. Do I need to worry about that here?

[spatel]

llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp

2288

Ah, I misunderstood how this was looping.
Still worth a note that we're creating 0/-1 directly here rather than relying on getNode because CBO could be opaque?

[craig.topper]

llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp

2288

For posterity, we loop between these two states

Combining: t15: i32 = or t13, OpaqueConstant:i32<2048>                           
Creating new node: t23: i32 = or Constant:i32<-1>, OpaqueConstant:i32<2048>      
Creating new node: t24: i32 = or Constant:i32<0>, OpaqueConstant:i32<2048>       
Creating new node: t25: i32 = select t7, t23, t24                                
 ... into: t25: i32 = select t7, t23, t24                                        
                                                                                 
Combining: t19: i64 = any_extend t25                                             
                                                                                 
Combining: t25: i32 = select t7, t23, t24                                        
Creating new node: t26: i32 = select t7, Constant:i32<-1>, Constant:i32<0>       
Creating new node: t27: i32 = or t26, OpaqueConstant:i32<2048>                   
 ... into: t27: i32 = or t26, OpaqueConstant:i32<2048>

llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp

SDValue DAGCombiner::foldBinOpIntoSelect(SDNode *BO) {
 
   // Bail out if any constants are opaque because we can't constant fold those.
   // The exception is "and" and "or" with either 0 or -1 in which case we can
   // propagate non constant operands into select. I.e.:
   // and (select Cond, 0, -1), X --> select Cond, 0, X
   // 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 &&
       !isConstantOrConstantVector(CBO, true) &&
       !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);
+  SDValue NewCT, NewCF;
+
+  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)
     if (!CanFoldNonConst && !NewCT.isUndef() &&
         !isConstantOrConstantVector(NewCT, true) &&
         !DAG.isConstantFPBuildVectorOrConstantFP(NewCT))
       return SDValue();
 
-  SDValue NewCF = SelOpNo ? DAG.getNode(BinOpcode, DL, VT, CBO, CF)
-                          : DAG.getNode(BinOpcode, DL, VT, CF, CBO);
+    NewCF = SelOpNo ? DAG.getNode(BinOpcode, DL, VT, CBO, CF)
     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());
   return SelectOp;
 }
 
 static SDValue foldAddSubBoolOfMaskedVal(SDNode *N, SelectionDAG &DAG) {
   assert((N->getOpcode() == ISD::ADD || N->getOpcode() == ISD::SUB) &&

llvm/test/CodeGen/RISCV/pr58511.ll

+; 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
+}