[llvm-commits] patch: THUMB2 support for PUSH a single reg

[liangh at codeaurora.org]

Hi Jim,

You are right. I'll fix the doc.

Thanks.
-Liang

> In addition to Evan's comments, the patch description is misleading.
> Thumb2 is already using a (32-bit wide) push instruction (which per the
> ARM docs is encoded as a pre-indexed STR instruction). What it's not
> doing, and your patch seeks to address, is shrinking that instruction to a
> 16-bit wide Thumb1 encoding when possible. Please make sure to document
> both the patch description and the code accordingly.
>
> -Jim
>
> On Oct 20, 2012, at 11:18 AM, Evan Cheng <evan.cheng at apple.com> wrote:
>
>> 1: Stylistic nitpick:
>> +  if(!Skip){
>> Please make sure there is a space between 'if' and '('.
>>
>> 2.
>> +  case ARM::t2STR_PRE:{
>> +    if (MI->getOperand(2).getReg() != ARM::SP)
>> +      return false;
>>
>> Is this correct? Does it need to check the offset?
>>
>> 3.
>>
>> +    Skip=true;
>> +    break;
>> +  }
>>   case ARM::t2LDMIA_UPD:
>>   case ARM::t2LDMDB_UPD:
>>   case ARM::t2STMIA_UPD:
>> @@ -430,46 +446,48 @@
>> Thumb2SizeReduce::ReduceLoadStore(MachineBasicBlock &MBB, MachineInstr
>> *MI,
>>   }
>>   }
>>
>> -  unsigned OffsetReg = 0;
>> -  bool OffsetKill = false;
>> -  if (HasShift) {
>> -    OffsetReg  = MI->getOperand(2).getReg();
>> -    OffsetKill = MI->getOperand(2).isKill();
>> +  if(!Skip){
>>
>> Why not use an early exit instead of the nested condition?
>>
>> Evan
>>
>> On Oct 19, 2012, at 4:02 PM, liangh at codeaurora.org wrote:
>>
>>> Hi,
>>>
>>> Currently, LLVM doesn't generate PUSH instructions for THUMB2 when a
>>> single register is pushed.
>>> For example, it issues "str r7, [sp, #-4]!" but not "push {r7}".
>>>
>>> This patch adds THUMB2 support for PUSH instruction with a single
>>> register. So that a real PUSH instruction will be generated for this
>>> case.
>>> I.e.: str r7, [sp, #-4]! => push {r7}
>>>
>>> Could you please review the attached patch?
>>> Thanks.
>>>
>>> -Liang<0001-Add-Thumb2-support-for-PUSH-instruction-with-a-singl.patch>_______________________________________________
>>> llvm-commits mailing list
>>> llvm-commits at cs.uiuc.edu
>>> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>>
>> _______________________________________________
>> llvm-commits mailing list
>> llvm-commits at cs.uiuc.edu
>> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>
>