Re: [ARM] Fix ICE in minipool handling at -Os

2013-03-27 Thread Eric Botcazou
> 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.

-- 
Eric Botcazou


Re: [ARM] Fix ICE in minipool handling at -Os

2013-03-25 Thread Richard Earnshaw

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  

* 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"