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

reed kotler rkotler at mips.com
Fri Feb 13 16:09:49 PST 2015


Unfortunately, my  patches that I'm trying to finally commit now are 
very old and they have been blocked
in the patch review queue for months now.

Some things are getting done during auto merge that may not be intended.

Since the original patches some other people have made some cosmetic 
changes to mips fast-isel and some
other things have changed in llvm.

I can go back and look at the patch history for these lines but if you 
have something specific you want you
can just say it.

On 02/13/2015 04:05 PM, Eric Christopher wrote:
>
>
> On Fri Feb 13 2015 at 4:00:39 PM reed kotler <rkotler at mips.com 
> <mailto:rkotler at mips.com>> wrote:
>
>     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.
>
>
>
> Rather than write it out again, how about you look at the patch that 
> changed the code that you're trying to change back?
>
> -eric
>
>     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/d76a8bdb/attachment.html>


More information about the llvm-commits mailing list