[PATCH] Beginning of alloca implementation for Mips fast-isel

[reed kotler]

Subtarget is initialized first and then TargetSupported.

It may have been a bad choice of name but TargetSupported is derived 
from Subtarget.
That name has been in Mips fast-isel for a long time now. It's a local 
variable in the mips fast-isel class.

Probably should have been called SubtargetSupported.

On 02/13/2015 03:50 PM, Eric Christopher wrote:
>
>
> On Fri Feb 13 2015 at 3:47:23 PM reed kotler <rkotler at mips.com 
> <mailto:rkotler at mips.com>> wrote:
>
>     Not sure I understand some of these comments.
>
>     On 02/13/2015 03:21 PM, Eric Christopher wrote:
>     > Hi Reed,
>     >
>     > Some quick inline comments.
>     >
>     > +      assert(isFIBase() && "Invalid base frame index  access!");
>     >
>     > formatting.
>     This how clang format does it. What do you want?
>
>
> You have an extra space between index and access.
>
>     > - const MipsSubtarget *Subtarget; const TargetInstrInfo &TII;
>     const TargetLowering &TLI;
>     >
>     > +  const MipsSubtarget *Subtarget;
>     >
>     > Unnecessary.
>     >
>     > +        Subtarget(&TM.getSubtarget<MipsSubtarget>()) {
>     >
>     > No.
>     >
>     > +    TargetSupported = ((Subtarget->getRelocationModel() ==
>     Reloc::PIC_) &&
>     > +                       ((Subtarget->hasMips32r2() ||
>     > Subtarget->hasMips32()) &&
>     > +                        (Subtarget->isABI_O32())));
>     >
>     > Ditto.
>
>     What is the issue here?
>
>
> You've moved where subtarget is initialized, and changed (in the wrong 
> way) how Subtarget and TargetSupported are initialized.
>
>     Formatting?
>
>     This is what clang format does.
>     >
>     > +  // This code is mostly cloned from AArch64 (which cloned it
>     from earlier
>     > +  // ports)'
>     >
>     > Unnecessary.
>     You mean the comment is not necessary?
>
>
> Correct.
>
>     >
>     > +    unsigned Offset = Addr.getOffset();
>     > +    ;
>     > +    MachineFrameInfo &MFI = *MF->getFrameInfo();
>     > +    ;
>     >
>     > ?
>     You mean the extra ";" lines?
>     Hmmm.. probably from clang format but I don't know.
>
>
> clang-format would not have put them there. It doesn't insert characters.
>
>     Looks wrong.
>
>
> Yep.
>
> -eric
>
>
>     > -eric
>     >
>     >
>     > http://reviews.llvm.org/D6426
>     >
>     > EMAIL PREFERENCES
>     > http://reviews.llvm.org/settings/panel/emailpreferences/
>     >
>     >
>
>     _______________________________________________
>     llvm-commits mailing list
>     llvm-commits at cs.uiuc.edu <mailto:llvm-commits at cs.uiuc.edu>
>     http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20150213/4368cc09/attachment.html>