[llvm-commits] [Review request] Moving code from PHIElimination to MachineBasicBlock

Chris Lattner clattner at apple.com
Sun Dec 5 10:38:07 PST 2010


On Dec 4, 2010, at 4:16 PM, Cameron Zwarich wrote:

> MachineBasicBlock should be responsible for the placement of phis in predecessor blocks, sparing PHIElimination the need to know the details. This will also let StrongPHIElimination share the code.

I don't have a strong opinion on this, but this function does seem pretty phi-elimination specific, and it seems strange for MBB.cpp to have to pull in MachineRegisterInfo.h.  If your goal is to share this between phi elimination implementation, it would probably be better in a PHIEliminationUtils.cpp file or something?

> I converted the argument from a reference to a pointer since that seems to be the style in MachineBasicBlock. I don't know if we are moving in one direction or the other in LLVM as a whole.

Sounds good to me.  In general, I'd prefer to use *'s everywhere.  I went through a misled period of time where  I thought it would be a great idea to use references to indicate that pointers were known-non-null.  This had the effect of leading to lots of arbitrary differences throughout the code :(

-Chris



More information about the llvm-commits mailing list