[llvm-commits] [llvm] r111973 - /llvm/trunk/lib/Target/ARM/ARMFastISel.cpp

[Jim Grosbach]

On Aug 24, 2010, at 3:30 PM, Eric Christopher wrote:

> 
> On Aug 24, 2010, at 3:28 PM, Bob Wilson wrote:
> 
>> 
>> On Aug 24, 2010, at 3:03 PM, Eric Christopher wrote:
>> 
>>> Author: echristo
>>> Date: Tue Aug 24 17:03:02 2010
>>> New Revision: 111973
>>> 
>>> URL: http://llvm.org/viewvc/llvm-project?rev=111973&view=rev
>>> Log:
>>> Fix thumb2 mode loads to have the correct operand ordering.  Add a todo
>>> to fix this in the port.
>>> 
>>> Modified:
>>>  llvm/trunk/lib/Target/ARM/ARMFastISel.cpp
>>> 
>>> Modified: llvm/trunk/lib/Target/ARM/ARMFastISel.cpp
>>> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/ARM/ARMFastISel.cpp?rev=111973&r1=111972&r2=111973&view=diff
>>> ==============================================================================
>>> --- llvm/trunk/lib/Target/ARM/ARMFastISel.cpp (original)
>>> +++ llvm/trunk/lib/Target/ARM/ARMFastISel.cpp Tue Aug 24 17:03:02 2010
>>> @@ -419,10 +419,15 @@
>>> // TODO: Verify the additions above work, otherwise we'll need to add the
>>> // offset instead of 0 and do all sorts of operand munging.
>>> unsigned ResultReg = createResultReg(FixedRC);
>>> -  unsigned Opc = AFI->isThumb2Function() ? ARM::tLDR : ARM::LDR;
>>> -  AddOptionalDefs(BuildMI(*FuncInfo.MBB, FuncInfo.InsertPt, DL,
>>> -                          TII.get(Opc), ResultReg)
>>> -                  .addReg(Reg).addReg(0).addImm(0));
>>> +  // TODO: Fix the Addressing modes so that these can share some code.
>>> +  if (AFI->isThumb2Function())
>>> +    AddOptionalDefs(BuildMI(*FuncInfo.MBB, FuncInfo.InsertPt, DL,
>>> +                            TII.get(ARM::tLDR), ResultReg)
>>> +                    .addReg(Reg).addImm(0).addReg(0));
>> 
>> You're checking for Thumb2 but then generating a Thumb1 LDR.  That doesn't seem right.
> 
> It's just a more restricted instruction afaict.  It seems to exist from my reading of things and the optimization passes that handle it. (Thumb2SizeReduction etc).

Basically, yeah. It's very restrictive on immediate offset (multiples of 4, 0-124). Since it's known here that the immediate is zero, that should be fine, if a bit off.

As a side note, if you use t2LDRi12 here, the Thumb2SizeReduction pass should be able to auto-magically shrink it to the 16-bit instruction.

-Jim