[PATCH] D70213: [ModuloSchedule] Fix a bug in experimental expander during prologue/epilogue stitching.

[Thomas Raoux via Phabricator via llvm-commits]

ThomasRaoux marked 2 inline comments as done.
ThomasRaoux added inline comments.


================
Comment at: llvm/lib/CodeGen/ModuloSchedule.cpp:1660
   InsertPt = DestBB->getFirstNonPHI();
-  for (MachineInstr &MI : SourceBB->phis()) {
-    MachineInstr *NewMI = MF.CloneMachineInstr(&MI);
-    DestBB->insert(InsertPt, NewMI);
-    Register OrigR = MI.getOperand(0).getReg();
-    Register R = MRI.createVirtualRegister(MRI.getRegClass(OrigR));
-    NewMI->getOperand(0).setReg(R);
-    NewMI->getOperand(1).setReg(OrigR);
-    NewMI->getOperand(2).setMBB(*DestBB->pred_begin());
-    Remaps[OrigR] = R;
-    CanonicalMIs[NewMI] = CanonicalMIs[&MI];
-    BlockMIs[{DestBB, CanonicalMIs[&MI]}] = NewMI;
-  }
   for (auto I = DestBB->getFirstNonPHI(); I != DestBB->end(); ++I)
+    for (MachineOperand &MO : I->uses()) {
----------------
jmolloy wrote:
> What's the rationale for this change? Is this just to avoid creating a phi that we destroy later?
> 
> If so, I'd prefer to keep the logic simple over optimal. We can always clean up dead nodes easily.
Yes the goal is to avoid inserting useless Phi. The problem is that when we have a large number of stages we can run into combinatorial explosion since we move the phi instructions block by block. I moved the logic in a lambda to make it easier to read.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D70213/new/

https://reviews.llvm.org/D70213