Re: [ARM] Fix PR49641

[Richard Earnshaw <rearnsha at arm dot com>]

On 07/07/11 21:02, Bernd Schmidt wrote:
> This corrects an error in store_multiple_operation. We're only
> generating the writeback version of the instruction on Thumb-1, so
> that's where we must make sure the base register isn't also stored.
> 
> The ARMv7 manual is unfortunately not totally clear that this does in
> fact produce unpredictable results; it seems to suggest that this is the
> case only for the T2 encoding. Older documentation makes it clear.
> 
> Tested on arm-eabi{,mthumb}.
> 

I agree that the wording here is unclear, but the pseudo code for the
decode makes the situation clearer, and does reflect what I really
believe to be the case.  Put explicitly:

For LDM:

- Encoding A1: Unpredictable if writeback and base in list (I believe
this is true for all architecture versions, despite what it says in the
current ARM ARM -- at least, my v5 copy certainly says unpredictable)

- Encoding T1: Not unpredictable, but deprecated (for base in list, the
loaded value used and writeback ignored).  Note, however, that in UAL
the ! operator on the base register must not be used if the base
register appears in the list.

- Encoding T2: Unpredictable if writeback and base in list



For STM:

- Encoding T2: Unpredictable if writeback and base in list regardless of
the position.

- Encodings T1 and A1: Unpredictable if writeback and base in list and
not lowest numbered register (note that encoding T1 always has
writeback).  In the case where the base is the first register in the
list, then the original value of base will be stored; deprecated.

This is all quite complicated, I hope I've expressed it correctly... :-)

R.

> 
> Bernd
> 
> 
> pr49641.diff
> 
> 
> 	* config/arm/arm.c (store_multiple_sequence): Avoid cases where
> 	the base reg is stored iff compiling for Thumb1.
> 
> 	* gcc.target/arm/pr49641.c: New test.
> 
> Index: gcc/config/arm/arm.c
> ===================================================================
> --- gcc/config/arm/arm.c	(revision 175906)
> +++ gcc/config/arm/arm.c	(working copy)
> @@ -9950,7 +9950,10 @@ store_multiple_sequence (rtx *operands,
>  	  /* If it isn't an integer register, then we can't do this.  */
>  	  if (unsorted_regs[i] < 0
>  	      || (TARGET_THUMB1 && unsorted_regs[i] > LAST_LO_REGNUM)
> -	      || (TARGET_THUMB2 && unsorted_regs[i] == base_reg)
> +	      /* For Thumb1, we'll generate an instruction with update,
> +		 and the effects are unpredictable if the base reg is
> +		 stored.  */
> +	      || (TARGET_THUMB1 && unsorted_regs[i] == base_reg)
>  	      || (TARGET_THUMB2 && unsorted_regs[i] == SP_REGNUM)
>  	      || unsorted_regs[i] > 14)
>  	    return 0;
> Index: gcc/testsuite/gcc.target/arm/pr49641.c
> ===================================================================
> --- gcc/testsuite/gcc.target/arm/pr49641.c	(revision 0)
> +++ gcc/testsuite/gcc.target/arm/pr49641.c	(revision 0)
> @@ -0,0 +1,18 @@
> +/* { dg-do compile } */
> +/* { dg-options "-mthumb -O2" } */
> +/* { dg-require-effective-target arm_thumb1_ok } */
> +/* { dg-final { scan-assembler-not "stmia\[\\t \]*r3!\[^\\n]*r3" } } */
> +typedef struct {
> +  void *t1, *t2, *t3;
> +} z;
> +extern volatile int y;
> +static inline void foo(z *x) {
> +  x->t1 = &x->t2;
> +  x->t2 = ((void *)0);
> +  x->t3 = &x->t1;
> +}
> +extern z v;
> +void bar (void) {
> +   y = 0;
> +   foo(&v);
> +}