[PATCH] D55909: [ExpandISelPseudos] Recompute liveins after introducing a new MBB.

[Florian Hahn via Phabricator via llvm-commits]

fhahn marked an inline comment as done.
fhahn added inline comments.


================
Comment at: lib/CodeGen/ExpandISelPseudos.cpp:75
+          for (MachineInstr &MI : reverse(*MBB)) {
+            for (MachineOperand &MOP : reverse(MI.operands())) {
+              if (!MOP.isReg() ||
----------------
qcolombet wrote:
> t.p.northover wrote:
> > t.p.northover wrote:
> > > I think the logic needed here is a little more complicated. Fortunately, someone else has already been scarred for life by working it out (I expect) and you can just call `addLiveIns` from `LivePhysRegs.h`
> > Actually, shouldn't whatever's doing the splitting update liveness too? For all ExpandISelPseudos knows there could be a dozen new blocks scattered across the function that need updating.
> +1
>Actually, shouldn't whatever's doing the splitting update liveness too? For all ExpandISelPseudos knows there could be a dozen new blocks scattered across the function that need updating.


We could push the responsibility to the lowering code, but it would mean that we have to go through all lowering code. Doing it in ExpandISelLowering potentially misses cases as well though.

I'll take a look at the lowering code, a common pattern seems to be calling MBB->splice & MBB->transferSuccessorsAndPHIs in the lowering code (I checked ARM and X86). In some cases, there is manual handling for CPSR already, in some cases (like COPY_STRUCT_BYVAL_I32) it is missing. 


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

https://reviews.llvm.org/D55909