[TwoAddressInstructionPass] Bugfix in handling of sunk instructions.

[jonpa]

Summary

An instruction returned by TII->convertToThreeAddress() may contain a %noreg
(undef) operand, which is not expected by tryInstructionTransform(). So if
this MI is sunk to a lower point in MBB, it must be skipped when later
encountered.

A new set SunkInstrs is used for this purpose.

Note: there is no test supplied here, as this was triggered on SystemZ while
working on a review of instruction flags. A test case for this bugfix will be
included in the upcoming SystemZ commit.

[qcolombet]

LGTM.

Say in the comment that the test case will be part of a SystemZ commit.

[jonpa]

r319646.

lib/CodeGen/TwoAddressInstructionPass.cpp

class TwoAddressInstructionPass : public MachineFunctionPass {
   MachineBasicBlock *MBB;
 
   // Keep track the distance of a MI from the start of the current basic block.
   DenseMap<MachineInstr*, unsigned> DistanceMap;
 
   // Set of already processed instructions in the current block.
   SmallPtrSet<MachineInstr*, 8> Processed;
 
+  // Set of instructions converted to three-address by target and then sunk
+  // down current basic block.
+  SmallPtrSet<MachineInstr*, 8> SunkInstrs;
+
   // A map from virtual registers to physical registers which are likely targets
   // to be coalesced to due to copies from physical registers to virtual
   // registers. e.g. v1024 = move r0.
   DenseMap<unsigned, unsigned> SrcRegMap;
 
   // A map from virtual registers to physical registers which are likely targets
   // to be coalesced to due to copies to physical registers from virtual
   // registers. e.g. r1 = move v1024.
TwoAddressInstructionPass::convertInstTo3Addr(MachineBasicBlock::iterator &mi,
 
   MBB->erase(mi); // Nuke the old inst.
 
   if (!Sunk) {
     DistanceMap.insert(std::make_pair(NewMI, Dist));
     mi = NewMI;
     nmi = std::next(mi);
   }
+  else
+    SunkInstrs.insert(NewMI);
 
   // Update source and destination register maps.
   SrcRegMap.erase(RegA);
   DstRegMap.erase(RegB);
   return true;
 }
 
 /// Scan forward recursively for only uses, update maps if the use is a copy or
bool TwoAddressInstructionPass::runOnMachineFunction(MachineFunction &Func) {
   for (MachineFunction::iterator MBBI = MF->begin(), MBBE = MF->end();
        MBBI != MBBE; ++MBBI) {
     MBB = &*MBBI;
     unsigned Dist = 0;
     DistanceMap.clear();
     SrcRegMap.clear();
     DstRegMap.clear();
     Processed.clear();
+    SunkInstrs.clear();
     for (MachineBasicBlock::iterator mi = MBB->begin(), me = MBB->end();
          mi != me; ) {
       MachineBasicBlock::iterator nmi = std::next(mi);
-      if (mi->isDebugValue()) {
+      // Don't revisit an instruction previously converted by target. It may
+      // contain undef register operands (%noreg), which are not handled.
+      if (mi->isDebugValue() || SunkInstrs.count(&*mi)) {
         mi = nmi;
         continue;
       }
 
       // Expand REG_SEQUENCE instructions. This will position mi at the first
       // expanded instruction.
       if (mi->isRegSequence())
         eliminateRegSequence(mi);