This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC 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: [arm] Fix invalid store instructions


On Fri, 2004-06-18 at 18:54, Paul Brook wrote:
> In an arm store instruction the source register must not be the same as the 
> base address register when base writeback is enabled.
> 
> Some of the gcc regression tests cause this through the use of uninitialized 
> variables. It turns out it's also possible to cause this with the following 
> code:
> 
> foo(int a, char * b)
> {
>   register char * p asm("r2");
> 
>   p = b;
>   while (a--)
>     {
>       *p = (char)(int)p;
>       p++;
>     }
> }
> 
> The same restriction applies at stores of all widths, although I've only 
> managed to reproduce the bad code generation with byte stores.
> 
> The attached patch fixes the invalid instructions by outputting a seperate add 
> instructions. I also added an extra memory constraint and instruction 
> alternative to avoid pessimizing the non-writeback case.
> 
> Tested with cross to arm-none-elf.
> Ok?
> The test above is somewhat sensitive to the behaviour of the optimizers, 
> should I add it anyway?
> 
> Paul
> 
> 2004-06-18  Paul Brook  <paul@codesourcery.com>
> 
> 	* config/arm/arm-protos.h (arm_store_mem_op, arm_output_store): Add
> 	prototypes.
> 	* config/arm/arm.c (arm_store_mem_op, arm_output_store): New functions.
> 	* config/arm/arm.h (EXTRA_CONSTRAINT_STR_ARM): Handle 'Us'.
> 	* config/arm/arm.md (arm_movsi_insn, movhi_insn_arch4, arm_movqi_insn,
> 	arm_movsf_soft_insn): Add writeback alternative.
> 	* config/arm/iwmmxt.md (iwmmxt_movsi_insn, cond_iwmmxt_movsi_insn):
> 	Ditto.
> 	* config/arm/vfp.md (arm_movsi_vfp, movsf_vfp): Ditto.
> 	* doc/md.texi: Document 'Us' constraint.

My feeling is that this is an MI bug not a target specific one, so I
think it should really be fixed in the mid-end.

There is no way of representing an early-clobber type behaviour in the
constraints for a memory operand with pre/post operations so the
compiler really must assume that it does happen and prevent any further
use of the base register in the insn.

It's unclear to me at this time whether this has always been a
theoretical problem with the compiler, or if it's something new that's
been introduced by the tree-ssa changes.  I'm not aware that we suffered
from this problem before those changes were merged.

R.

[Given the above, this isn't particularly important, but you would also
need to handle pre/post_modify expressions.  That leads to further
complications because not all address offsets are legitimate immediates
for add/sub instructions.]


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