This is the mail archive of the binutils@sourceware.org mailing list for the binutils project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH v3] gas: Emit A2 encoding for ARM PUSH/POP with single register


On 02/04/12 20:00, Meador Inge wrote:
> Hi,
> 
> This patch changes GAS to emit the A2 encoding for PUSH/POP instructions
> with a single register.  This case is specified by the ARMARM: A8.8.132,
> A8.8.133 [1].  The A2 encoding is allowed on the following architecture
> versions: ARMv4*, ARMv5T*, ARMv6*, and ARMv7.
> 
> Tested with arm-none-eabi configuration.  No regressions.
> 
> OK?
> 
> P.S. If this is OK, then can someone commit for me?  I don't have write
> access.
> 
> [1] http://infocenter.arm.com/help/index.jsp?topic=/com.arm.doc.ddi0406c/index.html
> 
> Changes since v2 (Fixed issues pointed out by Richard Earnshaw):
> 
>    * The A2 encoding should only be applied to PUSH/POP mnemonics.
>    * Added more test cases.
>    * Some simplifications.
> 
> Changes since v1 (Improvements suggested by Matthew Gretton-Dann):
> 
>    * Use 'ffs' instead of manual loop to see if a bit is set.
>    * Use macros instead of magic number bit patterns.
> 
> 2012-04-02  Jie Zhang  <jie@codesourcery.com>
>             Meador Inge  <meadori@codesourcery.com>
> 
> 	* config/tc-arm.c (only_one_reg_in_list): New function.
> 	(encode_ldmstm): Ditto.
> 	(do_ldmstm): Use a different encoding when pushing or poping
> 	a single register.
> 	(A_COND_MASK): New macro.
> 	(A_PUSH_POP_OP_MASK): Ditto.
> 	(A1_OPCODE_PUSH): Ditto.
> 	(A2_OPCODE_PUSH): Ditto.
> 	(A2_OPCODE_POP): Ditto.
> 
> 2012-04-02  Jie Zhang  <jie@codesourcery.com>
>             Meador Inge  <meadori@codesourcery.com>
> 
> 	* gas/arm/push-pop.d: New testcase.
> 	* gas/arm/push-pop.s: Ditto.
> 	* gas/arm/stm-ldm.d: Ditto.
> 	* gas/arm/stm-ldm.s: Ditto.
> 

This is OK.

It won't hold up this patch, but I'll note here for completeness that
this exposes the fact that the disassembly of a single-register ldm/stm
from SP as pop/push is incorrect and that these really should be printed
out as ldm/stm operations.

R.

> diff --git a/gas/config/tc-arm.c b/gas/config/tc-arm.c
> index 585f78e..545b7ec 100644
> --- a/gas/config/tc-arm.c
> +++ b/gas/config/tc-arm.c
> @@ -622,6 +622,14 @@ struct asm_opcode
>  #define T2_OPCODE_MASK	0xfe1fffff
>  #define T2_DATA_OP_SHIFT 21
>  
> +#define A_COND_MASK         0xf0000000
> +#define A_PUSH_POP_OP_MASK  0x0fff0000
> +
> +/* Opcodes for pushing/poping registers to/from the stack.  */
> +#define A1_OPCODE_PUSH    0x092d0000
> +#define A2_OPCODE_PUSH    0x052d0004
> +#define A2_OPCODE_POP     0x049d0004
> +
>  /* Codes to distinguish the arithmetic instructions.  */
>  #define OPCODE_AND	0
>  #define OPCODE_EOR	1
> @@ -7795,11 +7803,21 @@ do_it (void)
>      }
>  }
>  
> +/* If there is only one register in the register list,
> +   then return its register number.  Otherwise return -1.  */
> +static int
> +only_one_reg_in_list (int range)
> +{
> +  int i = ffs (range) - 1;
> +  return (i > 15 || range != (1 << i)) ? -1 : i;
> +}
> +
>  static void
> -do_ldmstm (void)
> +encode_ldmstm(int from_push_pop_mnem)
>  {
>    int base_reg = inst.operands[0].reg;
>    int range = inst.operands[1].imm;
> +  int one_reg;
>  
>    inst.instruction |= base_reg << 16;
>    inst.instruction |= range;
> @@ -7832,6 +7850,23 @@ do_ldmstm (void)
>  	    as_warn (_("if writeback register is in list, it must be the lowest reg in the list"));
>  	}
>      }
> +
> +  /* If PUSH/POP has only one register, then use the A2 encoding.  */
> +  one_reg = only_one_reg_in_list (range);
> +  if (from_push_pop_mnem && one_reg >= 0)
> +    {
> +      int is_push = (inst.instruction & A_PUSH_POP_OP_MASK) == A1_OPCODE_PUSH;
> +
> +      inst.instruction &= A_COND_MASK;
> +      inst.instruction |= is_push ? A2_OPCODE_PUSH : A2_OPCODE_POP;
> +      inst.instruction |= one_reg << 12;
> +    }
> +}
> +
> +static void
> +do_ldmstm (void)
> +{
> +  encode_ldmstm (/*from_push_pop_mnem=*/FALSE);
>  }
>  
>  /* ARMv5TE load-consecutive (argument parse)
> @@ -8333,7 +8368,7 @@ do_push_pop (void)
>    inst.operands[0].isreg = 1;
>    inst.operands[0].writeback = 1;
>    inst.operands[0].reg = REG_SP;
> -  do_ldmstm ();
> +  encode_ldmstm (/*from_push_pop_mnem=*/TRUE);
>  }
>  
>  /* ARM V6 RFE (Return from Exception) loads the PC and CPSR from the
> diff --git a/gas/testsuite/gas/arm/push-pop.d b/gas/testsuite/gas/arm/push-pop.d
> new file mode 100644
> index 0000000..6eabbfa
> --- /dev/null
> +++ b/gas/testsuite/gas/arm/push-pop.d
> @@ -0,0 +1,14 @@
> +#objdump: -dr --prefix-addresses --show-raw-insn
> +#name: PUSH and POP
> +
> +# Test the `PUSH' and `POP' instructions
> +
> +.*: +file format .*arm.*
> +
> +Disassembly of section .text:
> +0+000 <.*> e52d0004 	push	{r0}		; \(str r0, \[sp, #-4\]!\)
> +0+004 <.*> e92d000e 	push	{r1, r2, r3}
> +0+008 <.*> e52d9004 	push	{r9}		; \(str r9, \[sp, #-4\]!\)
> +0+00c <.*> e49d9004 	pop	{r9}		; \(ldr r9, \[sp\], #4\)
> +0+010 <.*> e8bd000e 	pop	{r1, r2, r3}
> +0+014 <.*> e49d0004 	pop	{r0}		; \(ldr r0, \[sp\], #4\)
> diff --git a/gas/testsuite/gas/arm/push-pop.s b/gas/testsuite/gas/arm/push-pop.s
> new file mode 100644
> index 0000000..d86ec9e
> --- /dev/null
> +++ b/gas/testsuite/gas/arm/push-pop.s
> @@ -0,0 +1,8 @@
> +	.text
> +	.syntax unified
> +	push {r0}
> +	push {r1, r2, r3}
> +	push {r9}
> +	pop {r9}
> +	pop {r1, r2, r3}
> +	pop {r0}
> diff --git a/gas/testsuite/gas/arm/stm-ldm.d b/gas/testsuite/gas/arm/stm-ldm.d
> new file mode 100644
> index 0000000..564b8bc
> --- /dev/null
> +++ b/gas/testsuite/gas/arm/stm-ldm.d
> @@ -0,0 +1,14 @@
> +#objdump: -dr --prefix-addresses --show-raw-insn
> +#name: STM and LDM
> +
> +# Test the `STM*' and `LDM*' instructions
> +
> +.*: +file format .*arm.*
> +
> +Disassembly of section .text:
> +0+000 <.*> e92d0001 	push	{r0}
> +0+004 <.*> e92d000e 	push	{r1, r2, r3}
> +0+008 <.*> e92d0200 	push	{r9}
> +0+00c <.*> e8bd0200 	pop	{r9}
> +0+010 <.*> e8bd000e 	pop	{r1, r2, r3}
> +0+014 <.*> e8bd0001 	pop	{r0}
> diff --git a/gas/testsuite/gas/arm/stm-ldm.s b/gas/testsuite/gas/arm/stm-ldm.s
> new file mode 100644
> index 0000000..77bbfbb
> --- /dev/null
> +++ b/gas/testsuite/gas/arm/stm-ldm.s
> @@ -0,0 +1,8 @@
> +	.text
> +	.syntax unified
> +	stmfd	sp!, {r0}
> +	stmfd	sp!, {r1, r2, r3}
> +	stmfd	sp!, {r9}
> +	ldmia sp!, {r9}
> +	ldmia sp!, {r1, r2, r3}
> +	ldmia sp!, {r0}
> 
> 



Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]