Home Original page

[PATCH ARM/THUMB1] unified syntax MOV with both low register emitting unpredictable code

Matthew Gretton-Dann matthew.gretton-dann@arm.com
Tue Nov 15 15:14:00 GMT 2011 
On 14/11/11 20:21, Thomas Klein wrote:
> Hi,
>
> There seems to be a minor bug when running assembler with 'unified
> syntax' sources.
> The old v4t and v5t architectures did not support a MOV rd, rs with both
> low registers.
> At pre 'unified syntax' time an 'ADD rd, rs, #0' was used instead.
> This instruction changes the flags. So this is not a real move instruction.
>
> I think for architecture v4t and v5t it is better to emit a 'unified
> syntax ADDS rd, rs, #0'
> than to emit unpredictable code.
> Also the author should be informed about the change.
>
> regards
>    Thomas
>
>
> diff.out
>
>
> Index: gas/config/tc-arm.c
> ===================================================================
> RCS file: /cvs/src/src/gas/config/tc-arm.c,v
> retrieving revision 1.504
> diff -u -p -r1.504 tc-arm.c
> --- gas/config/tc-arm.c	7 Nov 2011 16:20:47 -0000	1.504
> +++ gas/config/tc-arm.c	13 Nov 2011 19:18:01 -0000
> @@ -10913,10 +10913,24 @@ do_t_mov_cmp (void)
>   	switch (inst.instruction)
>   	  {
>   	  case T_MNEM_mov:
> -	    inst.instruction = T_OPCODE_MOV_HR;
> -	    inst.instruction |= (Rn&  0x8)<<  4;
> -	    inst.instruction |= (Rn&  0x7);
> -	    inst.instruction |= Rm<<  3;
> +	    if (low_regs&&  !ARM_CPU_HAS_FEATURE (selected_cpu, arm_ext_v6t2))

This test is incorrect - you want to check for arm_ext_v6, as all v6 
variants whether Thumb-1 or Thumb-2 allow low registers in this instruction.

> +	      {
> +		 as_warn (_("MOV r%u,r%u is not permitted on this architecture "
> +			    "using ADDS r%u,r%u,#0 instead"),Rn,Rm,Rn,Rm);
> +		 /* In v4t or v5t a move of two lowregs produces
> +		    unpredictable results. Encode this the same way as in
> +		    non unified syntax.*/
> +		 inst.instruction = T_OPCODE_ADD_I3;
> +		 inst.instruction |= Rn;
> +		 inst.instruction |= Rm<<  3;
> +	      }
> +	    else
> +	      {
> +		 inst.instruction = T_OPCODE_MOV_HR;
> +		 inst.instruction |= (Rn&  0x8)<<  4;
> +		 inst.instruction |= (Rn&  0x7);
> +		 inst.instruction |= Rm<<  3;
> +	      }
>   	    break;
>
>   	  case T_MNEM_movs:
>
>
> ChangeLog
>
>
> 2011-11-14  Thomas Klein<th.r.klein@web.de>
>
> 	* config/tc-arm.c (do_t_mov_cmp): Encode low register mov for arch v4t
> 	and v5t the same way as in non unified syntax form.

Some regression tests would be good here.

Thanks,

Matt

-- 
Matthew Gretton-Dann
Principal Engineer, PD Software - Tools, ARM Ltd
More information about the Binutils mailing list