[Bug target/59695] New: bad code generation on aarch64 in aarch64_output_mi_thunk

[Bug target/59695] New: bad code generation on aarch64 in aarch64_output_mi_thunk

[doko at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=59695

            Bug ID: 59695
           Summary: bad code generation on aarch64 in
                    aarch64_output_mi_thunk
           Product: gcc
           Version: 4.9.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: target
          Assignee: unassigned at gcc dot gnu.org
          Reporter: doko at gcc dot gnu.org

seen in a segfault running the tests in the coinor-osi package,
https://launchpad.net/bugs/1263576, both in saucy and trusty, version 0.106.4
and 0.106.5. Version 0.103 doesn't show the issue.

both the 4.7 and 4.8 linaro branches show this behaviour, and trunk 20131121
(didn't build a newer one yet).

William Grant tracked that down to a bug with very negative vcall_offsets in
aarch64 multiple inheritance thunks. The example below has two consecutive
thunks, with the second adding 263 instead of subtracting 264.
aarch64_build_constant seems to not handle negative integers. He tried a quick
gcc patch to avoid using aarch64_build_constant, and the coinor-osi tests
succeed.

0000000000401ca4 <_ZTv0_n256_N1C2adEv>:
  401ca4:       f9400010        ldr     x16, [x0]
  401ca8:       f8500211        ldr     x17, [x16,#-256]
  401cac:       8b110000        add     x0, x0, x17
  401cb0:       17fffff9        b       401c94 <_ZN1C2adEv>

[...]

0000000000401cc4 <_ZTv0_n264_N1C2aeEv>:
  401cc4:       f9400010        ldr     x16, [x0]
  401cc8:       d28020f1        mov     x17, #0x107                     // #263
  401ccc:       f8716a11        ldr     x17, [x16,x17]
  401cd0:       8b110000        add     x0, x0, x17
  401cd4:       17fffff8        b       401cb4 <_ZN1C2aeEv>

Any chance for a quick 2013 review?

Thanks, Matthias

--- a/gcc/config/aarch64/aarch64.c
+++ b/gcc/config/aarch64/aarch64.c
@@ -2540,8 +2540,8 @@
       addr = plus_constant (Pmode, temp0, vcall_offset);
       else
     {
-      aarch64_build_constant (IP1_REGNUM, vcall_offset);
-      addr = gen_rtx_PLUS (Pmode, temp0, temp1);
+      aarch64_add_constant (IP0_REGNUM, IP1_REGNUM, vcall_offset);
+      addr = temp0;
     }

       aarch64_emit_move (temp1, gen_rtx_MEM (Pmode,addr));

[Bug target/59695] bad code generation on aarch64 in aarch64_output_mi_thunk

[mgretton at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=59695

mgretton at gcc dot gnu.org changed:

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

--- Comment #1 from mgretton at gcc dot gnu.org ---
I think the actual issue is with the code in aarch64_build_constant:

      /* zcount contains the number of additional MOVK instructions
     required if the constant is built up with an initial MOVZ instruction,
     while ncount is the number of MOVK instructions required if starting
     with a MOVN instruction.  Choose the sequence that yields the fewest
     number of instructions, preferring MOVZ instructions when they are both
     the same.  */
      if (ncount < zcount)
    {
      emit_move_insn (gen_rtx_REG (Pmode, regnum),
              GEN_INT ((~val) & 0xffff));
      tval = 0xffff;
    }
      else
    {
      emit_move_insn (gen_rtx_REG (Pmode, regnum),
              GEN_INT (val & 0xffff));
      tval = 0;
    }

The GEN_INT in the first if branch is incorrect as it truncates the immediate
at 16-bits, and so we will never generate the "MOVN" form.  What it should be
instead is: GEN_INT (~((~val) & 0xffff)) or equivalent.

[Bug target/59695] bad code generation on aarch64 in aarch64_output_mi_thunk

[clyon at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=59695

--- Comment #3 from clyon at gcc dot gnu.org ---
Author: clyon
Date: Fri Jan 17 11:05:04 2014
New Revision: 206703

URL: http://gcc.gnu.org/viewcvs?rev=206703&root=gcc&view=rev
Log:
2014-01-17  Kugan Vivekanandarajah  <kuganv@linaro.org>

    gcc/
    Backport from mainline
    2014-01-15  Matthew Gretton-Dann  <matthew.gretton-dann@linaro.org>
            Kugan Vivekanandarajah  <kuganv@linaro.org>

    PR target/59695
    * config/aarch64/aarch64.c (aarch64_build_constant): Fix incorrect
    truncation.

    gcc/testsuite/
    Backport from mainline
    2014-01-15  Matthew Gretton-Dann  <matthew.gretton-dann@linaro.org>
        Kugan Vivekanandarajah  <kuganv@linaro.org>

    PR target/59695
    * g++.dg/pr59695.C: New testcase.


Added:
    branches/gcc-4_8-branch/gcc/testsuite/g++.dg/pr59695.C
      - copied unchanged from r206628, trunk/gcc/testsuite/g++.dg/pr59695.C
Modified:
    branches/gcc-4_8-branch/   (props changed)
    branches/gcc-4_8-branch/gcc/ChangeLog
    branches/gcc-4_8-branch/gcc/config/aarch64/aarch64.c
    branches/gcc-4_8-branch/gcc/testsuite/ChangeLog

Propchange: branches/gcc-4_8-branch/
            ('svn:mergeinfo' modified)

[Bug target/59695] bad code generation on aarch64 in aarch64_output_mi_thunk

[ramana at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=59695

Ramana Radhakrishnan <ramana at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |RESOLVED
                 CC|                            |ramana at gcc dot gnu.org
            Version|4.9.0                       |4.8.0
         Resolution|---                         |FIXED
   Target Milestone|---                         |4.8.3

--- Comment #4 from Ramana Radhakrishnan <ramana at gcc dot gnu.org> ---
Fixed now presumably.