[Bug target/64304] New: AArch64 miscompilation with -mgeneral-regs-only

[Bug target/64304] New: AArch64 miscompilation with -mgeneral-regs-only

[me at williamgrant dot id.au]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=64304

            Bug ID: 64304
           Summary: AArch64 miscompilation with -mgeneral-regs-only
           Product: gcc
           Version: unknown
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: target
          Assignee: unassigned at gcc dot gnu.org
          Reporter: me at williamgrant dot id.au

The aarch64 target, from 4.8.0 to today's master (tested up to 4b686186), drops
an "and x0, x0, #0x7" in some integer-only code when built with
-mgeneral-regs-only. Ignoring register assignment, the only change in the
resultant code is the missing and. It works fine without -mgeneral-regs-only
and on several other targets.

This test case is quite fragile. The original code didn't need -fno-inline to
reproduce it, and the problem disappeared in an earlier test with
-fno-tree-vrp. -fdump-rtl-all reveals that the adddi vanishes between ud_dce
and combine, but I can't see any significant differences in the input RTL to
explain that.

$ gcc -mgeneral-regs-only -O2 -fno-inline -o gen-test gen-test.c
$ ./gen-test && echo success || echo fail
fail
$ gcc -O2 -fno-inline -o gen-test gen-test.c
$ ./gen-test && echo success || echo fail
success

----
unsigned char byte = 0;

void set_bit(unsigned int bit, unsigned char value) {
    unsigned char mask = (unsigned char)(1 << (bit & 7));
    if (!value) {
        byte &= (unsigned char)~mask;
    } else {
        byte |= mask;
    }
}

int main(int argc, char **argv) {
    set_bit(8, 1);
    set_bit(9, 1);
    return byte != 3;
}
----

[Bug target/64304] AArch64 miscompilation with -mgeneral-regs-only

[kugan at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=64304

kugan at gcc dot gnu.org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |kugan at gcc dot gnu.org

--- Comment #1 from kugan at gcc dot gnu.org ---
I haven't looked in detail but and:SI is gone during the combine stage. For
some reason, combine thinks it is OK to  remove and:SI.

Here is the .213r.ud_dce  and .214r.combine when it fails.


 (note 5 0 2 2 [bb 2] NOTE_INSN_BASIC_BLOCK)
-(insn 2 5 3 2 (set (reg/v:SI 82 [ bit ])
-        (reg:SI 0 x0 [ bit ])) bug.c:3 38 {*movsi_aarch64}
-     (expr_list:REG_DEAD (reg:SI 0 x0 [ bit ])
-        (nil)))
+(note 2 5 3 2 NOTE_INSN_DELETED)
 (insn 3 2 4 2 (set (reg/v:SI 83 [ value ])
         (zero_extend:SI (reg:QI 1 x1 [ value ]))) bug.c:3 76
{*zero_extendqisi2_aarch64}
      (expr_list:REG_DEAD (reg:QI 1 x1 [ value ])
         (nil)))
 (note 4 3 7 2 NOTE_INSN_FUNCTION_BEG)
-(insn 7 4 8 2 (set (reg:SI 84 [ D.1492 ])
-        (and:SI (reg/v:SI 82 [ bit ])
-            (const_int 7 [0x7]))) bug.c:4 399 {andsi3}
-     (expr_list:REG_DEAD (reg/v:SI 82 [ bit ])
-        (nil)))
+(note 7 4 8 2 NOTE_INSN_DELETED)
 (insn 8 7 9 2 (set (reg:SI 86)
         (const_int 1 [0x1])) bug.c:4 38 {*movsi_aarch64}
      (nil))
-(insn 9 8 10 2 (set (reg:SI 85 [ D.1492 ])
-        (ashift:SI (reg:SI 86)
-            (subreg:QI (reg:SI 84 [ D.1492 ]) 0))) bug.c:4 539
{*aarch64_ashl_sisd_or_int_si3}
-     (expr_list:REG_DEAD (reg:SI 86)
-        (expr_list:REG_DEAD (reg:SI 84 [ D.1492 ])
-            (expr_list:REG_EQUAL (ashift:SI (const_int 1 [0x1])
-                    (subreg:QI (reg:SI 84 [ D.1492 ]) 0))
-                (nil)))))
+(insn 9 8 10 2 (set (reg:QI 85 [ D.1492 ])
+        (ashift:QI (subreg:QI (reg:SI 86) 0)
+            (reg:QI 0 x0 [ bit ]))) bug.c:4 556 {*ashlqi3_insn}
+     (expr_list:REG_DEAD (reg:SI 0 x0 [ bit ])
+        (expr_list:REG_DEAD (reg:SI 86)
+            (nil))))
 (insn 10 9 11 2 (set (reg/v:SI 76 [ mask ])
-        (zero_extend:SI (subreg:QI (reg:SI 85 [ D.1492 ]) 0))) bug.c:4 76
{*zero_extendqisi2_aarch64}
-     (expr_list:REG_DEAD (reg:SI 85 [ D.1492 ])
-        (nil)))
-(insn 11 10 12 2 (set (reg:CC 66 cc)
-        (compare:CC (reg/v:SI 83 [ value ])
-            (const_int 0 [0]))) bug.c:5 330 {*cmpsi}
-     (expr_list:REG_DEAD (reg/v:SI 83 [ value ])
+        (zero_extend:SI (reg:QI 85 [ D.1492 ]))) bug.c:4 76
{*zero_extendqisi2_aarch64}
+     (expr_list:REG_DEAD (reg:QI 85 [ D.1492 ])
         (nil)))
.....

[Bug rtl-optimization/64304] AArch64 miscompilation with -mgeneral-regs-only

[jiwang at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=64304

Jiong Wang <jiwang at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |ASSIGNED
   Last reconfirmed|                            |2015-01-06
     Ever confirmed|0                           |1

--- Comment #2 from Jiong Wang <jiwang at gcc dot gnu.org> ---
a quick looks shows it's related with SHIFT_COUNT_TRUNCATED.

looks like generic code, especially combine pass, is with wrong usage of
SHIFT_COUNT_TRUNCATED in some situations.

[Bug rtl-optimization/64304] AArch64 miscompilation with -mgeneral-regs-only

[jiwang at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=64304

--- Comment #3 from Jiong Wang <jiwang at gcc dot gnu.org> ---
cased by one bug in combine pass.

patch under review at https://gcc.gnu.org/ml/gcc-patches/2015-01/msg00508.html

[Bug target/64304] AArch64 miscompilation with -mgeneral-regs-only

[pinskia at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=64304

Andrew Pinski <pinskia at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
          Component|rtl-optimization            |target

--- Comment #4 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
With ashift:QI pattern, the shifter is defined as truncated with
SHIFT_COUNT_TRUNCATED turned on.  But the instruction which the back-end uses
does not have that property so it is the back-end lying to the middle-end
again.  So a target bug.

[Bug target/64304] AArch64 miscompilation with -mgeneral-regs-only

[jiwang at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=64304

--- Comment #5 from Jiong Wang <jiwang at gcc dot gnu.org> ---
Author: jiwang
Date: Mon Jan 19 14:13:33 2015
New Revision: 219844

URL: https://gcc.gnu.org/viewcvs?rev=219844&root=gcc&view=rev
Log:
[AArch64] Remove ashift pattern for QI/HI

2015-01-19  Jiong Wang  <jiong.wang@arm.com>
        Andrew Pinski  <apinski@cavium.com>

  gcc/
    PR target/64304
    * config/aarch64/aarch64.md (define_insn "*ashl<mode>3_insn"): Deleted.
    (ashl<mode>3): Don't expand if operands[2] is not constant.

  gcc/testsuite/
    * gcc.target/aarch64/pr64304.c: New testcase.


Added:
    trunk/gcc/testsuite/gcc.target/aarch64/pr64304.c
Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/config/aarch64/aarch64.md
    trunk/gcc/testsuite/ChangeLog

[Bug target/64304] AArch64 miscompilation with -mgeneral-regs-only

[jiwang at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=64304

Jiong Wang <jiwang at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|ASSIGNED                    |RESOLVED
         Resolution|---                         |FIXED

--- Comment #6 from Jiong Wang <jiwang at gcc dot gnu.org> ---
mark as fixed.

[Bug target/64304] AArch64 miscompilation with -mgeneral-regs-only

[fyang at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=64304

--- Comment #7 from fyang at gcc dot gnu.org ---
Author: fyang
Date: Tue May  5 15:50:18 2015
New Revision: 222812

URL: https://gcc.gnu.org/viewcvs?rev=222812&root=gcc&view=rev
Log:
        Backported from mainline
        2015-01-19  Jiong Wang  <jiong.wang@arm.com>
                    Andrew Pinski  <apinski@cavium.com>

        PR target/64304
        * config/aarch64/aarch64.md (define_insn "*ashl<mode>3_insn"): Deleted.
        (ashl<mode>3): Don't expand if operands[2] is not constant.

Added:
    branches/gcc-4_9-branch/gcc/testsuite/gcc.target/aarch64/pr64304.c
Modified:
    branches/gcc-4_9-branch/gcc/ChangeLog
    branches/gcc-4_9-branch/gcc/config/aarch64/aarch64.md
    branches/gcc-4_9-branch/gcc/testsuite/ChangeLog

[Bug target/64304] AArch64 miscompilation with -mgeneral-regs-only

[fyang at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=64304

--- Comment #8 from fyang at gcc dot gnu.org ---
Author: fyang
Date: Tue May  5 15:59:12 2015
New Revision: 222814

URL: https://gcc.gnu.org/viewcvs?rev=222814&root=gcc&view=rev
Log:
        Backported from mainline
        2015-01-19  Jiong Wang  <jiong.wang@arm.com>
                    Andrew Pinski  <apinski@cavium.com>

        PR target/64304
        * config/aarch64/aarch64.md (define_insn "*ashl<mode>3_insn"): Deleted.
        (ashl<mode>3): Don't expand if operands[2] is not constant.

Added:
    branches/gcc-4_8-branch/gcc/testsuite/gcc.target/aarch64/pr64304.c
Modified:
    branches/gcc-4_8-branch/gcc/ChangeLog
    branches/gcc-4_8-branch/gcc/config/aarch64/aarch64.md
    branches/gcc-4_8-branch/gcc/testsuite/ChangeLog