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

[Eric Christopher]

On Fri Feb 13 2015 at 3:47:23 PM reed kotler <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
> 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/cdd116cb/attachment.html>