[PATCH] D78159: [MachineDCE] Make sure MachineDCE considers subregs when adding liveins.

[Marcello Maggioni via Phabricator via llvm-commits]

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


================
Comment at: llvm/lib/CodeGen/DeadMachineInstructionElim.cpp:129
       for (const auto &LI : (*S)->liveins())
-        LivePhysRegs.set(LI.PhysReg);
+        for (MCSubRegIterator SR(LI.PhysReg, TRI, /*IncludeSelf=*/true);
+             SR.isValid(); ++SR)
----------------
arsenm wrote:
> arsenm wrote:
> > RegAliasIterator? also need the super regs?
> Wait, actually why is this code here at all? There is already a LivePhysRegs class which is what I initially assumed this was
1) I don't think the all the aliases should be considered "live". I think that if a register is not in the "live" list then it means that as a unit it is not considered live. Like, the subregisters here, if they are not in the list it means that they are not used as subregisters themselves in the block (until they are defined themselves potentially in the block). They can be live though as part of the a bigger entity

2) Yeah, this code doesn't use that. I just fixed the problems here, but its a good point, maybe we should port this code to use LivePhysReg. I'll take a stab at that.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D78159