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

[Evan Cheng]

On Oct 23, 2012, at 3:35 PM, liangh at codeaurora.org wrote:

> Hi Evan,
> 
> Please see below inlined answers.
> 
> Thanks.
> -Liang
> 
>> 1: Stylistic nitpick:
>> +  if(!Skip){
>> Please make sure there is a space between 'if' and '('.
> 
> Will fix it.
> 
>> 
>> 2.
>> +  case ARM::t2STR_PRE:{
>> +    if (MI->getOperand(2).getReg() != ARM::SP)
>> +      return false;
>> 
>> Is this correct? Does it need to check the offset?
> 
> Yes. We are not targeting all t2STR_PRE. We are targeting "PUSH", so we
> only care about instructions like "str r7, [sp, #-4]!" who has SP as a
> base address.
> 
>> 
>> 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?
> 
> That was to share the common code after this skipped section. To my
> understanding, copying those lines and exit early is not better; because
> people would have to change two places if they want to change these shared
> code before return, and they might have concern on touching the code
> located in a special case. But if you think that is a cleanner way I'm
> open to make the change as you suggested.

It's hard to see that from just the patch. In addition to the patch, please also include a copy of the updated Thumb2SizeReduction.cpp.

Thanks,

Evan

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