[SystemZ] Improve foldMemoryOperandImpl().

[jonpa]

Summary

Swap the compare operands if LHS is spilled while updating the CCMask:s of the CC users.

This is relatively straight forward if the live-in lists for non-allocatable registers (CC) are assumed to be correct during register allocation. The experimental verifyLiveInLists_CC() has not detected any missing CC in any live-in lists so far, but this is still a missing piece to add if this is found useful enough.

On SPEC 2006, I see:

cg             :                10184                10861     +677
lg             :               373873               373204     -669
je             :               118876               119381     +505
cgrje          :                19726                19301     -425
c              :                17535                17941     +406
l              :                73704                73319     -385
jlh            :                61456                61810     +354
cgrjlh         :                11370                11189     -181
crjlh          :                 9502                 9353     -149
crje           :                 6488                 6406      -82
jle            :                12939                13000      +61
crjhe          :                 3843                 3787      -56
cgr            :                 1805                 1754      -51
crjle          :                 2508                 2462      -46
jhe            :                10908                10953      +45
lr             :                27011                26967      -44

I interpret this to mean that this patch handles some additional ~1000 cases. This seems to mean that the compare is now folded with the load load instead of with the jump, like 'lg; cgrje' => 'cg; je'. I am not sure this is really better, then? Would it be worth the trouble of adding some kind of live-in lists check before regalloc for non-allocatable phys reg(s)?

[uweigand]

I interpret this to mean that this patch handles some additional ~1000 cases. This seems to mean that the compare is now folded with the load load instead of with the jump, like 'lg; cgrje' => 'cg; je'. I am not sure this is really better, then? Would it be worth the trouble of adding some kind of live-in lists check before regalloc for non-allocatable phys reg(s)?

In general, 'lg; cgrje' should have the same performance as 'cg; je' (both have one single-issue and one dual-issue opcode on current micro-archs). However, the 'cg; je' version does not require a GPR to hold the value to be compared against, so it may be preferable due to register pressure concerns.

I'm not sure about the isCCLiveOut. In SystemZElimCompare we simply rely on this being correct. Why can't we rely on it here as well? Can we find out the specific rules that specify when the isLiveIn flags can be relied upon and when they cannot?

lib/Target/SystemZ/SystemZInstrInfo.cpp

1840 ↗

(On Diff #219689)

I'm not sure it is safe to assert here. If there's no CC users of a compare, then the compare is dead and will usually have been optimized away. But I'm not sure we can fully rely on that to have happened in all cases (e.g. what about -O0?).

1849 ↗

(On Diff #219689)

This logic duplicates the CC commutation operation done in SystemZISelLowering::combineCCMask, so it would be preferable to have a helper routine somewhere.

1871 ↗

(On Diff #219689)

Why do you need to modify the instruction here in the first place? The caller throws it away anyway and creates the memory variant. Should this just do the commutation in place there?

lib/Target/SystemZ/SystemZRegisterInfo.cpp

81 ↗

(On Diff #219689)

If we move it here, the copy in SystemZElimCompare is now no longer needed, right?

[jonpa]

In general, 'lg; cgrje' should have the same performance as 'cg; je' (both have one single-issue and one dual-issue opcode on current micro-archs). However, the 'cg; je' version does not require a GPR to hold the value to be compared against, so it may be preferable due to register pressure concerns.

That's a good point. I see "Spill/Reload" comments in output go up with 4, while Copies go -28. (206 files different). On SPEC 2017, I see ~2600 cases, with "Spill/Reload" comments in output go up with 127, and Copies go -71. (371 files different).

I don't know exactly why there is an *increase* in spill/reload instructions and a decrease in register move instructions, but it anyways seem to be from this viewpoint not that interesting of a change, or?

I'm not sure about the isCCLiveOut. In SystemZElimCompare we simply rely on this being correct. Why can't we rely on it here as well? Can we find out the specific rules that specify when the isLiveIn flags can be relied upon and when they cannot?

When I asked about this on the list in June, the answer is that the live-in lists "...were intended only for post-ra passes and are only setup during the final rewriting pass.". So it seems there is *not* some kind of general agreement that live-in lists need to be correct at all times pre-/during RA. However, that seems to refer to VirtRegMap adding the allocated physical registers to the live-in lists, while CC is non-allocatable.

I tried a simple .mir program where I made CC used by a branch in a basic block where the definition (compare) was in the predecessor block. The MachineVerifier caught this as a "Bad machine code: Using an undefined physical register". If I add the CC to the live-in list, it passes the initial verification. So in a way the MachineVerifier has at least in this example demanded CC to either be defined in the basic block or be in the live-in list at the point of use.

SystemZElimCompare is after regalloc, but I can see that trusting live-in lists there or anywhere else seems to have the same implications. Maybe I was thinking that regalloc would set up all the live in lists correctly after its analysis of the function.

I am still not sure if I should go ahead and spend more time on this, given the very small change in spill/copies..?

[jonpa]

Also recognize CLR and CLGR instructions which also can be commuted in order to fold the spilled operand. Compare logical register sets CC the same way as signed comparisons, so the trySwapCompareOperands() should work as is. This folds another ~200 reloads on benchmarks.

Also, a closer look at the issue with the (slight) increased number of spill/reload instructions in the output shows that MachineBlockPlacement is responsible for this difference since it will not duplicate the now bigger code snippets as willingly as when the load is folded. With -disable-block-placement the total difference over benchmarks is now negative, as expected.

However, a potential deficiency in the register allocator has also been reported (https://bugs.llvm.org/show_bug.cgi?id=43405). This was a test case where a folded compare operand caused a difference in the remainder of the allocation (the reload snippet no longer there to allocate), which ended up with adding 2 extra reloads. Hopefully this is something that can be improved in the register allocator.

[jonpa]

Patch rebased and using LivePhysRegs. New tests added.

I thought about using a CutOff counter in the search for CC Users (a value of 50 is NFC on SPEC 2006). But then I thought that maybe this isn't really needed since many instructions define CC at which point the search stops...

I also realized that we could do WFCDB -> CDBR -> CDB, WFCSB -> CEBR -> CEB if the non-spilled reg is allocated to FP bank (and possibly also constrain a non-allocated reg to FP reg). This compare could also be swapped since the CC=3 for FP compares would not have to be handled, right? I am thinking this could wait until this patch has been committed, but not sure if it would be better to do both at the same time and then run benchmarks just one time...

The instruction mix difference is about the same as before on SPEC 2006, and ~230 files are affected.

[uweigand]

In D67437#1734010, @jonpa wrote:

Patch rebased and using LivePhysRegs. New tests added.

I thought about using a CutOff counter in the search for CC Users (a value of 50 is NFC on SPEC 2006). But then I thought that maybe this isn't really needed since many instructions define CC at which point the search stops...

Agreed.

I also realized that we could do WFCDB -> CDBR -> CDB, WFCSB -> CEBR -> CEB if the non-spilled reg is allocated to FP bank (and possibly also constrain a non-allocated reg to FP reg). This compare could also be swapped since the CC=3 for FP compares would not have to be handled, right? I am thinking this could wait until this patch has been committed, but not sure if it would be better to do both at the same time and then run benchmarks just one time...

We definitely can swap FP compares. Converting WFCDB to FP is a bit of a trade-off since you need to restrict the register bank ... that probably needs more performance verification. I agree extending this to FP can be done in a separate patch.

Re-added some inline comments that seem to have been lost ...

llvm/lib/Target/SystemZ/SystemZInstrInfo.cpp

1735

I believe we do not need this check if the loop below finds another CC def after MI. We only need to check live-outs if this MI is the last CC def in the basic block.

1750

I'm not sure it is safe to assert here. If there's no CC users of a compare, then the compare is dead and will usually have been optimized away. But I'm not sure we can fully rely on that to have happened in all cases (e.g. what about -O0?).

1758

This logic duplicates the CC commutation operation done in SystemZISelLowering::combineCCMask, so it would be preferable to have a helper routine somewhere.

1781

Why do you need to modify the instruction here in the first place? The caller throws it away anyway and creates the memory variant. Should this just do the commutation in place there?

[jonpa]

Thanks for review - patch updated per suggestions.

llvm/lib/Target/SystemZ/SystemZInstrInfo.cpp

1735

done

1750

OK

1758

ok: getSwappedCCMask()

1781

The high-muxes patch does the exact same thing in PostRewrite in expandCmpMux() (to handle the "low-high" case), so I thought it might be a nice function to have. But I can see that it makes things easier here to instead use NeedsCommute as then we don't need to swap the operands anymore like you suggest.

[uweigand]

See minor comment inline. Otherwise, this now looks good to me, but we should check performance results before checking it in ...

llvm/lib/Target/SystemZ/SystemZInstrInfo.cpp

1769

This does not at all depend on a SystemZInstrInfo object, so it should not require callers to pass one in. Could be either made a static member function, or maybe just a non-class function (probably in the SystemZ namespace).

[jonpa]

getSwappedCCMask() function moved out into the SystemZ namespace.

llvm/lib/Target/SystemZ/SystemZInstrInfo.cpp

1769

Ah, right...

[jonpa]

Patch extended to also fold reloads into conditional load operands (both LOCR and SELR).

The mapping from LOC(G)R to LOC(G) gives ~ 250 folded instructions on SPEC 2006.

New MemFoldPseudo instructions for LOCG and LOCMux, so that the SELGR/SELRMux can be mapped to them, and later handled in SystemZPostRewrite as was already done previously for e.g. AG. This gives ~100 folded cases becoming LOC on SPEC 2006 on z15. There were no cases of COPYs insterted in SystemZPostRewrite due to this on the given day on SPEC 2006.

There are two new MemFoldPseudo opcodes that are not used since we always use LOCRMux : LOCFH_MemFoldPseudo and LOC_MemFoldPseudo.

These mappings are superfluous in getMemOpcode():

LOCFHR -> LOCFH 
LOCR   -> LOC        
SELR   -> LOC_MemFoldPseudo

in getTargetMemOpcode():

LOCFH_MemFoldPseudo -> LOCFH  
LOC_MemFoldPseudo   -> LOC

New test case for folding reload into LOCG/LOCMux both on z14 and z15.

Perhaps just reuse reverseCCMask() in SystemZISelLowering.cpp instead of the new SystemZ::getSwappedCCMask() ..?

[uweigand]

In D67437#1812901, @jonpa wrote:

Patch extended to also fold reloads into conditional load operands (both LOCR and SELR).

OK, makes sense.

There are two new MemFoldPseudo opcodes that are not used since we always use LOCRMux : LOCFH_MemFoldPseudo and LOC_MemFoldPseudo.

Then we really shouldn't generate them --- see inline comments.

Perhaps just reuse reverseCCMask() in SystemZISelLowering.cpp instead of the new SystemZ::getSwappedCCMask() ..?

Yes, agreed. See also inline comments.

llvm/lib/Target/SystemZ/SystemZInstrFormats.td

2846 ↗

(On Diff #237167)

I think it would be better to leave this here (to allow for selectively creating _MemFoldPseudo only when we need it). Also, for consistency, we should then set OpKey/OpType in CondUnaryRSY.

3231 ↗

(On Diff #237167)

Move OpKey and OpType into CondBinaryRRF.

3246 ↗

(On Diff #237167)

For consistency, also append cls1 to mnemonic here.

4776 ↗

(On Diff #237167)

Append cls.

4778 ↗

(On Diff #237167)

Append cls.

4823 ↗

(On Diff #237167)

Append cls1.

4838 ↗

(On Diff #237167)

Append cls1.

4876 ↗

(On Diff #237167)

Again, I think it would be better to leave this as-is, except for adding OpKey/OpType, and then adding another multiclass below that creates the MemFoldPseudo.

5097 ↗

(On Diff #237167)

This should have a different name that makes it clear that it creates a MemFold pseudo. Maybe "CondUnaryRSYPairAndMemFold" (ideally, we should rename the other MemFold pseudo multiclasses to match).

We shouldn't set OpKey/OpType here, but in CondUnaryRSY.

In fact, we may want to also move setting of MemKey/MemType (for MemType "target") into the original classes as well, then we don't need to duplicate stuff here.

5098 ↗

(On Diff #237167)

Also, we need a CondUnaryRSYPseudoAndMemFold analogously here.

llvm/lib/Target/SystemZ/SystemZInstrInfo.cpp

1770

You're right, this really should use the code in reverseCCMask -- then we also wouldn't have to deal with any failure cases.

llvm/lib/Target/SystemZ/SystemZInstrInfo.td

534 ↗

(On Diff #237167)

Should use ...AndMemFold exactly there where we need them.

Also, the "mnemonic" for mux pseudos should probably be something like "MUXloc", then can keep simply appending "r" without extra hassle.

[jonpa]

Patch updated per review (NFC). The new unused opcodes are gone, and the only superfluous mappings now added are in getMemOpcode():

LOCFHR -> LOCFH
LOCR   -> LOC

(which are not really "wrong")

Using reverseCCMask() includes the overflow bit being handled as well (and not checked for), but I suppose this must be equivalent since the compares never set the OF bit, right? (in combineCCMask and prepareCompareSwapOperands). It seems that we are with this patch using reverseCCMask() only for CCUsers in the presence of a compare. So to me it would make more sense to have an assert in reverseCCMask against the OF bit set, rather than handling it. This would be more overall readable given that we do introduce the use of the OF bit in SystemZElimCompare later on (or maybe I am missing something?). In other words, make it explit that the CCMask is produced by a compare and nothing else, like getSwappedCCMask() did. Would we want to use the OF (unordered bit) with FP compares?

Changed mnemonics per suggestion to MUX... This simplifies one line where just an "r" can be added, but the nested subst() in MemFoldPseudo_CondMove is not remedied. Can we still call this string 'mnemonic', or should it 'mnem' or something to show that it is not the same as the name of the instruction (on the other hand, it is already a pseudo opcode...)?

[uweigand]

In D67437#1815569, @jonpa wrote:

Patch updated per review (NFC). The new unused opcodes are gone, and the only superfluous mappings now added are in getMemOpcode():

LOCFHR -> LOCFH
LOCR   -> LOC

(which are not really "wrong")

Yes, I think this is fine.

Using reverseCCMask() includes the overflow bit being handled as well (and not checked for), but I suppose this must be equivalent since the compares never set the OF bit, right? (in combineCCMask and prepareCompareSwapOperands). It seems that we are with this patch using reverseCCMask() only for CCUsers in the presence of a compare. So to me it would make more sense to have an assert in reverseCCMask against the OF bit set, rather than handling it. This would be more overall readable given that we do introduce the use of the OF bit in SystemZElimCompare later on (or maybe I am missing something?). In other words, make it explit that the CCMask is produced by a compare and nothing else, like getSwappedCCMask() did. Would we want to use the OF (unordered bit) with FP compares?

Yes, we need the unordered bit handling for FP compares. For integer compares the bit is never set, so I think this is still fine ...

Changed mnemonics per suggestion to MUX... This simplifies one line where just an "r" can be added, but the nested subst() in MemFoldPseudo_CondMove is not remedied. Can we still call this string 'mnemonic', or should it 'mnem' or something to show that it is not the same as the name of the instruction (on the other hand, it is already a pseudo opcode...)?

See inline comment, I think this can now be simplified.

Except for the minor tweaks pointed on inline, this now LGTM. Thanks!

llvm/lib/Target/SystemZ/SystemZInstrFormats.td

4791 ↗

(On Diff #237504)

I believe this can now simply be

let OpKey = !subst("loc", "sel", mnemonic)#"r"#cls;

5106 ↗

(On Diff #237504)

You should now be able to simply use

def "" : CondUnaryRSYPair<...>

right?

[jonpa]

Minor updated per review.

[uweigand]

LGTM, thanks!

llvm/lib/Target/SystemZ/SystemZISelLowering.cpp

if (DAG.isConstantIntBuildVectorOrConstantInt(Op0) ||
       DCI.AddToWorklist(Op1.getNode());
       return DAG.getVectorShuffle(VecVT, SDLoc(N), Op0, Op1, SV->getMask());
     }
   }
 
   return SDValue();
 }
 
-static bool combineCCMask(SDValue &CCReg, int &CCValid, int &CCMask) {
+static bool combineCCMask(SDValue &CCReg, int &CCValid, int &CCMask,
+                          const SystemZInstrInfo *TII) {
   // We have a SELECT_CCMASK or BR_CCMASK comparing the condition code
   // set by the CCReg instruction using the CCValid / CCMask masks,
   // If the CCReg instruction is itself a ICMP testing the condition
   // code set by some other instruction, see whether we can directly
   // use that condition code.
 
   // Verify that we have an ICMP against some constant.
   if (CCValid != SystemZ::CCMASK_ICMP)
if (CompareLHS->getOpcode() == ISD::SRA) {
     // Avoid introducing CC spills (because SRA would clobber CC).
     if (!CompareLHS->hasOneUse())
       return false;
     // Verify that the ICMP compares against zero.
     if (CompareRHS->getZExtValue() != 0)
       return false;
 
     // Compute the effective CC mask for the new branch or select.
-    switch (CCMask) {
-    case SystemZ::CCMASK_CMP_EQ: break;
-    case SystemZ::CCMASK_CMP_NE: break;
-    case SystemZ::CCMASK_CMP_LT: CCMask = SystemZ::CCMASK_CMP_GT; break;
-    case SystemZ::CCMASK_CMP_GT: CCMask = SystemZ::CCMASK_CMP_LT; break;
-    case SystemZ::CCMASK_CMP_LE: CCMask = SystemZ::CCMASK_CMP_GE; break;
-    case SystemZ::CCMASK_CMP_GE: CCMask = SystemZ::CCMASK_CMP_LE; break;
-    default: return false;
-    }
+    int32_t NewCCMask = TII->getSwappedCCMask(CCMask);
+    if (NewCCMask == INT32_MIN)
+      return false; // Unrecognized CCMask
+    CCMask = NewCCMask;
 
     // Return the updated CCReg link.
     CCReg = IPM->getOperand(0);
     return true;
   }
 
   return false;
 }
 
 SDValue SystemZTargetLowering::combineBR_CCMASK(
     SDNode *N, DAGCombinerInfo &DCI) const {
   SelectionDAG &DAG = DCI.DAG;
+  const SystemZInstrInfo *TII =
+      static_cast<const SystemZInstrInfo *>(Subtarget.getInstrInfo());
 
   // Combine BR_CCMASK (ICMP (SELECT_CCMASK)) into a single BR_CCMASK.
   auto *CCValid = dyn_cast<ConstantSDNode>(N->getOperand(1));
   auto *CCMask = dyn_cast<ConstantSDNode>(N->getOperand(2));
   if (!CCValid || !CCMask)
     return SDValue();
 
   int CCValidVal = CCValid->getZExtValue();
   int CCMaskVal = CCMask->getZExtValue();
   SDValue Chain = N->getOperand(0);
   SDValue CCReg = N->getOperand(4);
 
-  if (combineCCMask(CCReg, CCValidVal, CCMaskVal))
+  if (combineCCMask(CCReg, CCValidVal, CCMaskVal, TII))
     return DAG.getNode(SystemZISD::BR_CCMASK, SDLoc(N), N->getValueType(0),
                        Chain,
                        DAG.getTargetConstant(CCValidVal, SDLoc(N), MVT::i32),
                        DAG.getTargetConstant(CCMaskVal, SDLoc(N), MVT::i32),
                        N->getOperand(3), CCReg);
   return SDValue();
 }
 
 SDValue SystemZTargetLowering::combineSELECT_CCMASK(
     SDNode *N, DAGCombinerInfo &DCI) const {
   SelectionDAG &DAG = DCI.DAG;
+  const SystemZInstrInfo *TII =
+      static_cast<const SystemZInstrInfo *>(Subtarget.getInstrInfo());
 
   // Combine SELECT_CCMASK (ICMP (SELECT_CCMASK)) into a single SELECT_CCMASK.
   auto *CCValid = dyn_cast<ConstantSDNode>(N->getOperand(2));
   auto *CCMask = dyn_cast<ConstantSDNode>(N->getOperand(3));
   if (!CCValid || !CCMask)
     return SDValue();
 
   int CCValidVal = CCValid->getZExtValue();
   int CCMaskVal = CCMask->getZExtValue();
   SDValue CCReg = N->getOperand(4);
 
-  if (combineCCMask(CCReg, CCValidVal, CCMaskVal))
+  if (combineCCMask(CCReg, CCValidVal, CCMaskVal, TII))
     return DAG.getNode(SystemZISD::SELECT_CCMASK, SDLoc(N), N->getValueType(0),
                        N->getOperand(0), N->getOperand(1),
                        DAG.getTargetConstant(CCValidVal, SDLoc(N), MVT::i32),
                        DAG.getTargetConstant(CCMaskVal, SDLoc(N), MVT::i32),
                        CCReg);
   return SDValue();
 }
 

llvm/lib/Target/SystemZ/SystemZInstrInfo.h

public:
 
   // If Opcode is a COMPARE opcode for which an associated fused COMPARE AND *
   // operation exists, return the opcode for the latter, otherwise return 0.
   // MI, if nonnull, is the compare instruction.
   unsigned getFusedCompare(unsigned Opcode,
                            SystemZII::FusedCompareType Type,
                            const MachineInstr *MI = nullptr) const;
 
+  // Try to find all CC users of the compare instruction (MBBI) and update
+  // all of them to maintain equivalent behavior after swapping the compare
+  // operands. Return false if not all users can be conclusively found and
+  // handled. The compare instruction is *not* changed.
+  bool prepareCompareSwapOperands(MachineBasicBlock::iterator MBBI) const;
+
+  // Return the CCMask to use instead of InCCmask after swapping operands of
+  // a compare.
+  int32_t getSwappedCCMask(int32_t InCCMask) const;
+
   // If Opcode is a LOAD opcode for with an associated LOAD AND TRAP
   // operation exists, returh the opcode for the latter, otherwise return 0.
   unsigned getLoadAndTrap(unsigned Opcode) const;
 
   // Emit code before MBBI in MI to move immediate value Value into
   // physical register Reg.
   void loadImmediate(MachineBasicBlock &MBB,
                      MachineBasicBlock::iterator MBBI,

llvm/lib/Target/SystemZ/SystemZInstrInfo.cpp

if (MMO->getSize() == Size && !MMO->isVolatile() && !MMO->isAtomic()) {
       }
     }
   }
 
   // If the spilled operand is the final one or the instruction is
   // commutable, try to change <INSN>R into <INSN>.
   unsigned NumOps = MI.getNumExplicitOperands();
   int MemOpcode = SystemZ::getMemOpcode(Opcode);
+  if (MemOpcode == -1)
+    return nullptr;
+
+  // Try to swap compare operands if possible.
+  bool NeedsCommute = false;
+  if ((MI.getOpcode() == SystemZ::CR || MI.getOpcode() == SystemZ::CGR ||
+       MI.getOpcode() == SystemZ::CLR || MI.getOpcode() == SystemZ::CLGR) &&
+      OpNum == 0 && prepareCompareSwapOperands(MI))
+    NeedsCommute = true;
 
   // See if this is a 3-address instruction that is convertible to 2-address
   // and suitable for folding below.  Only try this with virtual registers
   // and a provided VRM (during regalloc).
-  bool NeedsCommute = false;
-  if (SystemZ::getTwoOperandOpcode(Opcode) != -1 && MemOpcode != -1) {
+  if (SystemZ::getTwoOperandOpcode(Opcode) != -1) {
     if (VRM == nullptr)
-      MemOpcode = -1;
+      return nullptr;
     else {
       assert(NumOps == 3 && "Expected two source registers.");
       Register DstReg = MI.getOperand(0).getReg();
       Register DstPhys =
           (Register::isVirtualRegister(DstReg) ? VRM->getPhys(DstReg) : DstReg);
       Register SrcReg = (OpNum == 2 ? MI.getOperand(1).getReg()
                                     : ((OpNum == 1 && MI.isCommutable())
                                            ? MI.getOperand(2).getReg()
                                          : Register()));
       if (DstPhys && !SystemZ::GRH32BitRegClass.contains(DstPhys) && SrcReg &&
           Register::isVirtualRegister(SrcReg) &&
           DstPhys == VRM->getPhys(SrcReg))
         NeedsCommute = (OpNum == 1);
       else
-        MemOpcode = -1;
+        return nullptr;
     }
   }
 
-  if (MemOpcode >= 0) {
-    if ((OpNum == NumOps - 1) || NeedsCommute) {
-      const MCInstrDesc &MemDesc = get(MemOpcode);
-      uint64_t AccessBytes = SystemZII::getAccessSize(MemDesc.TSFlags);
-      assert(AccessBytes != 0 && "Size of access should be known");
-      assert(AccessBytes <= Size && "Access outside the frame index");
-      uint64_t Offset = Size - AccessBytes;
-      MachineInstrBuilder MIB = BuildMI(*InsertPt->getParent(), InsertPt,
-                                        MI.getDebugLoc(), get(MemOpcode));
+    if (MI.isCompare()) {
+      assert(NumOps == 2 && "Expected 2 register operands for a compare.");
+      MIB.add(MI.getOperand(NeedsCommute ? 1 : 0));
+    }
+    else {
       MIB.add(MI.getOperand(0));
       if (NeedsCommute)
         MIB.add(MI.getOperand(2));
       else
         for (unsigned I = 1; I < OpNum; ++I)
           MIB.add(MI.getOperand(I));
-      MIB.addFrameIndex(FrameIndex).addImm(Offset);
-      if (MemDesc.TSFlags & SystemZII::HasIndex)
-        MIB.addReg(0);
-      transferDeadCC(&MI, MIB);
-      return MIB;
-    }
-  }
+    }
 
   return nullptr;
 }
 
 MachineInstr *SystemZInstrInfo::foldMemoryOperandImpl(
     MachineFunction &MF, MachineInstr &MI, ArrayRef<unsigned> Ops,
     MachineBasicBlock::iterator InsertPt, MachineInstr &LoadMI,
     LiveIntervals *LIS) const {
case SystemZ::CLG:
       return SystemZ::CLGT;
     default:
       return 0;
     }
   }
   return 0;
 }
 
+bool SystemZInstrInfo::
+prepareCompareSwapOperands(MachineBasicBlock::iterator const MBBI) const {
+  assert(MBBI->isCompare() && MBBI->getOperand(0).isReg() &&
+         MBBI->getOperand(1).isReg() && !MBBI->mayLoad() &&
+         "Not a compare reg/reg.");
+
+  MachineBasicBlock *MBB = MBBI->getParent();
+  bool CCLive = true;
+  SmallVector<MachineInstr *, 4> CCUsers;
+  for (MachineBasicBlock::iterator Itr = std::next(MBBI);
+       Itr != MBB->end(); ++Itr) {
+    if (Itr->readsRegister(SystemZ::CC)) {
+      unsigned Flags = Itr->getDesc().TSFlags;
+      if ((Flags & SystemZII::CCMaskFirst) || (Flags & SystemZII::CCMaskLast))
+        CCUsers.push_back(&*Itr);
+      else
+        return false;
+    }
+    if (Itr->definesRegister(SystemZ::CC)) {
+      CCLive = false;
+      break;
+    }
+  }
+  if (CCLive) {
+    LivePhysRegs LiveRegs(*MBB->getParent()->getSubtarget().getRegisterInfo());
+    LiveRegs.addLiveOuts(*MBB);
+    if (LiveRegs.contains(SystemZ::CC))
+      return false;
+  }
+
+  // Update all CC users.
+  for (unsigned Idx = 0; Idx < CCUsers.size(); ++Idx) {
+    unsigned Flags = CCUsers[Idx]->getDesc().TSFlags;
+    unsigned FirstOpNum = ((Flags & SystemZII::CCMaskFirst) ?
+                           0 : CCUsers[Idx]->getNumExplicitOperands() - 2);
+    MachineOperand &CCMaskMO = CCUsers[Idx]->getOperand(FirstOpNum + 1);
+    int32_t NewCCMask = getSwappedCCMask(CCMaskMO.getImm());
+    assert(NewCCMask != INT32_MIN && "Unexpected CCMask value.");
+    CCMaskMO.setImm(NewCCMask);
+  }
+
+  return true;
+}
+
+int32_t SystemZInstrInfo::getSwappedCCMask(int32_t InCCMask) const {
+  switch(InCCMask) {
+  case SystemZ::CCMASK_CMP_LT:    return SystemZ::CCMASK_CMP_GT;
+  case SystemZ::CCMASK_CMP_GT:    return SystemZ::CCMASK_CMP_LT;
+  case SystemZ::CCMASK_CMP_LE:    return SystemZ::CCMASK_CMP_GE;
+  case SystemZ::CCMASK_CMP_GE:    return SystemZ::CCMASK_CMP_LE;
+  case SystemZ::CCMASK_CMP_EQ:
+  case SystemZ::CCMASK_CMP_NE:    return InCCMask;
+  default: break;
+  }
+
+  return INT32_MIN;  // Unrecognized
+}
+
 unsigned SystemZInstrInfo::getLoadAndTrap(unsigned Opcode) const {
   if (!STI.hasLoadAndTrap())
     return 0;
   switch (Opcode) {
   case SystemZ::L:
   case SystemZ::LY:
     return SystemZ::LAT;
   case SystemZ::LG:

llvm/test/CodeGen/SystemZ/int-cmp-56.mir

+# RUN: llc -mtriple=s390x-linux-gnu -mcpu=z14 -run-pass greedy %s -o - \
+# RUN:   | FileCheck %s
+#
+# Test that a reload can be folded into a compare instruction after swapping
+# operands (when the LHS register is spilled).
+
+--- |
+  declare i64 @foo()
+  define i64 @fun1(i64* %ptr0)  { ret i64 0 }
+  define i64 @fun2(i64* %ptr0)  { ret i64 0 }
+
+  declare i32 @foo32()
+  define i32 @fun3(i32* %ptr0)  { ret i32 0 }
+  define i32 @fun4(i32* %ptr0)  { ret i32 0 }
+...
+
+
+# Test CGR -> CG
+# CHECK: name:            fun1
+# CHECK: CG %10, %stack.0, 0, $noreg, implicit-def $cc :: (load 8 from %stack.0)
+# CHECK-NEXT: %12:gr64bit = LOCGHI %12, 8, 14, 12, implicit killed $cc
+---
+name:            fun1
+alignment:       16
+tracksRegLiveness: true
+registers:
+  - { id: 0, class: addr64bit }
+  - { id: 1, class: gr64bit }
+  - { id: 2, class: gr64bit }
+  - { id: 3, class: gr64bit }
+  - { id: 4, class: gr64bit }
+  - { id: 5, class: gr64bit }
+  - { id: 6, class: gr64bit }
+  - { id: 7, class: gr64bit }
+  - { id: 8, class: gr64bit }
+  - { id: 9, class: gr64bit }
+  - { id: 10, class: gr64bit }
+  - { id: 11, class: gr64bit }
+  - { id: 12, class: gr64bit }
+  - { id: 13, class: gr64bit }
+  - { id: 14, class: gr64bit }
+  - { id: 15, class: gr64bit }
+  - { id: 16, class: gr64bit }
+  - { id: 17, class: gr64bit }
+  - { id: 18, class: gr64bit }
+  - { id: 19, class: gr64bit }
+liveins:
+  - { reg: '$r2d', virtual-reg: '%0' }
+frameInfo:
+  maxAlignment:    1
+  hasCalls:        true
+machineFunctionInfo: {}
+body:             |
+  bb.0:
+    liveins: $r2d
+  
+    %0:addr64bit = COPY $r2d
+    %1:gr64bit = LG %0, 0, $noreg
+    %2:gr64bit = LG %0, 16, $noreg
+    %3:gr64bit = LG %0, 32, $noreg
+    %4:gr64bit = LG %0, 48, $noreg
+    %5:gr64bit = LG %0, 64, $noreg
+    %6:gr64bit = LG %0, 80, $noreg
+    %7:gr64bit = LG %0, 96, $noreg
+    %8:gr64bit = LG %0, 112, $noreg
+    %9:gr64bit = LG %0, 128, $noreg
+    ADJCALLSTACKDOWN 0, 0
+    CallBRASL @foo, csr_systemz, implicit-def dead $r14d, implicit-def dead $cc, implicit $fpc, implicit-def $r2d
+    %10:gr64bit = COPY $r2d
+    ADJCALLSTACKUP 0, 0
+    CGR %10, %1, implicit-def $cc
+    %12:gr64bit = COPY %10
+    %12:gr64bit = LOCGHI %12, 0, 14, 10, implicit killed $cc
+    CGR %10, %2, implicit-def $cc
+    %12:gr64bit = LOCGHI %12, 1, 14, 10, implicit killed $cc
+    CGR %10, %3, implicit-def $cc
+    %12:gr64bit = LOCGHI %12, 2, 14, 10, implicit killed $cc
+    CGR %10, %4, implicit-def $cc
+    %12:gr64bit = LOCGHI %12, 3, 14, 10, implicit killed $cc
+    CGR %10, %5, implicit-def $cc
+    %12:gr64bit = LOCGHI %12, 4, 14, 10, implicit killed $cc
+    CGR %10, %6, implicit-def $cc
+    %12:gr64bit = LOCGHI %12, 5, 14, 10, implicit killed $cc
+    CGR %10, %7, implicit-def $cc
+    %12:gr64bit = LOCGHI %12, 6, 14, 10, implicit killed $cc
+    CGR %10, %8, implicit-def $cc
+    %12:gr64bit = LOCGHI %12, 7, 14, 10, implicit killed $cc
+    CGR %9, %10, implicit-def $cc
+    %12:gr64bit = LOCGHI %12, 8, 14, 10, implicit killed $cc
+    $r2d = COPY %12
+    Return implicit $r2d
+...
+
+
+# Test CLGR -> CLG
+# CHECK: name:            fun2
+# CHECK: CLG %10, %stack.0, 0, $noreg, implicit-def $cc :: (load 8 from %stack.0)
+# CHECK-NEXT: %12:gr64bit = LOCGHI %12, 8, 14, 12, implicit killed $cc
+---
+name:            fun2
+alignment:       16
+tracksRegLiveness: true
+registers:
+  - { id: 0, class: addr64bit }
+  - { id: 1, class: gr64bit }
+  - { id: 2, class: gr64bit }
+  - { id: 3, class: gr64bit }
+  - { id: 4, class: gr64bit }
+  - { id: 5, class: gr64bit }
+  - { id: 6, class: gr64bit }
+  - { id: 7, class: gr64bit }
+  - { id: 8, class: gr64bit }
+  - { id: 9, class: gr64bit }
+  - { id: 10, class: gr64bit }
+  - { id: 11, class: gr64bit }
+  - { id: 12, class: gr64bit }
+  - { id: 13, class: gr64bit }
+  - { id: 14, class: gr64bit }
+  - { id: 15, class: gr64bit }
+  - { id: 16, class: gr64bit }
+  - { id: 17, class: gr64bit }
+  - { id: 18, class: gr64bit }
+  - { id: 19, class: gr64bit }
+liveins:
+  - { reg: '$r2d', virtual-reg: '%0' }
+frameInfo:
+  maxAlignment:    1
+  hasCalls:        true
+machineFunctionInfo: {}
+body:             |
+  bb.0:
+    liveins: $r2d
+  
+    %0:addr64bit = COPY $r2d
+    %1:gr64bit = LG %0, 0, $noreg
+    %2:gr64bit = LG %0, 16, $noreg
+    %3:gr64bit = LG %0, 32, $noreg
+    %4:gr64bit = LG %0, 48, $noreg
+    %5:gr64bit = LG %0, 64, $noreg
+    %6:gr64bit = LG %0, 80, $noreg
+    %7:gr64bit = LG %0, 96, $noreg
+    %8:gr64bit = LG %0, 112, $noreg
+    %9:gr64bit = LG %0, 128, $noreg
+    ADJCALLSTACKDOWN 0, 0
+    CallBRASL @foo, csr_systemz, implicit-def dead $r14d, implicit-def dead $cc, implicit $fpc, implicit-def $r2d
+    %10:gr64bit = COPY $r2d
+    ADJCALLSTACKUP 0, 0
+    CGR %10, %1, implicit-def $cc
+    %12:gr64bit = COPY %10
+    %12:gr64bit = LOCGHI %12, 0, 14, 10, implicit killed $cc
+    CGR %10, %2, implicit-def $cc
+    %12:gr64bit = LOCGHI %12, 1, 14, 10, implicit killed $cc
+    CGR %10, %3, implicit-def $cc
+    %12:gr64bit = LOCGHI %12, 2, 14, 10, implicit killed $cc
+    CGR %10, %4, implicit-def $cc
+    %12:gr64bit = LOCGHI %12, 3, 14, 10, implicit killed $cc
+    CGR %10, %5, implicit-def $cc
+    %12:gr64bit = LOCGHI %12, 4, 14, 10, implicit killed $cc
+    CGR %10, %6, implicit-def $cc
+    %12:gr64bit = LOCGHI %12, 5, 14, 10, implicit killed $cc
+    CGR %10, %7, implicit-def $cc
+    %12:gr64bit = LOCGHI %12, 6, 14, 10, implicit killed $cc
+    CGR %10, %8, implicit-def $cc
+    %12:gr64bit = LOCGHI %12, 7, 14, 10, implicit killed $cc
+    CLGR %9, %10, implicit-def $cc
+    %12:gr64bit = LOCGHI %12, 8, 14, 10, implicit killed $cc
+    $r2d = COPY %12
+    Return implicit $r2d
+...
+
+
+# Test CR -> C
+# CHECK: name:            fun3
+# CHECK: C %10, %stack.0, 0, $noreg, implicit-def $cc :: (load 4 from %stack.0)
+# CHECK: %12:gr32bit = LOCHIMux %12, 8, 14, 12, implicit killed $cc
+---
+name:            fun3
+alignment:       16
+tracksRegLiveness: true
+registers:
+  - { id: 0, class: addr64bit }
+  - { id: 1, class: gr32bit }
+  - { id: 2, class: gr32bit }
+  - { id: 3, class: gr32bit }
+  - { id: 4, class: gr32bit }
+  - { id: 5, class: gr32bit }
+  - { id: 6, class: gr32bit }
+  - { id: 7, class: gr32bit }
+  - { id: 8, class: gr32bit }
+  - { id: 9, class: gr32bit }
+  - { id: 10, class: gr32bit }
+  - { id: 11, class: gr32bit }
+  - { id: 12, class: gr32bit }
+  - { id: 13, class: gr32bit }
+  - { id: 14, class: gr32bit }
+  - { id: 15, class: gr32bit }
+  - { id: 16, class: gr32bit }
+  - { id: 17, class: gr32bit }
+  - { id: 18, class: gr32bit }
+  - { id: 19, class: gr32bit }
+liveins:
+  - { reg: '$r2d', virtual-reg: '%0' }
+frameInfo:
+  maxAlignment:    1
+  hasCalls:        true
+machineFunctionInfo: {}
+body:             |
+  bb.0:
+    liveins: $r2d
+  
+    %0:addr64bit = COPY $r2d
+    %1:gr32bit = LMux %0, 0, $noreg
+    %2:gr32bit = LMux %0, 8, $noreg
+    %3:gr32bit = LMux %0, 16, $noreg
+    %4:gr32bit = LMux %0, 24, $noreg
+    %5:gr32bit = LMux %0, 32, $noreg
+    %6:gr32bit = LMux %0, 40, $noreg
+    %7:gr32bit = LMux %0, 48, $noreg
+    %8:gr32bit = LMux %0, 56, $noreg
+    %9:gr32bit = LMux %0, 64, $noreg
+    ADJCALLSTACKDOWN 0, 0
+    CallBRASL @foo, csr_systemz, implicit-def dead $r14d, implicit-def dead $cc, implicit $fpc, implicit-def $r2l
+    %10:gr32bit = COPY $r2l
+    ADJCALLSTACKUP 0, 0
+    CR %10, %1, implicit-def $cc
+    %12:gr32bit = COPY %10
+    %12:gr32bit = LOCHIMux %12, 0, 14, 10, implicit killed $cc
+    CR %10, %2, implicit-def $cc
+    %12:gr32bit = LOCHIMux %12, 1, 14, 10, implicit killed $cc
+    CR %10, %3, implicit-def $cc
+    %12:gr32bit = LOCHIMux %12, 2, 14, 10, implicit killed $cc
+    CR %10, %4, implicit-def $cc
+    %12:gr32bit = LOCHIMux %12, 3, 14, 10, implicit killed $cc
+    CR %10, %5, implicit-def $cc
+    %12:gr32bit = LOCHIMux %12, 4, 14, 10, implicit killed $cc
+    CR %10, %6, implicit-def $cc
+    %12:gr32bit = LOCHIMux %12, 5, 14, 10, implicit killed $cc
+    CR %10, %7, implicit-def $cc
+    %12:gr32bit = LOCHIMux %12, 6, 14, 10, implicit killed $cc
+    CR %10, %8, implicit-def $cc
+    %12:gr32bit = LOCHIMux %12, 7, 14, 10, implicit killed $cc
+    CR %9, %10, implicit-def $cc
+    %12:gr32bit = LOCHIMux %12, 8, 14, 10, implicit killed $cc
+    $r2l = COPY %12
+    Return implicit $r2l
+...
+
+
+# Test CLR -> CL
+# CHECK: name:            fun4
+# CHECK: CL %10, %stack.0, 0, $noreg, implicit-def $cc :: (load 4 from %stack.0)
+# CHECK: %12:gr32bit = LOCHIMux %12, 8, 14, 12, implicit killed $cc
+---
+name:            fun4
+alignment:       16
+tracksRegLiveness: true
+registers:
+  - { id: 0, class: addr64bit }
+  - { id: 1, class: gr32bit }
+  - { id: 2, class: gr32bit }
+  - { id: 3, class: gr32bit }
+  - { id: 4, class: gr32bit }
+  - { id: 5, class: gr32bit }
+  - { id: 6, class: gr32bit }
+  - { id: 7, class: gr32bit }
+  - { id: 8, class: gr32bit }
+  - { id: 9, class: gr32bit }
+  - { id: 10, class: gr32bit }
+  - { id: 11, class: gr32bit }
+  - { id: 12, class: gr32bit }
+  - { id: 13, class: gr32bit }
+  - { id: 14, class: gr32bit }
+  - { id: 15, class: gr32bit }
+  - { id: 16, class: gr32bit }
+  - { id: 17, class: gr32bit }
+  - { id: 18, class: gr32bit }
+  - { id: 19, class: gr32bit }
+liveins:
+  - { reg: '$r2d', virtual-reg: '%0' }
+frameInfo:
+  maxAlignment:    1
+  hasCalls:        true
+machineFunctionInfo: {}
+body:             |
+  bb.0:
+    liveins: $r2d
+  
+    %0:addr64bit = COPY $r2d
+    %1:gr32bit = LMux %0, 0, $noreg
+    %2:gr32bit = LMux %0, 8, $noreg
+    %3:gr32bit = LMux %0, 16, $noreg
+    %4:gr32bit = LMux %0, 24, $noreg
+    %5:gr32bit = LMux %0, 32, $noreg
+    %6:gr32bit = LMux %0, 40, $noreg
+    %7:gr32bit = LMux %0, 48, $noreg
+    %8:gr32bit = LMux %0, 56, $noreg
+    %9:gr32bit = LMux %0, 64, $noreg
+    ADJCALLSTACKDOWN 0, 0
+    CallBRASL @foo, csr_systemz, implicit-def dead $r14d, implicit-def dead $cc, implicit $fpc, implicit-def $r2l
+    %10:gr32bit = COPY $r2l
+    ADJCALLSTACKUP 0, 0
+    CR %10, %1, implicit-def $cc
+    %12:gr32bit = COPY %10
+    %12:gr32bit = LOCHIMux %12, 0, 14, 10, implicit killed $cc
+    CR %10, %2, implicit-def $cc
+    %12:gr32bit = LOCHIMux %12, 1, 14, 10, implicit killed $cc
+    CR %10, %3, implicit-def $cc
+    %12:gr32bit = LOCHIMux %12, 2, 14, 10, implicit killed $cc
+    CR %10, %4, implicit-def $cc
+    %12:gr32bit = LOCHIMux %12, 3, 14, 10, implicit killed $cc
+    CR %10, %5, implicit-def $cc
+    %12:gr32bit = LOCHIMux %12, 4, 14, 10, implicit killed $cc
+    CR %10, %6, implicit-def $cc
+    %12:gr32bit = LOCHIMux %12, 5, 14, 10, implicit killed $cc
+    CR %10, %7, implicit-def $cc
+    %12:gr32bit = LOCHIMux %12, 6, 14, 10, implicit killed $cc
+    CR %10, %8, implicit-def $cc
+    %12:gr32bit = LOCHIMux %12, 7, 14, 10, implicit killed $cc
+    CLR %9, %10, implicit-def $cc
+    %12:gr32bit = LOCHIMux %12, 8, 14, 10, implicit killed $cc
+    $r2l = COPY %12
+    Return implicit $r2l
+...