Not ready for commit yet; only includes X86 and WebAssembly.
Adds a method to MachineFunctionPass for derived classes to declare that they
support virtual registers, and an assert in the base runOnFunction to
enforce it. The idea is to make it more clear which passes support virtual
registers and to avoid accidentally exposing VRegs to passes which aren't
prepared to handle them.
Details
- Reviewers
qcolombet
Diff Detail
Event Timeline
include/llvm/CodeGen/MachineFunctionPass.h | ||
---|---|---|
61 | 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? |
include/llvm/CodeGen/MachineFunctionPass.h | ||
---|---|---|
61 | 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. |
Hi Derek,
I’m catching up with emails now.
I think I’ll get to this (and your other thread) next week.
Cheers,
-Quentin
include/llvm/CodeGen/MachineFunctionPass.h | ||
---|---|---|
61 |
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. |
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?