[SystemZ] Build TM from scratch in convertToLoadAndTest, to get CC operand in right place.

[jonpa]

Summary

This is needed since the SchedModel defines a list of write latencies, which is to be used by mapping to the MI operand order. Therefore the CC must not end up last in the list in the cases where RegAlloc adds an extra implicit-def operand.

In order to build a new MI, the iteration method in optimizeCompareZero() had to be fixed so that the iterator does not get invalidated as the old MI is erased.

RegAlloc adds such operands for the SuperReg it seems, when SubRegIdx != 0. The other potentially relevant cases where setDesc is called and then the CC register is added would be in SystemZShortenInst::shortenOn001AddCC. Those instructions do however not seem to use subregs to begin with, so it seems they do not need to worry about regalloc operands.

At least the SPEC looks good with just the LT handling per this patch.

[uweigand]

Otherwise, this LGTM.

lib/Target/SystemZ/SystemZElimCompare.cpp

437

Shouldn't this be rend instead of instr_rend, since this is a reverse_iterator, not a reverse_instr_iterator?

[jonpa]

Shouldn't this be rend instead of instr_rend, since this is a reverse_iterator, not a reverse_instr_iterator?

Ah, yes. Thanks.

r334161

lib/Target/SystemZ/SystemZElimCompare.cpp

bool SystemZElimCompare::convertToLoadAndTest(
     MachineInstr &MI, MachineInstr &Compare,
     SmallVectorImpl<MachineInstr *> &CCUsers) {
 
   // Try to adjust CC masks for the LOAD AND TEST opcode that could replace MI.
   unsigned Opcode = TII->getLoadAndTest(MI.getOpcode());
   if (!Opcode || !adjustCCMasksForInstr(MI, Compare, CCUsers, Opcode))
     return false;
 
-  MI.setDesc(TII->get(Opcode));
-  MachineInstrBuilder(*MI.getParent()->getParent(), MI)
-      .addReg(SystemZ::CC, RegState::ImplicitDefine);
+  // Rebuild to get the CC operand in the right place.
+  MachineInstr *BuiltMI =
+    BuildMI(*MI.getParent(), MI, MI.getDebugLoc(), TII->get(Opcode));
+  for (const auto &MO : MI.operands())
+    BuiltMI->addOperand(MO);
+  BuiltMI->setMemRefs(MI.memoperands_begin(), MI.memoperands_end());
+  MI.eraseFromParent();
+
   return true;
 }
 
 // The CC users in CCUsers are testing the result of a comparison of some
 // value X against zero and we know that any CC value produced by MI would
 // also reflect the value of X.  ConvOpc may be used to pass the transfomed
 // opcode MI will have if this succeeds.  Try to adjust CCUsers so that they
 // test the result of MI directly, returning true on success.  Leave

 bool SystemZElimCompare::optimizeCompareZero(
     MachineInstr &Compare, SmallVectorImpl<MachineInstr *> &CCUsers) {
   if (!isCompareZero(Compare))
     return false;
 
   // Search back for CC results that are based on the first operand.
   unsigned SrcReg = getCompareSourceReg(Compare);
   MachineBasicBlock &MBB = *Compare.getParent();
-  MachineBasicBlock::iterator MBBI = Compare, MBBE = MBB.begin();
   Reference CCRefs;
   Reference SrcRefs;
-  while (MBBI != MBBE) {
-    --MBBI;
-    MachineInstr &MI = *MBBI;
+  for (MachineBasicBlock::reverse_iterator MBBI =
+         std::next(MachineBasicBlock::reverse_iterator(&Compare)),
+         MBBE = MBB.instr_rend(); MBBI != MBBE;) {
+    MachineInstr &MI = *MBBI++;
     if (resultTests(MI, SrcReg)) {
       // Try to remove both MI and Compare by converting a branch to BRCT(G).
       // or a load-and-trap instruction.  We don't care in this case whether
       // CC is modified between MI and Compare.
       if (!CCRefs.Use && !SrcRefs) {
         if (convertToBRCT(MI, Compare, CCUsers)) {
           BranchOnCounts += 1;
           return true;
for (MachineBasicBlock::reverse_iterator MBBI =
     CCRefs |= getRegReferences(MI, SystemZ::CC);
     if (CCRefs.Use && CCRefs.Def)
       break;
   }
 
   // Also do a forward search to handle cases where an instruction after the
   // compare can be converted, like
   // LTEBRCompare %f0s, %f0s; %f2s = LER %f0s  =>  LTEBRCompare %f2s, %f0s
-  MBBI = Compare, MBBE = MBB.end();
-  while (++MBBI != MBBE) {
-    MachineInstr &MI = *MBBI;
+  for (MachineBasicBlock::iterator MBBI =
+         std::next(MachineBasicBlock::iterator(&Compare)), MBBE = MBB.end();
+       MBBI != MBBE;) {
+    MachineInstr &MI = *MBBI++;
     if (preservesValueOf(MI, SrcReg)) {
       // Try to eliminate Compare by reusing a CC result from MI.
       if (convertToLoadAndTest(MI, Compare, CCUsers)) {
         EliminatedComparisons += 1;
         return true;
       }
     }
     if (getRegReferences(MI, SrcReg).Def)