[PATCH] D36415: Insert IMPLICIT_DEFS for undef uses in tail merging

[Matthias Braun via Phabricator via llvm-commits]

MatzeB added inline comments.


================
Comment at: lib/CodeGen/BranchFolding.cpp:385-398
+      CalcLiveIns.init(*TRI);
+      CalcLiveIns.addLiveOutsNoPristines(*Pred);
+      for (const MachineInstr &MI : llvm::reverse(*Pred))
+        CalcLiveIns.stepBackward(MI);
+
+      LivePhysRegs LiveIns(*TRI);
+      LiveIns.addLiveIns(*Pred);
----------------
kparzysz wrote:
> MatzeB wrote:
> > I'm confused: Don't you need to compare the live-outs of `Pred` with the live-ins of `NewDest`? Adding the liveouts of Pred and stepping backwards over the instructions gives you the live-ins of Pred, but I don't see what they are good for.
> > 
> > I would have expected this to look like this:
> > ```
> > PredLiveOuts.init(*TRI);
> > PrevLiveOuts.addLiveOutsNoPristines(*Pred);
> > // Add implicit defs for NewDest live-in values not available at end of Pred.
> > for (const MachineBasicBlock::RegisterMaskPair &LI : NewDest->liveins()) {
> >   // computeLiveIn() currently doesn't add partial live-ins so we are fine with an assert.
> >   assert(LI.LaneMask == LaneBitmask::getAll() && "found partial live-in");
> >   if (PredLiveOuts.available(*MRI, LI.PhysReg)) {
> >     BuildMI ...
> >   }
> > }
> > ```
> You're right, this won't work.  Still, we need to traverse the block, and it will have to use stepForward.  Consider this case:
> 
> ```
> MBB:
>   liveins: R0
>   R0<def,dead> = ... R0<kill>    ; stays in MBB
>   ... = R0<undef>                ; goes into common tail, but no longer <undef>
> ```
> 
> We need to add R0<def> = IMPLICIT_DEF here.
> 
> We can't rely on addLiveOuts from LivePhysRegs, because NewDest->liveins() will (by definition) be a subset of Pred->addLiveOuts.
> We can't rely on addLiveOuts from LivePhysRegs, because NewDest->liveins() will (by definition) be a subset of Pred->addLiveOuts.

Fair point. This is trickier than it seemed at first. I'm still convinced though that it can be done, and I really want to avoid `stepForward()`. I'll have a shot at implementing it.


Repository:
  rL LLVM

https://reviews.llvm.org/D36415