[llvm] r194019 - Submit the basic port of the rest of ARM constant islands code to Mips.

Eric Christopher echristo at gmail.com
Mon Nov 4 14:49:32 PST 2013


On Mon, Nov 4, 2013 at 2:48 PM, reed kotler <rkotler at mips.com> wrote:

>  I will take care of this right. I was already in the process of doing as
> you suggest.
> Some code I wanted as a record in the source control so I can see where I
> need to fix things.
>
>
This isn't the right way to do it, I'm sorry. You can use local version
control if you want to keep in progress patches.

-eric



> Reed
>
>
> On 11/04/2013 02:33 PM, Eric Christopher wrote:
>
>
>> +        STI(&TM.getSubtarget<MipsSubtarget>()), MF(0), MCP(0),
>> PrescannedForConstants(false){}
>>
>>
>  80-col.
>
>
>> +#ifdef IN_PROGRESS
>>
>
>  No. Please remove this sort of code.
>
>
>> +#ifdef IN_PROGRESS
>> +        case ARM::t2BR_JT:
>>
>
>  Especially not when you've just copied code out of the ARM port.
>
>
>> +
>> +      }
>> +
>> +
>> +      if (Opc == Mips::CONSTPOOL_ENTRY)
>> +        continue;
>> +
>> +
>>
>
>  I'm a fan of vertical whitespace, but this is a bit much.
>
>
>>
>> +#ifdef IN_PROGRESS
>> +          // Taking the address of a CP entry.
>> +          case ARM::LEApcrel:
>>
>
>  Again.
>
>
>> +
>> +  // On Thumb, offsets==2 mod 4 are rounded down by the hardware for
>> +  // purposes of the displacement computation; compensate for that here.
>> +  // For unknown alignments, getMaxDisp() constrains the range instead.
>> +#ifdef IN_PROGRESS
>> +  if (isThumb && U.KnownAlignment)
>> +    UserOffset &= ~3u;
>> +#endif
>>
>
>  Again. There are many more cases in the rest of the code.
>
>
>> +/// isCPEntryInRange - Returns true if the distance between specific MI
>> and
>> +/// specific ConstPool entry instruction can fit in MI's displacement
>> field.
>> +bool MipsConstantIslands::isCPEntryInRange(MachineInstr *MI, unsigned
>> UserOffset,
>> +                                      MachineInstr *CPEMI, unsigned
>> MaxDisp,
>> +                                      bool NegOk, bool DoDump) {
>>
>
>  Why a DoDump variable?
>
>
>> +  // Avoid splitting an IT block.
>>
>
>  You have IT blocks? This is why we don't just copy code.
>
>  Again, please remove all of the non-mips code out of this immediately.
>
>  -eric
>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20131104/3f113c9c/attachment.html>


More information about the llvm-commits mailing list