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

Quentin Colombet via llvm-commits llvm-commits at lists.llvm.org
Fri Feb 5 08:49:05 PST 2016


Hi Derek,

I’m catching up with emails now.
I think I’ll get to this (and your other thread) next week.

Cheers,
-Quentin

> On Feb 4, 2016, at 4:03 PM, Derek Schuff <dschuff at google.com> wrote:
> 
> dschuff 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; }
> 
> ----------------
> 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.
> 
> 
> http://reviews.llvm.org/D16483
> 
> 
> 



More information about the llvm-commits mailing list