[x86] limit vector increment fold to allow load folding

[spatel]

Summary

The tests are based on the example from:
https://llvm.org/PR52032

I suspect that it looks worse than it actually is. :)
That is, llvm-mca says there's no uop/timing difference with the load folding and pcmpeq vs. broadcast on Haswell (and probably other targets).
The load-folding definitely makes the code smaller, so it's good for that at least. So this requires carving a narrow hole in the transform to get just this case without changing others that look good as-is (in other words, the transform still seems good for most examples).

[lebedev.ri]

llvm/lib/Target/X86/X86ISelDAGToDAG.cpp

900

What about multi-use load, we usually don't load-fold those?

[spatel]

llvm/lib/Target/X86/X86ISelDAGToDAG.cpp

900

Yes, good point. I really want to use the same logic as in X86ISelLowering's MayFoldLoad, but it's a static function. Let me update that.

[spatel]

Patch updated:

1. Make mayFoldLoad visible to X86ISelDAGToDAG and use it in the predicate.
2. Added a test with extra uses of loads, so we can see the result in that case (unchanged / negative test for this patch).

I can make the helper visibility/formatting changes an NFC pre-commit if that looks ok. We have functions like that scattered around, so it's not clear to me if there's a better way.

[craig.topper]

llvm/lib/Target/X86/X86ISelLowering.cpp

5053

While you're in here, can you use cast here since the dyn_cast isn't checked for null.

[lebedev.ri]

SGTM in general.

llvm/lib/Target/X86/X86ISelDAGToDAG.cpp

901

I'm stuck on this one-use check. Is it really reasonable?
We don't really know that the constant is used nearby, do we?
There doesn't seem to be a test for it.
https://godbolt.org/z/Ea9sad3Ya

[spatel]

llvm/lib/Target/X86/X86ISelLowering.cpp

5053

Yes - I can do that as a 1-liner. Beyond that, I wasn't sure what cleanup we want to do.
Could update this group of 4 "MayFold*" functions together (pull the declarations into the header and X86 namespace even though there's no current external user of the other 3)?

[spatel]

llvm/lib/Target/X86/X86ISelDAGToDAG.cpp

901

Right, we don't know exactly where the other use of that constant is or even if it's part of another increment op with this check, so it's a (weak) heuristic.

I'll add a test like you've shown. Without that check, we would alter codegen on it as shown below. It's a close call, but I think it's a regression to increase the load uops on that -- even if it is one less macro instruction.

diff --git a/llvm/test/CodeGen/X86/combine-sub.ll b/llvm/test/CodeGen/X86/combine-sub.ll
index a399c5175dd6..5090895c0ab8 100644
--- a/llvm/test/CodeGen/X86/combine-sub.ll
+++ b/llvm/test/CodeGen/X86/combine-sub.ll
@@ -290,9 +290,8 @@ define void @PR52032_oneuse_constant(<8 x i32>* %p) {
 ;
 ; AVX-LABEL: PR52032_oneuse_constant:
 ; AVX:       # %bb.0:
-; AVX-NEXT:    vmovdqu (%rdi), %ymm0
-; AVX-NEXT:    vpcmpeqd %ymm1, %ymm1, %ymm1
-; AVX-NEXT:    vpsubd %ymm1, %ymm0, %ymm0
+; AVX-NEXT:    vpbroadcastd {{.*#+}} ymm0 = [1,1,1,1,1,1,1,1]
+; AVX-NEXT:    vpaddd (%rdi), %ymm0, %ymm0
 ; AVX-NEXT:    vmovdqu %ymm0, (%rdi)
 ; AVX-NEXT:    vzeroupper
 ; AVX-NEXT:    retq

[spatel]

Patch updated:
Added one-use-of-constant "1" test (currently negative for this patch) with explanatory comment.

[craig.topper]

llvm/lib/Target/X86/X86ISelLowering.cpp

5042

Not related to this patch, but this function is incorrect for SSE which requires alignment or a subtarget feature that disables the alignment check. Are all existing uses scalar or AVX only?

[craig.topper]

llvm/lib/Target/X86/X86ISelLowering.cpp

5042

We also don't fold non-temporal loads if we have an instruction for it. See useNonTemporalLoad

[RKSimon]

llvm/lib/Target/X86/X86ISelLowering.h

915

(minor) Add description now that its exposed

[spatel]

llvm/lib/Target/X86/X86ISelLowering.cpp

5042

Let's try to deal with this first (and fold in the formatting changes to that patch):
D112545

[spatel]

Patch updated:
Rebased after 6c0a2c2804c0 and added function definition code comment.

[RKSimon]

LGTM - although might be better to pre-commit the change to X86::mayFoldLoad separately from the actual PR52032 fix

[RKSimon]

Any other comments?

[lebedev.ri]

Seems fine to me.

llvm/lib/Target/X86/X86ISelDAGToDAG.cpp

if (N->getOpcode() == X86ISD::AND && !N->hasAnyUseOfValue(1)) {
                                     N->getOperand(0), N->getOperand(1));
       --I;
       CurDAG->ReplaceAllUsesOfValueWith(SDValue(N, 0), Res);
       ++I;
       MadeChange = true;
       continue;
     }
 
-    /// Convert vector increment or decrement to sub/add with an all-ones
-    /// constant:
-    /// add X, <1, 1...> --> sub X, <-1, -1...>
-    /// sub X, <1, 1...> --> add X, <-1, -1...>
-    /// The all-ones vector constant can be materialized using a pcmpeq
-    /// instruction that is commonly recognized as an idiom (has no register
-    /// dependency), so that's better/smaller than loading a splat 1 constant.
+    // Convert vector increment or decrement to sub/add with an all-ones
+    // constant:
+    // add X, <1, 1...> --> sub X, <-1, -1...>
+    // sub X, <1, 1...> --> add X, <-1, -1...>
+    // The all-ones vector constant can be materialized using a pcmpeq
+    // instruction that is commonly recognized as an idiom (has no register
+    // dependency), so that's better/smaller than loading a splat 1 constant.
+    //
+    // But don't do this if it would inhibit a potentially profitable load
+    // folding opportunity for the other operand. That only occurs with the
+    // intersection of:
+    // (1) The other operand (op0) is load foldable.
+    // (2) The op is an add (otherwise, we are *creating* an add and can still
+    //     load fold the other op).
+    // (3) The target has AVX (otherwise, we have a destructive add and can't
+    //     load fold the other op without killing the constant op).
+    // (4) The constant 1 vector has multiple uses (so it is profitable to load
+    //     into a register anyway).
+    auto mayPreventLoadFold = [&]() {
+      return X86::mayFoldLoad(N->getOperand(0)) && N->getOpcode() == ISD::ADD &&
+             Subtarget->hasAVX() && !N->getOperand(1).hasOneUse();
+    };
     if ((N->getOpcode() == ISD::ADD || N->getOpcode() == ISD::SUB) &&
-        N->getSimpleValueType(0).isVector()) {
-
+        N->getSimpleValueType(0).isVector() && !mayPreventLoadFold()) {
       APInt SplatVal;
       if (X86::isConstantSplat(N->getOperand(1), SplatVal) &&
           SplatVal.isOne()) {
         SDLoc DL(N);
 
         MVT VT = N->getSimpleValueType(0);
         unsigned NumElts = VT.getSizeInBits() / 32;
         SDValue AllOnes =

llvm/lib/Target/X86/X86ISelLowering.h

bool isCalleePop(CallingConv::ID CallingConv,
                      bool is64Bit, bool IsVarArg, bool GuaranteeTCO);
 
     /// If Op is a constant whose elements are all the same constant or
     /// undefined, return true and return the constant value in \p SplatVal.
     /// If we have undef bits that don't cover an entire element, we treat these
     /// as zero if AllowPartialUndefs is set, else we fail and return false.
     bool isConstantSplat(SDValue Op, APInt &SplatVal,
                          bool AllowPartialUndefs = true);
+
+    bool mayFoldLoad(SDValue Op, bool AssumeSingleUse = false);
   } // end namespace X86
 
   //===--------------------------------------------------------------------===//
   //  X86 Implementation of the TargetLowering interface
   class X86TargetLowering final : public TargetLowering {
   public:
     explicit X86TargetLowering(const X86TargetMachine &TM,
                                const X86Subtarget &STI);

llvm/lib/Target/X86/X86ISelLowering.cpp

X86TargetLowering::createFastISel(FunctionLoweringInfo &funcInfo,
                                   const TargetLibraryInfo *libInfo) const {
   return X86::createFastISel(funcInfo, libInfo);
 }
 
 //===----------------------------------------------------------------------===//
 //                           Other Lowering Hooks
 //===----------------------------------------------------------------------===//
 
-static bool MayFoldLoad(SDValue Op, bool AssumeSingleUse = false) {
+bool X86::mayFoldLoad(SDValue Op, bool AssumeSingleUse) {
   return (AssumeSingleUse || Op.hasOneUse()) && ISD::isNormalLoad(Op.getNode());
 }
 
 static bool MayFoldLoadIntoBroadcastFromMem(SDValue Op, MVT EltVT,
                                             bool AssumeSingleUse = false) {
-  if (!MayFoldLoad(Op, AssumeSingleUse))
+  if (!X86::mayFoldLoad(Op, AssumeSingleUse))
     return false;
 
   // We can not replace a wide volatile load with a broadcast-from-memory,
   // because that would narrow the load, which isn't legal for volatiles.
   const LoadSDNode *Ld = dyn_cast<LoadSDNode>(Op.getNode());
   return !Ld->isVolatile() ||
          Ld->getValueSizeInBits(0) == EltVT.getScalarSizeInBits();
 }
 
 static bool MayFoldIntoStore(SDValue Op) {
   return Op.hasOneUse() && ISD::isNormalStore(*Op.getNode()->use_begin());
 }
 
static SDValue lowerShuffleAsDecomposedShuffleMerge(
   // If we effectively only demand the 0'th element of \p Input, and not only
   // as 0'th element, then broadcast said input,
   // and change \p InputMask to be a no-op (identity) mask.
   auto canonicalizeBroadcastableInput = [DL, VT, &Subtarget,
                                          &DAG](SDValue &Input,
                                                MutableArrayRef<int> InputMask) {
     unsigned EltSizeInBits = Input.getScalarValueSizeInBits();
     if (!Subtarget.hasAVX2() &&
-        (!Subtarget.hasAVX() || EltSizeInBits < 32 || !MayFoldLoad(Input)))
+        (!Subtarget.hasAVX() || EltSizeInBits < 32 || !X86::mayFoldLoad(Input)))
       return;
     if (isNoopShuffleMask(InputMask))
       return;
     assert(isBroadcastShuffleMask(InputMask) &&
            "Expected to demand only the 0'th element.");
     Input = DAG.getNode(X86ISD::VBROADCAST, DL, VT, Input);
     for (auto I : enumerate(InputMask)) {
       int &InputMaskElt = I.value();
static SDValue lowerV2X128Shuffle(const SDLoc &DL, MVT VT, SDValue V1,
                                   const APInt &Zeroable,
                                   const X86Subtarget &Subtarget,
                                   SelectionDAG &DAG) {
   if (V2.isUndef()) {
     // Attempt to match VBROADCAST*128 subvector broadcast load.
     bool SplatLo = isShuffleEquivalent(Mask, {0, 1, 0, 1}, V1);
     bool SplatHi = isShuffleEquivalent(Mask, {2, 3, 2, 3}, V1);
     if ((SplatLo || SplatHi) && !Subtarget.hasAVX512() && V1.hasOneUse() &&
-        MayFoldLoad(peekThroughOneUseBitcasts(V1))) {
+        X86::mayFoldLoad(peekThroughOneUseBitcasts(V1))) {
       auto *Ld = cast<LoadSDNode>(peekThroughOneUseBitcasts(V1));
       if (!Ld->isNonTemporal()) {
         MVT MemVT = VT.getHalfNumVectorElementsVT();
         unsigned Ofs = SplatLo ? 0 : MemVT.getStoreSize();
         SDVTList Tys = DAG.getVTList(VT, MVT::Other);
         SDValue Ptr = DAG.getMemBasePlusOffset(Ld->getBasePtr(),
                                                TypeSize::Fixed(Ofs), DL);
         SDValue Ops[] = {Ld->getChain(), Ptr};
if (VT.is256BitVector() || VT.is512BitVector()) {
     assert(isPowerOf2_32(NumEltsIn128) &&
            "Vectors will always have power-of-two number of elements.");
 
     // If we are not inserting into the low 128-bit vector chunk,
     // then prefer the broadcast+blend sequence.
     // FIXME: relax the profitability check iff all N1 uses are insertions.
     if (!VT.is128BitVector() && IdxVal >= NumEltsIn128 &&
         ((Subtarget.hasAVX2() && EltSizeInBits != 8) ||
-         (Subtarget.hasAVX() && (EltSizeInBits >= 32) && MayFoldLoad(N1)))) {
+         (Subtarget.hasAVX() && (EltSizeInBits >= 32) && X86::mayFoldLoad(N1)))) {
       SDValue N1SplatVec = DAG.getSplatBuildVector(VT, dl, N1);
       SmallVector<int, 8> BlendMask;
       for (unsigned i = 0; i != NumElts; ++i)
         BlendMask.push_back(i == IdxVal ? i + NumElts : i);
       return DAG.getVectorShuffle(VT, dl, N0, N1SplatVec, BlendMask);
     }
 
     // Get the desired 128-bit vector chunk.
if (EltVT == MVT::f32) {
       //   these bits. For example (insert (extract, 3), 2) could be matched by
       //   putting the '3' into bits [7:6] of X86ISD::INSERTPS.
       // Bits [5:4] of the constant are the destination select. This is the
       //   value of the incoming immediate.
       // Bits [3:0] of the constant are the zero mask. The DAG Combiner may
       //   combine either bitwise AND or insert of float 0.0 to set these bits.
 
       bool MinSize = DAG.getMachineFunction().getFunction().hasMinSize();
-      if (IdxVal == 0 && (!MinSize || !MayFoldLoad(N1))) {
+      if (IdxVal == 0 && (!MinSize || !X86::mayFoldLoad(N1))) {
         // If this is an insertion of 32-bits into the low 32-bits of
         // a vector, we prefer to generate a blend with immediate rather
         // than an insertps. Blends are simpler operations in hardware and so
         // will always have equal or better performance than insertps.
         // But if optimizing for size and there's a load folding opportunity,
         // generate insertps because blendps does not have a 32-bit memory
         // operand form.
         N1 = DAG.getNode(ISD::SCALAR_TO_VECTOR, dl, MVT::v4f32, N1);
SDValue X86TargetLowering::LowerSELECT(SDValue Op, SelectionDAG &DAG) const {
 
   // Or finally, promote i8 cmovs if we have CMOV,
   //                 or i16 cmovs if it won't prevent folding a load.
   // FIXME: we should not limit promotion of i8 case to only when the CMOV is
   //        legal, but EmitLoweredSelect() can not deal with these extensions
   //        being inserted between two CMOV's. (in i16 case too TBN)
   //        https://bugs.llvm.org/show_bug.cgi?id=40974
   if ((Op.getValueType() == MVT::i8 && Subtarget.hasCMov()) ||
-      (Op.getValueType() == MVT::i16 && !MayFoldLoad(Op1) &&
-       !MayFoldLoad(Op2))) {
+      (Op.getValueType() == MVT::i16 && !X86::mayFoldLoad(Op1) &&
+       !X86::mayFoldLoad(Op2))) {
     Op1 = DAG.getNode(ISD::ANY_EXTEND, DL, MVT::i32, Op1);
     Op2 = DAG.getNode(ISD::ANY_EXTEND, DL, MVT::i32, Op2);
     SDValue Ops[] = { Op2, Op1, CC, Cond };
     SDValue Cmov = DAG.getNode(X86ISD::CMOV, DL, MVT::i32, Ops);
     return DAG.getNode(ISD::TRUNCATE, DL, Op.getValueType(), Cmov);
   }
 
   // X86ISD::CMOV means set the result (which is operand 1) to the RHS if
if (UnaryShuffle) {
     // Attempt to match against broadcast-from-vector.
     // Limit AVX1 to cases where we're loading+broadcasting a scalar element.
     if ((Subtarget.hasAVX2() ||
          (Subtarget.hasAVX() && 32 <= MaskEltSizeInBits)) &&
         (!IsMaskedShuffle || NumRootElts == NumMaskElts)) {
       if (isUndefOrEqual(Mask, 0)) {
         if (V1.getValueType() == MaskVT &&
             V1.getOpcode() == ISD::SCALAR_TO_VECTOR &&
-            MayFoldLoad(V1.getOperand(0))) {
+            X86::mayFoldLoad(V1.getOperand(0))) {
           if (Depth == 0 && Root.getOpcode() == X86ISD::VBROADCAST)
             return SDValue(); // Nothing to do!
           Res = V1.getOperand(0);
           Res = DAG.getNode(X86ISD::VBROADCAST, DL, MaskVT, Res);
           return DAG.getBitcast(RootVT, Res);
         }
         if (Subtarget.hasAVX2()) {
           if (Depth == 0 && Root.getOpcode() == X86ISD::VBROADCAST)
static SDValue combineCommutableSHUFP(SDValue N, MVT VT, const SDLoc &DL,
 
   // SHUFP(LHS, RHS) -> SHUFP(RHS, LHS) iff LHS is foldable + RHS is not.
   auto commuteSHUFP = [&VT, &DL, &DAG](SDValue Parent, SDValue V) {
     if (V.getOpcode() != X86ISD::SHUFP || !Parent->isOnlyUserOf(V.getNode()))
       return SDValue();
     SDValue N0 = V.getOperand(0);
     SDValue N1 = V.getOperand(1);
     unsigned Imm = V.getConstantOperandVal(2);
-    if (!MayFoldLoad(peekThroughOneUseBitcasts(N0)) ||
-        MayFoldLoad(peekThroughOneUseBitcasts(N1)))
+    if (!X86::mayFoldLoad(peekThroughOneUseBitcasts(N0)) ||
+        X86::mayFoldLoad(peekThroughOneUseBitcasts(N1)))
       return SDValue();
     Imm = ((Imm & 0x0F) << 4) | ((Imm & 0xF0) >> 4);
     return DAG.getNode(X86ISD::SHUFP, DL, VT, N1, N0,
                        DAG.getTargetConstant(Imm, DL, MVT::i8));
   };
 
   switch (N.getOpcode()) {
   case X86ISD::VPERMILPI:
if (Op0.getOpcode() == X86ISD::MOVDDUP && VT == MVT::v4f64 &&
       return DAG.getNode(X86ISD::VBROADCAST, DL, VT,
                          DAG.getNode(ISD::EXTRACT_VECTOR_ELT, DL, MVT::f64,
                                      Op0.getOperand(0),
                                      DAG.getIntPtrConstant(0, DL)));
 
     // concat_vectors(scalar_to_vector(x),scalar_to_vector(x)) -> broadcast(x)
     if (Op0.getOpcode() == ISD::SCALAR_TO_VECTOR &&
         (Subtarget.hasAVX2() ||
-         (EltSizeInBits >= 32 && MayFoldLoad(Op0.getOperand(0)))) &&
+         (EltSizeInBits >= 32 && X86::mayFoldLoad(Op0.getOperand(0)))) &&
         Op0.getOperand(0).getValueType() == VT.getScalarType())
       return DAG.getNode(X86ISD::VBROADCAST, DL, VT, Op0.getOperand(0));
 
     // concat_vectors(extract_subvector(broadcast(x)),
     //                extract_subvector(broadcast(x))) -> broadcast(x)
     if (Op0.getOpcode() == ISD::EXTRACT_SUBVECTOR &&
         Op0.getOperand(0).getValueType() == VT) {
       if (Op0.getOperand(0).getOpcode() == X86ISD::VBROADCAST ||
bool X86TargetLowering::IsDesirableToPromoteOp(SDValue Op, EVT &PVT) const {
   case ISD::ZERO_EXTEND:
   case ISD::ANY_EXTEND:
     break;
   case ISD::SHL:
   case ISD::SRA:
   case ISD::SRL: {
     SDValue N0 = Op.getOperand(0);
     // Look out for (store (shl (load), x)).
-    if (MayFoldLoad(N0) && IsFoldableRMW(N0, Op))
+    if (X86::mayFoldLoad(N0) && IsFoldableRMW(N0, Op))
       return false;
     break;
   }
   case ISD::ADD:
   case ISD::MUL:
   case ISD::AND:
   case ISD::OR:
   case ISD::XOR:
     Commute = true;
     LLVM_FALLTHROUGH;
   case ISD::SUB: {
     SDValue N0 = Op.getOperand(0);
     SDValue N1 = Op.getOperand(1);
     // Avoid disabling potential load folding opportunities.
-    if (MayFoldLoad(N1) &&
+    if (X86::mayFoldLoad(N1) &&
         (!Commute || !isa<ConstantSDNode>(N0) ||
          (Op.getOpcode() != ISD::MUL && IsFoldableRMW(N1, Op))))
       return false;
-    if (MayFoldLoad(N0) &&
+    if (X86::mayFoldLoad(N0) &&
         ((Commute && !isa<ConstantSDNode>(N1)) ||
          (Op.getOpcode() != ISD::MUL && IsFoldableRMW(N0, Op))))
       return false;
     if (IsFoldableAtomicRMW(N0, Op) ||
         (Commute && IsFoldableAtomicRMW(N1, Op)))
       return false;
   }
   }

llvm/test/CodeGen/X86/combine-sub.ll

 ; AVX-NEXT:    vpcmpeqd %xmm1, %xmm1, %xmm1
 ; AVX-NEXT:    vpsubd %xmm1, %xmm0, %xmm0
 ; AVX-NEXT:    retq
   %xor = xor <4 x i32> %x, <i32 -1, i32 -1, i32 -1, i32 -1>
   %sub = sub <4 x i32> zeroinitializer, %xor
   ret <4 x i32> %sub
 }
 
+; With AVX, this could use broadcast (an extra load) and
+; load-folded 'add', but currently we favor the virtually
+; free pcmpeq instruction.
+
 define void @PR52032_oneuse_constant(<8 x i32>* %p) {
 ; SSE-LABEL: PR52032_oneuse_constant:
 ; SSE:       # %bb.0:
 ; SSE-NEXT:    movdqu (%rdi), %xmm0
 ; SSE-NEXT:    movdqu 16(%rdi), %xmm1
 ; SSE-NEXT:    pcmpeqd %xmm2, %xmm2
 ; SSE-NEXT:    psubd %xmm2, %xmm1
 ; SSE-NEXT:    psubd %xmm2, %xmm0

 ; AVX-NEXT:    vzeroupper
 ; AVX-NEXT:    retq
   %i3 = load <8 x i32>, <8 x i32>* %p, align 4
   %i4 = add nsw <8 x i32> %i3, <i32 1, i32 1, i32 1, i32 1, i32 1, i32 1, i32 1, i32 1>
   store <8 x i32> %i4, <8 x i32>* %p, align 4
   ret void
 }
 
+; With AVX, we don't transform 'add' to 'sub' because that prevents load folding.
+; With SSE, we do it because we can't load fold the other op without overwriting the constant op.
+
 define void @PR52032(<8 x i32>* %p) {
 ; SSE-LABEL: PR52032:
 ; SSE:       # %bb.0:
 ; SSE-NEXT:    pcmpeqd %xmm0, %xmm0
 ; SSE-NEXT:    movdqu (%rdi), %xmm1
 ; SSE-NEXT:    movdqu 16(%rdi), %xmm2
 ; SSE-NEXT:    movdqu 32(%rdi), %xmm3
 ; SSE-NEXT:    movdqu 48(%rdi), %xmm4
 ; SSE-NEXT:    psubd %xmm0, %xmm2
 ; SSE-NEXT:    psubd %xmm0, %xmm1
 ; SSE-NEXT:    movdqu %xmm1, (%rdi)
 ; SSE-NEXT:    movdqu %xmm2, 16(%rdi)
 ; SSE-NEXT:    psubd %xmm0, %xmm4
 ; SSE-NEXT:    psubd %xmm0, %xmm3
 ; SSE-NEXT:    movdqu %xmm3, 32(%rdi)
 ; SSE-NEXT:    movdqu %xmm4, 48(%rdi)
 ; SSE-NEXT:    retq
 ;
 ; AVX-LABEL: PR52032:
 ; AVX:       # %bb.0:
-; AVX-NEXT:    vpcmpeqd %ymm0, %ymm0, %ymm0
-; AVX-NEXT:    vmovdqu (%rdi), %ymm1
-; AVX-NEXT:    vmovdqu 32(%rdi), %ymm2
-; AVX-NEXT:    vpsubd %ymm0, %ymm1, %ymm1
+; AVX-NEXT:    vpbroadcastd {{.*#+}} ymm0 = [1,1,1,1,1,1,1,1]
+; AVX-NEXT:    vpaddd (%rdi), %ymm0, %ymm1
 ; AVX-NEXT:    vmovdqu %ymm1, (%rdi)
-; AVX-NEXT:    vpsubd %ymm0, %ymm2, %ymm0
+; AVX-NEXT:    vpaddd 32(%rdi), %ymm0, %ymm0
 ; AVX-NEXT:    vmovdqu %ymm0, 32(%rdi)
 ; AVX-NEXT:    vzeroupper
 ; AVX-NEXT:    retq
   %i3 = load <8 x i32>, <8 x i32>* %p, align 4
   %i4 = add nsw <8 x i32> %i3, <i32 1, i32 1, i32 1, i32 1, i32 1, i32 1, i32 1, i32 1>
   store <8 x i32> %i4, <8 x i32>* %p, align 4
   %p2 = getelementptr inbounds <8 x i32>, <8 x i32>* %p, i64 1
   %i8 = load <8 x i32>, <8 x i32>* %p2, align 4
   %i9 = add nsw <8 x i32> %i8, <i32 1, i32 1, i32 1, i32 1, i32 1, i32 1, i32 1, i32 1>
   store <8 x i32> %i9, <8 x i32>* %p2, align 4
   ret void
 }
 
+; Same as above, but 128-bit ops:
+; With AVX, we don't transform 'add' to 'sub' because that prevents load folding.
+; With SSE, we do it because we can't load fold the other op without overwriting the constant op.
+
 define void @PR52032_2(<4 x i32>* %p) {
 ; SSE-LABEL: PR52032_2:
 ; SSE:       # %bb.0:
 ; SSE-NEXT:    pcmpeqd %xmm0, %xmm0
 ; SSE-NEXT:    movdqu (%rdi), %xmm1
 ; SSE-NEXT:    movdqu 16(%rdi), %xmm2
 ; SSE-NEXT:    psubd %xmm0, %xmm1
 ; SSE-NEXT:    movdqu %xmm1, (%rdi)
 ; SSE-NEXT:    psubd %xmm0, %xmm2
 ; SSE-NEXT:    movdqu %xmm2, 16(%rdi)
 ; SSE-NEXT:    retq
 ;
 ; AVX-LABEL: PR52032_2:
 ; AVX:       # %bb.0:
-; AVX-NEXT:    vpcmpeqd %xmm0, %xmm0, %xmm0
-; AVX-NEXT:    vmovdqu (%rdi), %xmm1
-; AVX-NEXT:    vmovdqu 16(%rdi), %xmm2
-; AVX-NEXT:    vpsubd %xmm0, %xmm1, %xmm1
+; AVX-NEXT:    vpbroadcastd {{.*#+}} xmm0 = [1,1,1,1]
+; AVX-NEXT:    vpaddd (%rdi), %xmm0, %xmm1
 ; AVX-NEXT:    vmovdqu %xmm1, (%rdi)
-; AVX-NEXT:    vpsubd %xmm0, %xmm2, %xmm0
+; AVX-NEXT:    vpaddd 16(%rdi), %xmm0, %xmm0
 ; AVX-NEXT:    vmovdqu %xmm0, 16(%rdi)
 ; AVX-NEXT:    retq
   %i3 = load <4 x i32>, <4 x i32>* %p, align 4
   %i4 = add nsw <4 x i32> %i3, <i32 1, i32 1, i32 1, i32 1>
   store <4 x i32> %i4, <4 x i32>* %p, align 4
   %p2 = getelementptr inbounds <4 x i32>, <4 x i32>* %p, i64 1
   %i8 = load <4 x i32>, <4 x i32>* %p2, align 4
   %i9 = add nsw <4 x i32> %i8, <i32 1, i32 1, i32 1, i32 1>
   store <4 x i32> %i9, <4 x i32>* %p2, align 4
   ret void
 }
 
+; If we are starting with a 'sub', it is always better to do the transform.
+
 define void @PR52032_3(<4 x i32>* %p) {
 ; SSE-LABEL: PR52032_3:
 ; SSE:       # %bb.0:
 ; SSE-NEXT:    pcmpeqd %xmm0, %xmm0
 ; SSE-NEXT:    movdqu (%rdi), %xmm1
 ; SSE-NEXT:    movdqu 16(%rdi), %xmm2
 ; SSE-NEXT:    paddd %xmm0, %xmm1
 ; SSE-NEXT:    movdqu %xmm1, (%rdi)
; AVX-NEXT:    retq
   store <4 x i32> %i4, <4 x i32>* %p, align 4
   %p2 = getelementptr inbounds <4 x i32>, <4 x i32>* %p, i64 1
   %i8 = load <4 x i32>, <4 x i32>* %p2, align 4
   %i9 = sub nsw <4 x i32> %i8, <i32 1, i32 1, i32 1, i32 1>
   store <4 x i32> %i9, <4 x i32>* %p2, align 4
   ret void
 }
 
+; If there's no chance of profitable load folding (because of extra uses), we convert 'add' to 'sub'.
+
 define void @PR52032_4(<4 x i32>* %p, <4 x i32>* %q) {
 ; SSE-LABEL: PR52032_4:
 ; SSE:       # %bb.0:
 ; SSE-NEXT:    movdqu (%rdi), %xmm0
 ; SSE-NEXT:    movdqa %xmm0, (%rsi)
 ; SSE-NEXT:    pcmpeqd %xmm1, %xmm1
 ; SSE-NEXT:    psubd %xmm1, %xmm0
 ; SSE-NEXT:    movdqu %xmm0, (%rdi)