[PATCH] D48154: [VirtRegRewriter] Avoid clobbering registers when expanding copy bundles

[Justin Bogner via Phabricator via llvm-commits]

bogner added inline comments.


================
Comment at: lib/CodeGen/VirtRegMap.cpp:426
+      for (const MachineInstr *Src : Srcs)
+        if (Src != Dst)
+          if (TRI->regsOverlap(Dst->getOperand(0).getReg(),
----------------
MatzeB wrote:
> I think regsOverlap also has a Src==Dst shortcut, so this should be unnecessary. So the 2 ifs can shrink to `return TRI->regsOverlap()`
This check is for if the src and dst are the same instruction, in which case we're okay with them aliasing. For example, in

    $q1_q2_q3 = COPY $q0_q1_q2 {
      $q4 = COPY $q3
    }

We aren't interested in the fact that $q1_q2_q3 aliases $q0_q1_q2 - that's the instruction's own operand. What we care about is that $q1_q2_q3 aliases $q3.


================
Comment at: lib/CodeGen/VirtRegMap.cpp:455-461
+      if (Inst != BundleStart) {
+        Inst->removeFromBundle();
+        MBB.insert(FirstMI->getIterator(), Inst);
+      } else if (Inst->isBundledWithSucc()) {
+        Inst->unbundleFromSucc();
+        BundleStart = &*std::next(Inst->getIterator());
+      }
----------------
MatzeB wrote:
> I guess this could just be `Inst->removeFromBundle()` in both cases (to simplify the code).
removeFromBundle is a bit confusingly named - it's removeFromParent for bundled instructions. We use unbundleFromSucc here so it stays in the block but stops being bundled.


https://reviews.llvm.org/D48154