Bug 69124 - [5/6 Regression] wrong code in thumb mode on arm-linux-gnueabihf
Summary: [5/6 Regression] wrong code in thumb mode on arm-linux-gnueabihf
Status: RESOLVED DUPLICATE of bug 69447
Alias: None
Product: gcc
Classification: Unclassified
Component: rtl-optimization (show other bugs)
Version: 5.3.1
: P2 normal
Target Milestone: 5.4
Assignee: Not yet assigned to anyone
URL:
Keywords: wrong-code
Depends on:
Blocks:
 
Reported: 2016-01-03 15:19 UTC by Sebastian Andrzej Siewior
Modified: 2016-01-28 10:03 UTC (History)
4 users (show)

See Also:
Host:
Target: arm-linux-gnueabihf
Build:
Known to work: 4.9.3
Known to fail: 5.3.1, 6.0
Last reconfirmed: 2016-01-05 00:00:00


Attachments
test case, fp_mul_comba_32.c (5.62 KB, text/x-csrc)
2016-01-03 15:19 UTC, Sebastian Andrzej Siewior
Details
-E output of the test case (14.72 KB, text/plain)
2016-01-03 15:20 UTC, Sebastian Andrzej Siewior
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Sebastian Andrzej Siewior 2016-01-03 15:19:39 UTC
Created attachment 37213 [details]
test case, fp_mul_comba_32.c

Attached a testcase that miss compiles on ARM since gcc-5 (Debian gcc, built with --with-arch=armv7-a --with-fpu=vfpv3-d16 --with-float=hard).

gcc -o fp_mul fp_mul_comba_32.c -O2 -Wall -Wextra -fno-strict-aliasing -fwrapv -fno-aggressive-loop-optimizations

./fp_mul
27 0x0a7d8ff8 != 0xce804437
28 0x1c8a1f70 != 0x1c8a1f6f
-> 3

I don't see this with problem with -O1, or arm-linux-gnueabi target (i.e. no output). 

The bug does not trigger with -fsanitize=undefined but it does not show any output.
Comment 1 Sebastian Andrzej Siewior 2016-01-03 15:20:26 UTC
Created attachment 37214 [details]
-E output of the test case
Comment 2 Mikael Pettersson 2016-01-04 08:53:25 UTC
I can't reproduce this with either gcc-6-20151227 or gcc-5-20151229 on hard-float armv7l (--with-arch=armv7-a --with-tune=cortex-a9 --with-float=hard --with-fpu=vfpv3-d16 configure options).

Debian is known to heavily modify their GCC sources.  Unless you can reproduce this with a GCC built from vanilla upstream sources I suggest you report this problem to Debian.
Comment 3 Sebastian Andrzej Siewior 2016-01-04 14:59:34 UTC
gcc -v
Target: arm-linux-gnueabihf
Configured with: ../src/configure -v --with-pkgversion='Debian 5.3.1-4' --with-bugurl=file:///usr/share/doc/gcc-5/README.Bugs --enable-languages=c,ada,c++,java,go,d,fortran,objc,obj-c++ --prefix=/usr --program-suffix=-5 --enable-shared --enable-linker-build-id --libexecdir=/usr/lib --without-included-gettext --enable-threads=posix --libdir=/usr/lib --enable-nls --with-sysroot=/ --enable-clocale=gnu --enable-libstdcxx-debug --enable-libstdcxx-time=yes --with-default-libstdcxx-abi=new --enable-gnu-unique-object --disable-libitm --disable-libquadmath --enable-plugin --with-system-zlib --disable-browser-plugin --enable-java-awt=gtk --enable-gtk-cairo --with-java-home=/usr/lib/jvm/java-1.5.0-gcj-5-armhf/jre --enable-java-home --with-jvm-root-dir=/usr/lib/jvm/java-1.5.0-gcj-5-armhf --with-jvm-jar-dir=/usr/lib/jvm-exports/java-1.5.0-gcj-5-armhf --with-arch-directory=arm --with-ecj-jar=/usr/share/java/eclipse-ecj.jar --enable-objc-gc --enable-multiarch --disable-sjlj-exceptions --with-arch=armv7-a --with-fpu=vfpv3-d16 --with-float=hard --with-mode=thumb --enable-checking=release --build=arm-linux-gnueabihf --host=arm-linux-gnueabihf --target=arm-linux-gnueabihf
Thread model: posix
gcc version 5.3.1 20151219 (Debian 5.3.1-4) 

gcc -o fp_mul fp_mul_comba_32.c -g -O2 -march=armv7-a -mtune=cortex-a9 -mhard-float -mfpu=vfpv3-d16
gcc -o fp_mul-nt fp_mul_comba_32.c -g -O2 -march=armv7-a -mhard-float -mfpu=vfpv3-d16

(sid_armhf-dchroot)bigeasy@harris:~/tc$ ./fp_mul
(sid_armhf-dchroot)bigeasy@harris:~/tc$ ./fp_mul-nt 
27 0x0a7d8ff8 != 0xce804437
28 0x1c8a1f70 != 0x1c8a1f6f
-> 3

So it does not trigger here with -mtune=cortex-a9 as well. Do you know what is selected for -mtune as default if it is not specified at build-time? I tried with cortex-a7 but it does not trigger :/
Comment 4 Richard Earnshaw 2016-01-04 15:09:52 UTC
Does -fno-strict-aliasing help?
Comment 5 Mikael Pettersson 2016-01-04 15:27:34 UTC
The OP's compiler has --with-mode=thumb!  If I compile with -mtune=generic-armv7-a -mthumb then I see the following errors from the test case:

12 0xc0b165f1 != 0xdf5e0cae
13 0x8b329fe4 != 0x8b329fe3
19 0x841bee4e != 0x7b21ac34
20 0x2563cc91 != 0x84be291e
21 0x5f503954 != 0xb8f7b936
22 0x36cb8e86 != 0x36cb8e85
31 0x8c99a1f4 != 0x915bd91c
32 0x22e09d96 != 0x22e09d95
-> 3

This is with -fno-strict-aliasing etc as per the initial description, and both gcc 5 and 6 show this behaviour.
Comment 6 Matthias Klose 2016-01-05 14:42:40 UTC
$ gcc -g -O2 -march=armv7-a -mfloat-abi=hard -mfpu=vfpv3-d16 -mthumb fp_mul_comba_32.c && ./a.out 
27 0x0a7d8ff8 != 0xce804437
28 0x1c8a1f70 != 0x1c8a1f6f
-> 3

works with -marm.
Comment 7 Matthias Klose 2016-01-05 14:44:02 UTC
> Debian is known to heavily modify their GCC sources.

While Debian applies patches, these are almost all not code-modifying patches, just the release plus updates from the release branches.
Comment 8 ktkachov 2016-01-05 15:28:58 UTC
Confirmed as well.
Would be nice to have a smaller testcase though,
or a bisect to the offending commit so we can have a sane look at before/after codegen and compiler dumps
Comment 9 ktkachov 2016-01-05 17:36:30 UTC
So I did a bisection between 4.9 and 5.
The offending commit is r217624:
Author: vmakarov <vmakarov@138bc75d-0d04-0410-961f-82ee72b054a4>
Date:   Sun Nov 16 05:00:30 2014 +0000

    2014-11-15  Vladimir Makarov  <vmakarov@redhat.com>
    
    	* lra-remat.c (cand_transf_func): Process regno for
    	rematerialization too.
    	* lra.c (lra): Switch on rematerialization pass.

i.e. when the LRA rematiralisation pass was turned on.
Comment 10 ktkachov 2016-01-05 17:53:18 UTC
(In reply to ktkachov from comment #9)
> So I did a bisection between 4.9 and 5.
> The offending commit is r217624:
> Author: vmakarov <vmakarov@138bc75d-0d04-0410-961f-82ee72b054a4>
> Date:   Sun Nov 16 05:00:30 2014 +0000
> 
>     2014-11-15  Vladimir Makarov  <vmakarov@redhat.com>
>     
>     	* lra-remat.c (cand_transf_func): Process regno for
>     	rematerialization too.
>     	* lra.c (lra): Switch on rematerialization pass.
> 
> i.e. when the LRA rematiralisation pass was turned on.

Consequently, compiling with -fno-lra-remat the testcase runs successfully (no output)
Comment 11 Mikael Pettersson 2016-01-05 18:04:59 UTC
(In reply to ktkachov from comment #9)
> So I did a bisection between 4.9 and 5.
> The offending commit is r217624:
> Author: vmakarov <vmakarov@138bc75d-0d04-0410-961f-82ee72b054a4>
> Date:   Sun Nov 16 05:00:30 2014 +0000
> 
>     2014-11-15  Vladimir Makarov  <vmakarov@redhat.com>
>     
>     	* lra-remat.c (cand_transf_func): Process regno for
>     	rematerialization too.
>     	* lra.c (lra): Switch on rematerialization pass.
> 
> i.e. when the LRA rematiralisation pass was turned on.

Strange, my bisect identified r217588 as the culprit, although it also talks about lra materialization:

2014-11-14  Vladimir Makarov  <vmakarov@redhat.com>

       * lra-int.h (lra_create_live_ranges): Add parameter.
       * lra-lives.c (temp_bitmap): Move higher.
       (initiate_live_solver): Move temp_bitmap initialization into
       lra_live_ranges_init.
       (finish_live_solver): Move temp_bitmap clearing into
       live_ranges_finish.
       (process_bb_lives): Add parameter.  Use it to control live info
       update and dead insn elimination.  Pass it to mark_regno_live and
       mark_regno_dead.
       (lra_create_live_ranges): Add parameter.  Pass it to
       process_bb_lives.
       (lra_live_ranges_init, lra_live_ranges_finish): See changes in
       initiate_live_solver and finish_live_solver.
       * lra-remat.c (do_remat): Process insn non-operand hard regs too.
       Use temp_bitmap to update avail_cands.
       * lra.c (lra): Pass new parameter to lra_create_live_ranges.  Move
       check with lra_need_for_spill_p after live range pass.  Switch on
       rematerialization pass.
Comment 12 ktkachov 2016-01-28 10:03:37 UTC
Marking as dup of PR 69447. That one has a more manageable testcase and the LRA remat fix for that fixes the wrong-code here

*** This bug has been marked as a duplicate of bug 69447 ***