[x86] add load folding restriction to pushAddIntoCmovOfConsts()

[spatel]

Summary

With only a load-fold the diffs look neutral. If there's a load and store (rmw) fold opportunity as shown in the test based on #53862, then we end up with an extra instruction.

[pengfei]

Tests LGTM.

[RKSimon]

LGTM - cheers

llvm/lib/Target/X86/X86ISelLowering.cpp

SDValue RHS =
       DAG.getVectorShuffle(OpVT, DL, N0.getOperand(1), N1.getOperand(1), Mask);
   return DAG.getNode(X86ISD::VPMADDWD, DL, VT, LHS, RHS);
 }
 
 /// CMOV of constants requires materializing constant operands in registers.
 /// Try to fold those constants into an 'add' instruction to reduce instruction
 /// count. We do this with CMOV rather the generic 'select' because there are
 /// earlier folds that may be used to turn select-of-constants into logic hacks.
-static SDValue pushAddIntoCmovOfConsts(SDNode *N, SelectionDAG &DAG) {
+static SDValue pushAddIntoCmovOfConsts(SDNode *N, SelectionDAG &DAG,
+                                       const X86Subtarget &Subtarget) {
   // If an operand is zero, add-of-0 gets simplified away, so that's clearly
   // better because we eliminate 1-2 instructions. This transform is still
   // an improvement without zero operands because we trade 2 move constants and
   // 1 add for 2 adds (LEA) as long as the constants can be represented as
   // immediate asm operands (fit in 32-bits).
   auto isSuitableCmov = [](SDValue V) {
     if (V.getOpcode() != X86ISD::CMOV || !V.hasOneUse())
       return false;
     if (!isa<ConstantSDNode>(V.getOperand(0)) ||
         !isa<ConstantSDNode>(V.getOperand(1)))
       return false;
     return isNullConstant(V.getOperand(0)) || isNullConstant(V.getOperand(1)) ||
            (V.getConstantOperandAPInt(0).isSignedIntN(32) &&
             V.getConstantOperandAPInt(1).isSignedIntN(32));
   };
 
   // Match an appropriate CMOV as the first operand of the add.
   SDValue Cmov = N->getOperand(0);
   SDValue OtherOp = N->getOperand(1);
   if (!isSuitableCmov(Cmov))
     std::swap(Cmov, OtherOp);
   if (!isSuitableCmov(Cmov))
     return SDValue();
 
+  // Don't remove a load folding opportunity for the add. That would neutralize
+  // any improvements from removing constant materializations.
+  if (X86::mayFoldLoad(OtherOp, Subtarget))
+    return SDValue();
+
   EVT VT = N->getValueType(0);
   SDLoc DL(N);
   SDValue FalseOp = Cmov.getOperand(0);
   SDValue TrueOp = Cmov.getOperand(1);
 
   // We will push the add through the select, but we can potentially do better
   // if we know there is another add in the sequence and this is pointer math.
   // In that case, we can absorb an add into the trailing memory op and avoid

 static SDValue combineAdd(SDNode *N, SelectionDAG &DAG,
                           TargetLowering::DAGCombinerInfo &DCI,
                           const X86Subtarget &Subtarget) {
   EVT VT = N->getValueType(0);
   SDValue Op0 = N->getOperand(0);
   SDValue Op1 = N->getOperand(1);
   SDLoc DL(N);
 
-  if (SDValue Select = pushAddIntoCmovOfConsts(N, DAG))
+  if (SDValue Select = pushAddIntoCmovOfConsts(N, DAG, Subtarget))
     return Select;
 
   if (SDValue MAdd = matchPMADDWD(DAG, Op0, Op1, DL, VT, Subtarget))
     return MAdd;
   if (SDValue MAdd = matchPMADDWD_2(DAG, Op0, Op1, DL, VT, Subtarget))
     return MAdd;
   if (SDValue MAdd = combineAddOfPMADDWD(DAG, Op0, Op1, DL, VT))
     return MAdd;

llvm/test/CodeGen/X86/add-cmov.ll

; CHECK-NEXT:    retq
   %dec = add i16 %ld, -1
   store i16 %dec, i16* %sel, align 4
   ret void
 }
 
 define i32 @loadfold_select_const_arms(i32* %x, i1 %y) {
 ; CHECK-LABEL: loadfold_select_const_arms:
 ; CHECK:       # %bb.0:
-; CHECK-NEXT:    movl (%rdi), %eax
-; CHECK-NEXT:    leal -10(%rax), %ecx
-; CHECK-NEXT:    addl $10, %eax
 ; CHECK-NEXT:    testb $1, %sil
-; CHECK-NEXT:    cmovel %ecx, %eax
-; CHECK-NEXT:    # kill: def $eax killed $eax killed $rax
+; CHECK-NEXT:    movl $10, %ecx
+; CHECK-NEXT:    movl $-10, %eax
+; CHECK-NEXT:    cmovnel %ecx, %eax
+; CHECK-NEXT:    addl (%rdi), %eax
 ; CHECK-NEXT:    retq
   %cond = select i1 %y, i32 10, i32 -10
   %t0 = load i32, i32* %x, align 4
   %add = add nsw i32 %t0, %cond
   ret i32 %add
 }
 
 define void @rmw_add(i32* %x, i1 %y, i32 %z, i32 %w) {
; CHECK-NEXT:    retq
   %add = add nsw i32 %t0, %cond
   store i32 %add, i32* %x, align 4
   ret void
 }
 
 define void @rmw_select_const_arms(i32* %x, i1 %y) {
 ; CHECK-LABEL: rmw_select_const_arms:
 ; CHECK:       # %bb.0:
-; CHECK-NEXT:    movl (%rdi), %eax
-; CHECK-NEXT:    leal -10(%rax), %ecx
-; CHECK-NEXT:    addl $10, %eax
 ; CHECK-NEXT:    testb $1, %sil
-; CHECK-NEXT:    cmovel %ecx, %eax
-; CHECK-NEXT:    movl %eax, (%rdi)
+; CHECK-NEXT:    movl $10, %eax
+; CHECK-NEXT:    movl $-10, %ecx
+; CHECK-NEXT:    cmovnel %eax, %ecx
+; CHECK-NEXT:    addl %ecx, (%rdi)
 ; CHECK-NEXT:    retq
   %cond = select i1 %y, i32 10, i32 -10
   %t0 = load i32, i32* %x, align 4
   %add = add nsw i32 %t0, %cond
   store i32 %add, i32* %x, align 4
   ret void
 }
 
; CHECK-NEXT:    retq
   %add = add nsw i32 %t0, %cond
   store i32 %add, i32* %x, align 4
   ret i32 %t0
 }
 
 define i32 @rmw_select_const_arms_extra_add_use(i32* %x, i1 %y) {
 ; CHECK-LABEL: rmw_select_const_arms_extra_add_use:
 ; CHECK:       # %bb.0:
-; CHECK-NEXT:    movl (%rdi), %eax
-; CHECK-NEXT:    leal -10(%rax), %ecx
-; CHECK-NEXT:    addl $10, %eax
 ; CHECK-NEXT:    testb $1, %sil
-; CHECK-NEXT:    cmovel %ecx, %eax
+; CHECK-NEXT:    movl $10, %ecx
+; CHECK-NEXT:    movl $-10, %eax
+; CHECK-NEXT:    cmovnel %ecx, %eax
+; CHECK-NEXT:    addl (%rdi), %eax
 ; CHECK-NEXT:    movl %eax, (%rdi)
-; CHECK-NEXT:    # kill: def $eax killed $eax killed $rax
 ; CHECK-NEXT:    retq
   %cond = select i1 %y, i32 10, i32 -10
   %t0 = load i32, i32* %x, align 4
   %add = add nsw i32 %t0, %cond
   store i32 %add, i32* %x, align 4
   ret i32 %add
 }