[Patch, ARM] Fix pr 61208

[Patch, ARM] Fix pr 61208

[Richard Earnshaw]

[-- Attachment #1: Type: text/plain, Size: 1110 bytes --]

PR 61208 is a wrong code bug in Thumb2 where we can generate out of
range branches due to incorrect instruction size calculations.  It's
mostly gone latent on 4.9 and trunk (though could still happen at -O0
where splitting is done during final instruction generation).

Complicating things slightly is the fact that prior to 4.9 we had a
single alternative that was emitted statically; we now use an
insn_and_split (hence the reduced impact on 4.9).  That means different
patches are needed: one for 4.9 and trunk; another for 4.8 and earlier.

I've tried to minimize the 4.8 version as much as possible to reduce the
risk.  It seems simplest to just provide a new alternative for use with
Thumb2 that has the correct length.  Consequently there are two versions
of this patch attached.  One for 4.7 and 4.8, the other for 4.9 and trunk.

	PR target/61208
	* arm.md (arm_cmpdi_unsigned): Fix length calculation for Thumb2.

Testing of the backport version on 4.8 is OK, the trunk version is
ongoing, though I've smoke tested it.

Richi, is the backport version OK to go into 4.8?


R.

[-- Attachment #2: pr61208.patch --]
[-- Type: text/plain, Size: 1162 bytes --]

Index: gcc/config/arm/arm.md
===================================================================
--- gcc/config/arm/arm.md	(revision 210617)
+++ gcc/config/arm/arm.md	(working copy)
@@ -8371,8 +8371,8 @@ (define_insn_and_split "*arm_cmpdi_insn"
 
 (define_insn_and_split "*arm_cmpdi_unsigned"
   [(set (reg:CC_CZ CC_REGNUM)
-        (compare:CC_CZ (match_operand:DI 0 "s_register_operand" "l,r,r")
-                       (match_operand:DI 1 "arm_di_operand"     "Py,r,rDi")))]
+        (compare:CC_CZ (match_operand:DI 0 "s_register_operand" "l,r,r,r")
+                       (match_operand:DI 1 "arm_di_operand"     "Py,r,Di,rDi")))]
 
   "TARGET_32BIT"
   "#"   ; "cmp\\t%R0, %R1\;it eq\;cmpeq\\t%Q0, %Q1"
@@ -8392,9 +8392,9 @@ (define_insn_and_split "*arm_cmpdi_unsig
     operands[1] = gen_lowpart (SImode, operands[1]);
   }
   [(set_attr "conds" "set")
-   (set_attr "enabled_for_depr_it" "yes,yes,no")
-   (set_attr "arch" "t2,t2,*")
-   (set_attr "length" "6,6,8")
+   (set_attr "enabled_for_depr_it" "yes,yes,no,*")
+   (set_attr "arch" "t2,t2,t2,a")
+   (set_attr "length" "6,6,10,8")
    (set_attr "type" "multiple")]
 )
 

[-- Attachment #3: pr61208-backport.patch --]
[-- Type: text/plain, Size: 684 bytes --]

--- gcc/config/arm/arm.md	(revision 210631)
+++ gcc/config/arm/arm.md	(local)
@@ -7630,12 +7630,13 @@ (define_insn "*arm_cmpdi_insn"
 
 (define_insn "*arm_cmpdi_unsigned"
   [(set (reg:CC_CZ CC_REGNUM)
-	(compare:CC_CZ (match_operand:DI 0 "s_register_operand" "r")
-		       (match_operand:DI 1 "arm_di_operand"	"rDi")))]
+	(compare:CC_CZ (match_operand:DI 0 "s_register_operand" "r,r")
+		       (match_operand:DI 1 "arm_di_operand"	"rDi,rDi")))]
   "TARGET_32BIT"
   "cmp\\t%R0, %R1\;it eq\;cmpeq\\t%Q0, %Q1"
   [(set_attr "conds" "set")
-   (set_attr "length" "8")]
+   (set_attr "arch" "a,t2")
+   (set_attr "length" "8,10")]
 )
 
 (define_insn "*arm_cmpdi_zero"

Re: [Patch, ARM] Fix pr 61208

[Richard Biener]

On Tue, 20 May 2014, Richard Earnshaw wrote:

> PR 61208 is a wrong code bug in Thumb2 where we can generate out of
> range branches due to incorrect instruction size calculations.  It's
> mostly gone latent on 4.9 and trunk (though could still happen at -O0
> where splitting is done during final instruction generation).
> 
> Complicating things slightly is the fact that prior to 4.9 we had a
> single alternative that was emitted statically; we now use an
> insn_and_split (hence the reduced impact on 4.9).  That means different
> patches are needed: one for 4.9 and trunk; another for 4.8 and earlier.
> 
> I've tried to minimize the 4.8 version as much as possible to reduce the
> risk.  It seems simplest to just provide a new alternative for use with
> Thumb2 that has the correct length.  Consequently there are two versions
> of this patch attached.  One for 4.7 and 4.8, the other for 4.9 and trunk.
> 
> 	PR target/61208
> 	* arm.md (arm_cmpdi_unsigned): Fix length calculation for Thumb2.
> 
> Testing of the backport version on 4.8 is OK, the trunk version is
> ongoing, though I've smoke tested it.
> 
> Richi, is the backport version OK to go into 4.8?

Please wait until after 4.8.3 is released (the bug is not marked
as regression and has no known-to-work versions filled in).

Richard.