[ARM] Fix ICE in minipool handling at -Os

We ran into an ICE at -Os on the 4.7 branch for ARM (BE/VFPv3/ARM):

FAIL: gcc.c-torture/compile/920928-2.c  -Os  (internal compiler error)

It's an assertion deep in the ARM back-end:

  /* If an insn doesn't have a range defined for it, then it isn't
     expecting to be reworked by this code.  Better to stop now than
     to generate duff assembly code.  */
  gcc_assert (fix->forwards || fix->backwards);

This happens for arm_zero_extendhisi2_v6, but I fail to see what is different 
for it from arm_extendhisi2_v6, which is expecting to be reworked.

Hence the attached patch, which copies attributes from arm_extendhisi2_v6 to 
arm_zero_extendhisi2_v6.  No regressions on ARM, OK for the mainline?


2013-03-23  Eric Botcazou  <ebotcazou@adacore.com>

	* config/arm/arm.md (arm_zero_extendhisi2): Add pool_range and
	neg_pool_range attributes.
	(arm_zero_extendhisi2_v6): Likewise.

On 23/03/13 11:20, Eric Botcazou wrote:
> We ran into an ICE at -Os on the 4.7 branch for ARM (BE/VFPv3/ARM):
>
> FAIL: gcc.c-torture/compile/920928-2.c  -Os  (internal compiler error)
>
> It's an assertion deep in the ARM back-end:
>
>    /* If an insn doesn't have a range defined for it, then it isn't
>       expecting to be reworked by this code.  Better to stop now than
>       to generate duff assembly code.  */
>    gcc_assert (fix->forwards || fix->backwards);
>
> This happens for arm_zero_extendhisi2_v6, but I fail to see what is different
> for it from arm_extendhisi2_v6, which is expecting to be reworked.
>
> Hence the attached patch, which copies attributes from arm_extendhisi2_v6 to
> arm_zero_extendhisi2_v6.  No regressions on ARM, OK for the mainline?
>
>
> 2013-03-23  Eric Botcazou  <ebotcazou@adacore.com>
>
> 	* config/arm/arm.md (arm_zero_extendhisi2): Add pool_range and
> 	neg_pool_range attributes.
> 	(arm_zero_extendhisi2_v6): Likewise.
>
>

Having half-word accesses into the minipool is generally a bad idea. 
The limited offset range that's supported by these instructions means 
it's much more likely that we'll end up with a pool after a conditional 
branch or, worse, in the middle of a linear code sequence.  That means 
we have to jump around the pool, which costs performance.

We really need to find out why the compiler keeps trying to create these 
and fix that problem rather than work around the issue.

I'm not sure why the v6 variants allow this; I thought that had been 
taken out.

R.

>
> p.diff
>
>
> Index: config/arm/arm.md
> ===================================================================
> --- config/arm/arm.md	(revision 196816)
> +++ config/arm/arm.md	(working copy)
> @@ -4650,7 +4650,9 @@ (define_insn "*arm_zero_extendhisi2"
>      #
>      ldr%(h%)\\t%0, %1"
>     [(set_attr "type" "alu_shift,load_byte")
> -   (set_attr "predicable" "yes")]
> +   (set_attr "predicable" "yes")
> +   (set_attr "pool_range" "*,256")
> +   (set_attr "neg_pool_range" "*,244")]
>   )
>
>   (define_insn "*arm_zero_extendhisi2_v6"
> @@ -4660,8 +4662,10 @@ (define_insn "*arm_zero_extendhisi2_v6"
>     "@
>      uxth%?\\t%0, %1
>      ldr%(h%)\\t%0, %1"
> -  [(set_attr "predicable" "yes")
> -   (set_attr "type" "simple_alu_shift,load_byte")]
> +  [(set_attr "type" "simple_alu_shift,load_byte")
> +   (set_attr "predicable" "yes")
> +   (set_attr "pool_range" "*,256")
> +   (set_attr "neg_pool_range" "*,244")]
>   )
>
>   (define_insn "*arm_zero_extendhisi2addsi"
>

> Having half-word accesses into the minipool is generally a bad idea.
> The limited offset range that's supported by these instructions means
> it's much more likely that we'll end up with a pool after a conditional
> branch or, worse, in the middle of a linear code sequence.  That means
> we have to jump around the pool, which costs performance.

Note that this is -Os so performance considerations might come second.

> We really need to find out why the compiler keeps trying to create these
> and fix that problem rather than work around the issue.
> 
> I'm not sure why the v6 variants allow this; I thought that had been
> taken out.

Apparently not.