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

reed kotler rkotler at mips.com
Fri Feb 13 15:32:13 PST 2015


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?
> - 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?

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?
>
> +    unsigned Offset = Addr.getOffset();
> +    ;
> +    MachineFrameInfo &MFI = *MF->getFrameInfo();
> +    ;
>
> ?
You mean the extra ";" lines?
Hmmm.. probably from clang format but I don't know.
Looks wrong.

> -eric
>
>
> http://reviews.llvm.org/D6426
>
> EMAIL PREFERENCES
>    http://reviews.llvm.org/settings/panel/emailpreferences/
>
>




More information about the llvm-commits mailing list