[PATCH] D16483: Require MachineFunctionPasses to declare their support for virtual registers.

Quentin Colombet via llvm-commits llvm-commits at lists.llvm.org
Wed Feb 10 16:20:04 PST 2016


qcolombet added inline comments.

================
Comment at: include/llvm/CodeGen/MachineFunctionPass.h:61
@@ -57,2 +60,3 @@
 } // End llvm namespace
+#define SUPPORTS_VIRTUAL_REGISTERS   bool getSupportsVirtualRegisters() const override { return true; }
 
----------------
dschuff wrote:
> hfinkel wrote:
> > I think that what you gain in brevity you lose in clarity, because while the macro is shorter than the function call, it is now not immediately obvious what it does. Just include the function override.
> > 
> > An alternative is to make it an argument to the MachineFunctionPass constructor.
> > 
> > Also, I'm not entirely sure this is a problem worth solving, at least not as a one-off. We already have passes that only work either before or after register allocation, in or not in SSA form, before or after PHI elimination, etc. -- and we don't explicitly declare any of that.
> > 
> > Generically, it would be nice to express these dependencies in a more-consistent way, and I'm in favor of something that let's us do that. We already have a facility in the pass manager to express requirements, maybe we can build on that?
> > 
> Yeah it was my intention to expand the macros manually (and fill in the rest of the architectures) before committing. I guess I mentioned that in the mailing list post but not here.
> 
> As for general requirements, that's not a bad idea either. I toyed with making this a sort of pseudo analysis, but if we made some other notion about properties of the MIR at a particular time (is SSA, has virtual registers, etc) that would fit.
> I toyed with making this a sort of pseudo analysis, but if we made some other notion about properties of the MIR at a particular time (is SSA, has virtual registers, etc) that would fit.

I didn’t get that. I am guessing you wanted to say “wouldn’t fit” since the pass manager has currently no way to tell what happened at a particular time.

Anyhow, for the problem we are trying to solve, I am not sure a single boolean is enough. For instance, the peephole optimizer supports virtual registers, but only works on SSA code. I agree with Hal that what we want is a way to express dependencies in a more-consistent way.

To get concrete here, I think that we could start with some properties on the MachineFunction (like isSSA). Then, the machine verifier would check the assumptions are correct based on them, like after register allocation we don’t have any virtual registers anymore. Other passes would have a way to express what they support based on those properties. Finally, if we try to schedule a pass whereas the properties do not match what it supports, this is an error.


http://reviews.llvm.org/D16483





More information about the llvm-commits mailing list