[Bug target/60580] New: aarch64 generates wrong code for __attribute__ ((optimize("no-omit-frame-pointer")))

[Bug target/60580] New: aarch64 generates wrong code for __attribute__ ((optimize("no-omit-frame-pointer")))

[chemobejk at gmail dot com]

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

            Bug ID: 60580
           Summary: aarch64 generates wrong code for __attribute__
                    ((optimize("no-omit-frame-pointer")))
           Product: gcc
           Version: 4.8.3
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: target
          Assignee: unassigned at gcc dot gnu.org
          Reporter: chemobejk at gmail dot com

Created attachment 32388
  --> http://gcc.gnu.org/bugzilla/attachment.cgi?id=32388&action=edit
Example C code used i the command lines

We have a piece of code that we compile with -fomit-frame-pointer. The
exception is one low-level function that *MUST NOT* rely on the SP register,
i.e. that one function must be compiled with -fno-omit-frame-pointer. This
works fine with the GCC ARM backend, i.e. we have in the C code the following
function attribute:

 /*
  * This function calls XXX(), which modifies SP. This is incompatible to
  * -fomit-frame-pointer generated code as SP is used to access the frame.
  */
 __attribute__ ((optimize("no-omit-frame-pointer")))
 void some_low_level_function(....)


We are now trying to compile that piece of code with the aarch64 backend and
get incorrect code that uses SP to access the frame. I tried with the following
pre-compiled aarch64 toolchains:


Linaro release 2014.02:
COLLECT_GCC=gcc-linaro-aarch64-linux-gnu-4.8-2014.02_linux/bin/aarch64-linux-gnu-gcc
COLLECT_LTO_WRAPPER=/workarea/stefanb/playground/aarch64/gcc-linaro-aarch64-linux-gnu-4.8-2014.02_linux/bin/../libexec/gcc/aarch64-linux-gnu/4.8.3/lto-wrapper
Target: aarch64-linux-gnu
Configured with:
/cbuild/slaves/oorts/crosstool-ng/builds/aarch64-linux-gnu-linux/.build/src/gcc-linaro-4.8-2014.02/configure
--build=i686-build_pc-linux-gnu --host=i686-build_pc-linux-gnu
--target=aarch64-linux-gnu
--prefix=/cbuild/slaves/oorts/crosstool-ng/builds/aarch64-linux-gnu-linux/install
--with-sysroot=/cbuild/slaves/oorts/crosstool-ng/builds/aarch64-linux-gnu-linux/install/aarch64-linux-gnu/libc
--enable-languages=c,c++,fortran --disable-multilib --enable-multiarch
--with-arch=armv8-a --with-pkgversion='crosstool-NG linaro-1.13.1-4.8-2014.02 -
Linaro GCC 2014.02' --with-bugurl=https://bugs.launchpad.net/gcc-linaro
--enable-__cxa_atexit --disable-libmudflap --enable-libgomp --disable-libssp
--with-gmp=/cbuild/slaves/oorts/crosstool-ng/builds/aarch64-linux-gnu-linux/.build/aarch64-linux-gnu/build/static
--with-mpfr=/cbuild/slaves/oorts/crosstool-ng/builds/aarch64-linux-gnu-linux/.build/aarch64-linux-gnu/build/static
--with-mpc=/cbuild/slaves/oorts/crosstool-ng/builds/aarch64-linux-gnu-linux/.build/aarch64-linux-gnu/build/static
--with-isl=/cbuild/slaves/oorts/crosstool-ng/builds/aarch64-linux-gnu-linux/.build/aarch64-linux-gnu/build/static
--with-cloog=/cbuild/slaves/oorts/crosstool-ng/builds/aarch64-linux-gnu-linux/.build/aarch64-linux-gnu/build/static
--with-libelf=/cbuild/slaves/oorts/crosstool-ng/builds/aarch64-linux-gnu-linux/.build/aarch64-linux-gnu/build/static
--enable-threads=posix --disable-libstdcxx-pch --enable-linker-build-id
--enable-plugin
--with-local-prefix=/cbuild/slaves/oorts/crosstool-ng/builds/aarch64-linux-gnu-linux/install/aarch64-linux-gnu/libc
--enable-c99 --enable-long-long
Thread model: posix
gcc version 4.8.3 20140203 (prerelease) (crosstool-NG linaro-1.13.1-4.8-2014.02
- Linaro GCC 2014.02)


Android AOSP:

COLLECT_GCC=prebuilts/gcc/linux-x86/aarch64/aarch64-linux-android-4.8/bin/aarch64-linux-android-gcc 
COLLECT_LTO_WRAPPER=/workarea/stefanb/repos/tmake-only/prebuilts/gcc/linux-x86/aarch64/aarch64-linux-android-4.8/bin/../libexec/gcc/aarch64-linux-android/4.8/lto-wrapper 
Target: aarch64-linux-android                                                   
Configured with: /tmp/AOSP-toolchain/build/../gcc/gcc-4.8/configure
--prefix=/tmp/toolchain-build-aarch64-linux-4.8/prefix
--target=aarch64-linux-android --host=x86_64-linux-gnu --build=x86_64-linux-gnu
--with-gnu-as --with-gnu-ld --enable-languages=c,c++
--with-gmp=/tmp/toolchain-build-aarch64-linux-4.8/temp-install
--with-mpfr=/tmp/toolchain-build-aarch64-linux-4.8/temp-install
--with-mpc=/tmp/toolchain-build-aarch64-linux-4.8/temp-install
--with-cloog=/tmp/toolchain-build-aarch64-linux-4.8/temp-install
--with-isl=/tmp/toolchain-build-aarch64-linux-4.8/temp-install
--with-ppl=/tmp/toolchain-build-aarch64-linux-4.8/temp-install
--disable-ppl-version-check --disable-cloog-version-check
--disable-isl-version-check --enable-cloog-backend=isl
--with-host-libstdcxx='-static-libgcc -Wl,-Bstatic,-lstdc++,-Bdynamic -lm'
--disable-libssp --enable-threads --disable-nls --disable-libmudflap
--disable-libgomp --disable-libstdc__-v3 --disable-sjlj-exceptions
--disable-shared --disable-tls --disable-libitm --enable-initfini-array
--disable-nls --prefix=/tmp/toolchain-build-aarch64-linux-4.8/prefix
--with-sysroot=/tmp/toolchain-build-aarch64-linux-4.8/prefix/sysroot
--with-binutils-version=2.23 --with-mpfr-version=3.1.1 --with-mpc-version=1.0.1
--with-gmp-version=5.0.5 --with-gcc-version=4.8 --with-gdb-version=7.6
--with-gxx-include-dir=/tmp/toolchain-build-aarch64-linux-4.8/prefix/include/c++/4.8
--with-bugurl=http://source.android.com/source/report-bugs.html
--disable-bootstrap --disable-libquadmath --enable-plugins --enable-libgomp
--disable-libsanitizer --enable-gold --enable-graphite=yes
--with-cloog-version=0.18.0 --with-isl-version=0.11.1
--enable-eh-frame-hdr-for-static --disable-gold --disable-libgomp
--program-transform-name='s&^&aarch64-linux-android-&'
Thread model: posix
gcc version 4.8 (GCC)


With the attached test.c I get the following assembler code for func2():

Case 1:
aarch64-linux-android-gcc -O0 -march=armv8-a -fno-omit-frame-pointer -S test.c
-o -

 func2:
        stp     x29, x30, [sp, -32]!
        add     x29, sp, 0
        str     wzr, [x29,16]
        bl      func_no_leaf
        ldp     x29, x30, [sp], 32
        ret

-> Correct code

Case 2:
aarch64-linux-android-gcc -O0 -march=armv8-a -fno-omit-frame-pointer
-DINCLUDE_ATTRIBUTE -S test.c -o -

 func2:
        sub     sp, sp, #32
        str     x30, [sp]
        str     wzr, [sp,16]
        bl      func_no_leaf
        ldr     x30, [sp]
        add     sp, sp, 32
        ret

-> WRONG BEHAVIOUR: adding the function attribute to the source code forces the
whole module code to be compiled with -fomit-frame-pointer!

Case 3:
aarch64-linux-android-gcc -O0 -march=armv8-a -fomit-frame-pointer
-DINCLUDE_ATTRIBUTE -S test.c -o -

 func2:
        sub     sp, sp, #32
        str     x30, [sp]
        str     wzr, [sp,16]
        bl      func_no_leaf
        ldr     x30, [sp]
        add     sp, sp, 32
        ret

-> WRONG BEHAVIOUR: although there is a function attribute for func2() the code
looks like case 2, instead of the correct code in case 1

Case 4:
aarch64-linux-android-gcc -O0 -march=armv8-a -fomit-frame-pointer
-mno-omit-leaf-frame-pointer -DINCLUDE_ATTRIBUTE -S test.c -o -

 func2:
        stp     x29, x30, [sp, -32]!
        add     x29, sp, 0
        str     wzr, [x29,16]
        bl      func_no_leaf
        ldp     x29, x30, [sp], 32
        ret

-> CORRECT: func2() uses frame pointer, all other code omits the frame pointer.
This is the workaround we currently have to use to get our code to work.

[Bug target/60580] aarch64 generates wrong code for __attribute__ ((optimize("no-omit-frame-pointer")))

[jakub at gcc dot gnu.org]

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

Jakub Jelinek <jakub at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |jakub at gcc dot gnu.org,
                   |                            |jsm28 at gcc dot gnu.org,
                   |                            |uros at gcc dot gnu.org

--- Comment #1 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
This is a complete mess, what aarch64 backend does is completely incompatible
with optimize attribute.  I've tried to fix this up:
--- gcc/config/aarch64/aarch64.opt.jj    2014-01-20 14:50:22.000000000 +0100
+++ gcc/config/aarch64/aarch64.opt    2014-03-19 11:52:29.963772607 +0100
@@ -59,6 +59,11 @@ const char *aarch64_cpu_string
 Variable
 const char *aarch64_tune_string

+; Did we set flag_omit_frame_pointer just so aarch64_frame_pointer_required
+; would be called?
+Variable
+bool faked_omit_frame_pointer
+
 mbig-endian
 Target Report RejectNegative Mask(BIG_END)
 Assume target CPU is configured as big endian
--- gcc/config/aarch64/aarch64.c.jj    2014-03-18 12:26:13.000000000 +0100
+++ gcc/config/aarch64/aarch64.c    2014-03-19 12:20:19.471504562 +0100
@@ -315,10 +315,6 @@ static GTY(()) int gty_dummy;
 #define AARCH64_NUM_BITMASKS  5334
 static unsigned HOST_WIDE_INT aarch64_bitmasks[AARCH64_NUM_BITMASKS];

-/* Did we set flag_omit_frame_pointer just so
-   aarch64_frame_pointer_required would be called? */
-static bool faked_omit_frame_pointer;
-
 typedef enum aarch64_cond_code
 {
   AARCH64_EQ = 0, AARCH64_NE, AARCH64_CS, AARCH64_CC, AARCH64_MI, AARCH64_PL,
@@ -5278,7 +5274,11 @@ aarch64_override_options (void)
 static void
 aarch64_override_options_after_change (void)
 {
-  faked_omit_frame_pointer = false;
+  if (global_options_set.x_flag_omit_frame_pointer)
+    {
+      faked_omit_frame_pointer = false;
+      global_options_set.x_faked_omit_frame_pointer = false;
+    }

   /* To omit leaf frame pointers, we need to turn flag_omit_frame_pointer on
so
      that aarch64_frame_pointer_required will be called.  We need to remember
@@ -5288,6 +5288,9 @@ aarch64_override_options_after_change (v
     {
       flag_omit_frame_pointer = true;
       faked_omit_frame_pointer = true;
+      global_options_set.x_faked_omit_frame_pointer
+    = global_options_set.x_flag_omit_frame_pointer;
+      global_options_set.x_flag_omit_frame_pointer = false;
     }
 }

but while that fixes some of the cases, it doesn't fix everything, the problem
is that the non-TargetVariable Variables aren't actually Saved.

Now, i386 backend seems to handle it differently, instead of adding a
faked_omit_frame_pointer variable (which, as this PR suggests would need to be
handled as an Optimization Saved variable, but supposedly the opts framework
doesn't allow it yet), it clears omit_leaf_frame_pointer flag if
flag_omit_frame_pointer is true:
  /* Keep nonleaf frame pointers.  */
  if (opts->x_flag_omit_frame_pointer)
    opts->x_target_flags &= ~MASK_OMIT_LEAF_FRAME_POINTER;
  else if (TARGET_OMIT_LEAF_FRAME_POINTER_P (opts->x_target_flags))
    opts->x_flag_omit_frame_pointer = 1;
but does that only in in override_options hook.  That means that
say for -fomit-frame-pointer -momit-leaf-frame-pointer on the command line
supposedly that turns off TARGET_OMIT_LEAF_FRAME_POINTER and when some function
has __attribute__((optimize ("no-omit-frame-pointer"))) and is leaf, frame
pointer will not be omitted there.  But for -fno-omit-frame-pointer
-momit-leaf-frame-pointer on the command line and __attribute__((optimize
("no-omit-frame-pointer"))) supposedly it will not have frame pointer if it is
leaf.
The primary issue is that flag_omit_frame_pointer is Optimization Save flag,
while omit-leaf-frame-pointer is typcially Target Save, the former optimize
attribute thing, the latter target attribute thing.

Not sure what is the best way out of this, but I guess doing in aarch64 what
i386 does would be significant progress.

[Bug target/60580] aarch64 generates wrong code for __attribute__ ((optimize("no-omit-frame-pointer")))

[ramana at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |NEW
   Last reconfirmed|                            |2014-03-20
                 CC|                            |ramana at gcc dot gnu.org
     Ever confirmed|0                           |1

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

[Bug target/60580] aarch64 generates wrong code for __attribute__ ((optimize("no-omit-frame-pointer")))

[mshawcroft at gcc dot gnu.org]

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

mshawcroft at gcc dot gnu.org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |ASSIGNED

--- Comment #5 from mshawcroft at gcc dot gnu.org ---
Patch applied to trunk r208862

[Bug target/60580] aarch64 generates wrong code for __attribute__ ((optimize("no-omit-frame-pointer")))

[ktkachov at gcc dot gnu.org]

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

ktkachov at gcc dot gnu.org changed:

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

--- Comment #6 from ktkachov at gcc dot gnu.org ---
Marcus, can this be closed off?

[Bug target/60580] aarch64 generates wrong code for __attribute__ ((optimize("no-omit-frame-pointer")))

[jiwang at gcc dot gnu.org]

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

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

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

--- Comment #7 from Jiong Wang <jiwang at gcc dot gnu.org> ---
I'd take this for long term, hopefully could find a acceptable solution.

X86 is default with -fomit-frame-pointer which makes the logic a little bit
simpler, and thus X86 could survive in more normal case, but still fail in some
corner cases.

the fundamental problem is aarch64 and also x86 want to implement a finer
control of frame pointer which let leaf function be possible without setting up
frame record even under -fno-omit-frame-pointer. While gcc generic code is not
aware of that.

[Bug middle-end/60580] aarch64 generates wrong code for __attribute__ ((optimize("no-omit-frame-pointer")))

[jiwang at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Priority|P3                          |P4
          Component|target                      |middle-end
            Version|4.8.3                       |5.0
           Severity|normal                      |minor

[Bug middle-end/60580] aarch64 generates wrong code for __attribute__ ((optimize("no-omit-frame-pointer")))

[wdijkstr at arm dot com]

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

Wilco <wdijkstr at arm dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |wdijkstr at arm dot com

--- Comment #8 from Wilco <wdijkstr at arm dot com> ---
(In reply to Jiong Wang from comment #7)
> I'd take this for long term, hopefully could find a acceptable solution.
> 
> X86 is default with -fomit-frame-pointer which makes the logic a little bit
> simpler, and thus X86 could survive in more normal case, but still fail in
> some corner cases.
> 
> the fundamental problem is aarch64 and also x86 want to implement a finer
> control of frame pointer which let leaf function be possible without setting
> up frame record even under -fno-omit-frame-pointer. While gcc generic code
> is not aware of that.

Besides the solutions we discussed on the list to fix the underlying issues in
the mid-end, a workaround may be possible in the backend. If we can guarantee a
callback is made to the backend at the end of compilation of each function,
then we can restore the original value of the omit-frame-pointer variable. This
means the options code will never see the fake value, and attributes will work
as expected.

In principle any callback would work as long as the frame pointer variable is
not used for that function again, and it happens before entering the options
code again.

Obviously this is a nasty hack, but at least it works.