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

[Jim Grosbach]

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