[PATCH] D20769: [IPRA] Interprocedural Register Allocation

[Matthias Braun via llvm-commits]

I know some of the issues are more a matter of personal preference than there being one true way. I just felt I should point out how I feel about them before people start writing them down as the one true way in the coding conventions :) I will of course not block the patches if we end up with a different style here.

> On May 31, 2016, at 6:02 PM, Mehdi AMINI <mehdi.amini at apple.com> wrote:
> 
> mehdi_amini added inline comments.
> 
> ================
> Comment at: lib/Target/X86/X86RegUsageInfoPropagate.cpp:93
> @@ +92,3 @@
> +}
> +
> +bool RegUsageInfoPropagationPass::runOnMachineFunction(MachineFunction &MF) {
> ----------------
> MatzeB wrote:
>>> for(auto &MBB : MF) {
>>> for(auto &MI : MBB) {
>> 
>> Please do not use `auto` in contexts where it is not obvious what type it represents. It is friendlier for the readers to use `MachineBasicBlock &MBB` and `MachineInstr &MI` here.
>> 
> I feel  that it is clearly obvious that `MI` is a `MachineInstr` when reading `for(auto &MI : MBB) {`, I'll be explicit if it was something else. But I won't pick this fight :)
> 
> ================
> Comment at: lib/Target/X86/X86RegUsageInfoPropagate.cpp:114
> @@ +113,3 @@
> +        auto updateRegMask = [&](StringRef FuncName) {
> +          const auto *RegMask = PRUI->getRegUsageInfo(FuncName);
> +          if (RegMask) { // else skip optimization
> ----------------
> MatzeB wrote:
>> This does not seem complicated enough to me to warrant a lambda. What about:
>> 
>> 
>> ```
>> const char *FuncName = nullptr;
>> if (Operand.isGlobal()) {
>>  Name = Operand.getGlobal()->getName();
>> } else if (Operand.isSymbol()) {
>>  Name = Operand.getGlobal()->getName();
>> }
>> if (FuncName != nullptr && RegMask) {
>>  setRegMask(MI, &(*RegMask)[0]);
>>  changed = true;
>> }
>> ```
> I guess it is a matter of personal style, I'd try to avoid this construct when possible, but I don't really care here.
> 
> 
> http://reviews.llvm.org/D20769
> 
> 
>