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

[liangh at codeaurora.org]

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.

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