[ARMhf] gcc produces assembler it can't compile

[Snark]

I'm porting the sagemath software to ARM. I completed the port with ubuntu oneiric.

I upgraded to (a snapshot of) precise, and now a single file in the whole project doesn't build anymore.

Hence something has been broken from oneiric/armel to precise/armhf.

The attached mp.c is a "gcc -E" version of the problematic file, so it allows to easily reproduce the issue without fuss : if you compile it with "gcc -c -o mp.o mp.c -O3 -funroll-loops", then the assembler will find two cases of "branch out of range". Other switch combinations don't give problems, and the same combination is ok on x86_64 for example.

Ah, on precise the gcc version is the one of Fri, 20 Jan 2012 12:10:59 +100 uploaded by doko, and changelog "Merge with debian".

There's no problem giving more details or doing tests if you want.

[Snark (julien-puydt)]

The file compiles with "-O3 -funroll-loops -marm" (someone suggested I try)

[Tobin Davis (gruemaster)]

I can reproduce this on precise (armel & armhf).

ubuntu@panda2:~/test$ gcc -c -o mp.o mp.c -O3 -funroll-loops
/tmp/ccSVzO5N.s: Assembler messages:
/tmp/ccSVzO5N.s:202: Error: branch out of range
/tmp/ccSVzO5N.s:4145: Error: branch out of range

ubuntu@panda2:~/test$ gcc -c -o mp.o mp.c -O3 -funroll-loops -marm
ubuntu@panda2:~/test$

[Asa Sandahl (asa-sandahl)]

Thank you for the bug report. I've confirmed this with gcc-linaro-4.6-2012.01 on ARM:

asa-san@ursa4:~/bugs/mp$ /scratch/asa-san/cbuild/slaves/ursa4/gcc-linaro-4.6-2012.01/gcc-binary/bin/gcc -c -o mp.o mp.c -O3 -funroll-loops
/tmp/ccv0lfQ5.s: Assembler messages:
/tmp/ccv0lfQ5.s:116: Error: branch out of range
/tmp/ccv0lfQ5.s:343: Error: branch out of range

The work around is to compiler at -O1 or to remove the -funroll-loops flag.

The fault does not exist in FSF gcc-4.5.3 or gcc-4.6.2
Neither does it exist on gcc-linaro-4.5-2012.01.

The fault exists in gcc 4.7 (r183205).
The fault was introduced in gcc-linaro-4.6-2011.09.

I've set it to low priority as it is a ftbfs, there is a work-around,
and the fault exists upstream.

[Michael Hope (michaelh1)]

Asa, does the code contain any inline assembly? GCC estimates the size of each instruction and uses that to decide if it can use a short or a long branch. Long chunks of inline assembly can break that and make it think the code is shorter than it is.

[Asa Sandahl (asa-sandahl)]

Hi Michael,

I cant' find any

"asm"

chunks in there.
(Only in some external declarations.)

Can it look any other way?

/Åsa

On 15 February 2012 22:39, Michael Hope <email address hidden> wrote:

> Asa, does the code contain any inline assembly? GCC estimates the size
> of each instruction and uses that to decide if it can use a short or a
> long branch. Long chunks of inline assembly can break that and make it
> think the code is shorter than it is.
>
> --
> You received this bug notification because you are a member of Linaro
> Toolchain Developers, which is subscribed to Linaro GCC.
> https://bugs.launchpad.net/bugs/926855
>
> Title:
> [ARMhf] gcc produces assembler it can't compile
>
> Status in Linaro GCC:
> Triaged
> Status in “gcc-4.6” package in Ubuntu:
> Confirmed
>
> Bug description:
> I'm porting the sagemath software to ARM. I completed the port with
> ubuntu oneiric.
>
> I upgraded to (a snapshot of) precise, and now a single file in the
> whole project doesn't build anymore.
>
> Hence something has been broken from oneiric/armel to precise/armhf.
>
> The attached mp.c is a "gcc -E" version of the problematic file, so it
> allows to easily reproduce the issue without fuss : if you compile it
> with "gcc -c -o mp.o mp.c -O3 -funroll-loops", then the assembler will
> find two cases of "branch out of range". Other switch combinations
> don't give problems, and the same combination is ok on x86_64 for
> example.
>
> Ah, on precise the gcc version is the one of Fri, 20 Jan 2012 12:10:59
> +100 uploaded by doko, and changelog "Merge with debian".
>
> There's no problem giving more details or doing tests if you want.
>
> To manage notifications about this bug go to:
> https://bugs.launchpad.net/gcc-linaro/+bug/926855/+subscriptions
>

[Bernhard Rosenkraenzer (berolinux)]

Created attachment 26693
Preprocessed source triggering this issue

[bero@localhost ~]$ /opt/android-toolchain-4.7/bin/arm-linux-androideabi-g++ -mcpu=cortex-a9 -mthumb -Os -o agc2_amr_wb.o -c agc2_amr_wb.i
/tmp/ccBg0Qr8.s: Assembler messages:
/tmp/ccBg0Qr8.s:110: Error: branch out of range

The problem disappears when taking out -mcpu=cortex-a9 or replacing -Os with any other optimization level.

Reproducable with arm-linux-androideabi, arm-eabi and arm-linux-gnueabi compilers.

[Mikpe (mikpe)]

Fails with gcc 4.7 but works with 4.6, 4.5, and 4.4.

[Rearnsha (rearnsha)]

Confirmed.
lsls Rd, Rn, Rm
is only 2 bytes in size if Rd == Rn

Although the testcase only fails on trunk, the miscalculation is certain to be present on all maintained branches.

[stevenb (steven-gcc)]

Richard, I suppose you mean the problem is in this define_insn:

(define_insn "*thumb1_ashlsi3"
  [(set (match_operand:SI 0 "register_operand" "=l,l")
        (ashift:SI (match_operand:SI 1 "register_operand" "l,0")
                   (match_operand:SI 2 "nonmemory_operand" "N,l")))]
  "TARGET_THUMB1"
  "lsl\\t%0, %1, %2"
  [(set_attr "length" "2")
   (set_attr "conds" "set")])

which should set "length" depending on the operands?

(BTW when should ARM_LSL_NAME be used instead of "lsl"? Or is ARM_LSL_NAME not relevant for Thumb?)

[stevenb (steven-gcc)]

(If the pattern of comment #3 is to blame, then this goes back all the way to the check-in of that pattern, see http://gcc.gnu.org/viewcvs?view=revision&revision=33028)

[stevenb (steven-gcc)]

(In reply to comment #4)
> (If the pattern of comment #3 is to blame, then this ...

...probably fix it.

Index: arm.md
===================================================================
--- arm.md (revision 184318)
+++ arm.md (working copy)
@@ -3505,7 +3505,12 @@
      (match_operand:SI 2 "nonmemory_operand" "N,l")))]
   "TARGET_THUMB1"
   "lsl\\t%0, %1, %2"
- [(set_attr "length" "2")
+ [(set (attr "length")
+ (if_then_else
+ (eq (symbol_ref ("which_alternative"))
+ (const_int 0))
+ (const_int 4)
+ (const_int 2)))
    (set_attr "conds" "set")])

 (define_expand "ashrdi3"

And otherwise: sorry for all the noise :-)

[stevenb (steven-gcc)]

Or better:
Index: arm.md
===================================================================
--- arm.md (revision 184318)
+++ arm.md (working copy)
@@ -3505,7 +3505,7 @@
      (match_operand:SI 2 "nonmemory_operand" "N,l")))]
   "TARGET_THUMB1"
   "lsl\\t%0, %1, %2"
- [(set_attr "length" "2")
+ [(set_attr "length" "4,2")
    (set_attr "conds" "set")])

 (define_expand "ashrdi3"

[Rearnsha (rearnsha)]

(In reply to comment #3)
> Richard, I suppose you mean the problem is in this define_insn:
>
> (define_insn "*thumb1_ashlsi3"
> [(set (match_operand:SI 0 "register_operand" "=l,l")
> (ashift:SI (match_operand:SI 1 "register_operand" "l,0")
> (match_operand:SI 2 "nonmemory_operand" "N,l")))]
> "TARGET_THUMB1"
> "lsl\\t%0, %1, %2"
> [(set_attr "length" "2")
> (set_attr "conds" "set")])

No, that pattern is only for Thumb1, it never applies to Thumb2.

I'm currently testing a fix

[Rearnsha (rearnsha)]

Author: rearnsha
Date: Tue Feb 21 15:38:35 2012
New Revision: 184442

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=184442
Log:
 PR target/52294
 * thumb2.md (thumb2_shiftsi3_short): Split register and
 immediate shifts. For register shifts tie operands 0 and 1.
 (peephole2 for above): Check that register-controlled shifts
 have suitably tied operands.

Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/config/arm/thumb2.md

[Rearnsha (rearnsha)]

Author: rearnsha
Date: Tue Feb 21 23:46:05 2012
New Revision: 184452

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=184452
Log:
 PR target/52294
 * thumb2.md (thumb2_shiftsi3_short): Split register and
 immediate shifts. For register shifts tie operands 0 and 1.
 (peephole2 for above): Check that register-controlled shifts
 have suitably tied operands.

Modified:
    branches/gcc-4_6-branch/gcc/ChangeLog
    branches/gcc-4_6-branch/gcc/config/arm/thumb2.md

[Rearnsha (rearnsha)]

Author: rearnsha
Date: Tue Feb 21 23:51:16 2012
New Revision: 184454

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=184454
Log:
 PR target/52294
 * thumb2.md (thumb2_shiftsi3_short): Split register and
 immediate shifts. For register shifts tie operands 0 and 1.
 (peephole2 for above): Check that register-controlled shifts
 have suitably tied operands.

Modified:
    branches/gcc-4_5-branch/gcc/ChangeLog
    branches/gcc-4_5-branch/gcc/config/arm/thumb2.md

[Rearnsha (rearnsha)]

Fixed in 4.5, 4.6 and trunk

[Asa Sandahl (asa-sandahl)]

This ticket appears to be the same as:
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=52294.
a case where gcc generates a cbz instruction that ends up trying to reach too far.

Attaching the assembly file mp.s generated with:

asa-san@ursa4:~/bugs/mp$ /scratch/asa-san/cbuild/slaves/ursa4/gcc-linaro-4.6-2012.01/gcc-binary/bin/gcc -c -o mp.o mp.c -O3 -funroll-loops -save-temps -dp
mp.s: Assembler messages:
mp.s:116: Error: branch out of range
mp.s:343: Error: branch out of range

The patch by Richard Earnshaw affects the pattern *thumb2_shiftsi3_short in the backend.
Looking into mp.s, there are a lot of instructions that were generated from this pattern by the compiler.
lsls r3, r3, #2 @ 426 *thumb2_shiftsi3_short [length = 2]

The patch will become availabel in gcc-linaro-4.6 on the nextmerge from FSF trunk. and that should solve this issue.

[Matthias Klose (doko)]

fixed in 4.6.2-16ubuntu1

[Michael Hope (michaelh1)]

Assigning to Andrew as it will be picked up with the next merge.

[Snark (julien-puydt)]

I can confirm 4.6.2-16ubuntu1, uploaded Thu, 23 Feb 2012 19:39:11 +0100 fixes the problem : I can compile sage without compiler problems!

Thanks!

[Jye2 (jye2)]

Author: jye2
Date: Fri Jun 8 08:57:53 2012
New Revision: 188332

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=188332
Log:
2012-06-08 Joey Ye <email address hidden>

 Backport r184442 from mainline
 2012-02-21 Richard Earnshaw <email address hidden>

 PR target/52294
 * thumb2.md (thumb2_shiftsi3_short): Split register and
 immediate shifts. For register shifts tie operands 0 and 1.
 (peephole2 for above): Check that register-controlled shifts
 have suitably tied operands.

 Backport r183756 from mainline
 2012-01-31 Matthew Gretton-Dann <email address hidden>

 * config/arm/thumb2.md (thumb2_mov_notscc): Use MVN for true
 condition.

 Backport r183349 from mainline
 2012-01-20 Jakub Jelinek <email address hidden>

 PR target/51915
 * config/arm/arm.c (arm_count_output_move_double_insns): Call
 output_move_double on a copy of operands array.

 Backport r183095 from mainline
 2012-01-11 Matthew Gretton-Dann <email address hidden>

 * config/arm/arm.md (mov_notscc): Use MVN for false condition.

 Backport r182628 from mainline
 2011-12-21 Richard Earnshaw <email address hidden>

 PR target/51643
 * arm.c (arm_function_ok_for_sibcall): Use DECL_WEAK in previous
 change.

 Backport r182621 from mainline
 2011-12-21 Richard Earnshaw <email address hidden>

 PR target/51643
 * arm.c (arm_function_ok_for_sibcall): Don't try to tailcall a
 weak function on bare-metal EABI targets.

Testsuite:
 Backport r183349 from mainline
 2012-01-20 Jakub Jelinek <email address hidden>

 PR target/51915
 * gcc.target/arm/pr51915.c: New test.

 Backport r183095 from mainline
 2012-01-11 Matthew Gretton-Dann <email address hidden>

 * gcc.c-torture/execute/20120110-1.c: New testcase.

 Backport r182621 from mainline
 2011-12-21 Richard Earnshaw <email address hidden>

 PR target/51643
 * gcc.target/arm/sibcall-2.c: New test.

Added:
    branches/ARM/embedded-4_6-branch/gcc/testsuite/gcc.c-torture/execute/20120111-1.c
    branches/ARM/embedded-4_6-branch/gcc/testsuite/gcc.target/arm/pr51915.c
    branches/ARM/embedded-4_6-branch/gcc/testsuite/gcc.target/arm/sibcall-2.c
Modified:
    branches/ARM/embedded-4_6-branch/gcc/ChangeLog.arm
    branches/ARM/embedded-4_6-branch/gcc/config/arm/arm.c
    branches/ARM/embedded-4_6-branch/gcc/config/arm/arm.md
    branches/ARM/embedded-4_6-branch/gcc/config/arm/thumb2.md
    branches/ARM/embedded-4_6-branch/gcc/testsuite/ChangeLog.arm