[Bug c/111643] New: __attribute__((flatten)) with -O1 runs out of memory (killed cc1)

[Bug c/111643] New: __attribute__((flatten)) with -O1 runs out of memory (killed cc1)

[lukas.graetz@tu-darmstadt.de]

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

            Bug ID: 111643
           Summary: __attribute__((flatten)) with -O1 runs out of memory
                    (killed cc1)
           Product: gcc
           Version: 13.2.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: c
          Assignee: unassigned at gcc dot gnu.org
          Reporter: lukas.graetz@tu-darmstadt.de
  Target Milestone: ---

Created attachment 56017
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=56017&action=edit
C file

When I run

    gcc -c -O0 runs_out_of_memory.i -o runs_out_of_memory.o

(see the attached .i file) everything is fine. But when I run

    gcc -c -O1 runs_out_of_memory.i -o runs_out_of_memory.o

then I get:

    gcc: fatal error: Killed signal terminated program cc1

Apparently, quickly runs out of memory. I have 16 GB ram and the program is
rather simple. I tested it with gcc versions 9.4.0, 5.1 and 13.2 (target
x86_64-linux-gnu) on ubuntu 20.04.

I believe the problem is the __attribute__((flatten)) on several methods.

How I created the source file: The code comes from busybox (file
coreutils/expr.c) and musl header files. Additionally, I replaced every
function 'name' with 'name_original' and added a wrapper with
__attribute__((flatten)), for later instrumentation (I did this with a script).
I used that attribute, because to reduce the overhead of the wrapper functions
and I believe this should be fine. My reason why I introduced the wrappers in
the first place was to allow a fine-grained instrumentation of these functions.

[Bug ipa/111643] __attribute__((flatten)) with -O1 runs out of memory (killed cc1)

[pinskia at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
          Component|c                           |ipa
                 CC|                            |marxin at gcc dot gnu.org

--- Comment #1 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
I am 99% sure this is falls under don't do this as flatten inlines everything
it can that the function calls ...

[Bug ipa/111643] __attribute__((flatten)) with -O1 runs out of memory (killed cc1)

[glisse at gcc dot gnu.org]

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

Marc Glisse <glisse at gcc dot gnu.org> changed:

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

--- Comment #2 from Marc Glisse <glisse at gcc dot gnu.org> ---
(In reply to Andrew Pinski from comment #1)
> I am 99% sure this is falls under don't do this as flatten inlines
> everything it can that the function calls ...

Maybe people end up abusing flatten because we are missing a convenient way for
a caller to ask that a call be inlined? From the callee, we can use
always_inline (couldn't this be used on name_original in this testcase?), but
from the caller... Here even a non-recursive version of flatten would have
helped.

[Bug ipa/111643] __attribute__((flatten)) with -O1 runs out of memory (killed cc1)

[lukas.graetz@tu-darmstadt.de]

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

--- Comment #3 from Lukas Grätz <lukas.graetz@tu-darmstadt.de> ---
(In reply to Marc Glisse from comment #2)
> (In reply to Andrew Pinski from comment #1)
> > I am 99% sure this is falls under don't do this as flatten inlines
> > everything it can that the function calls ...
> 
> Maybe people end up abusing flatten because we are missing a convenient way
> for a caller to ask that a call be inlined? From the callee, we can use
> always_inline (couldn't this be used on name_original in this testcase?),
> but from the caller... Here even a non-recursive version of flatten would
> have helped.

Yes, this was what I was searching for, but I found only flatten. Also, that
flatten is applied recursively is not mentioned in the documentation and it is
also not what I would expect.

https://gcc.gnu.org/onlinedocs/gcc/Common-Function-Attributes.html

I don't want to always_inline name_original. What I want is to only inline
name_original when called by the wrapper function name, hence the flatten.
Because I replace every call to name with name_original where I don't want to
apply the instrumentation by the wrapper function name.

Thanks!

[Bug ipa/111643] __attribute__((flatten)) with -O1 runs out of memory (killed cc1)

[lukas.graetz@tu-darmstadt.de]

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

--- Comment #4 from Lukas Grätz <lukas.graetz@tu-darmstadt.de> ---
Sorry, just to clarify, whether I understood your two comments correctly.
Should foo() be inlined in the following example because flatten works
recursively?

void foo (void) {
    // CODE
}

int bar_original (void) {
    // CODE
    foo();
    // CODE
}

__attribute__((flatten))
int bar (void) {
    // INSTRUMENTATION CAN GO HERE
    return bar_original();
}

I thought that according to the documentation of flatten, foo() would not be
affected by the flatten attribute of bar(). It says: "For a function marked
with this attribute, every call inside this function is inlined, if possible."
The call to foo() is not directly inside the function bar(). Only if
bar_original() had also the __attribute__((flatten)), I would expect foo() to
be made inline in bar() because of recursive flatten. Of course, it could still
be inlined because some heuristics...

[Bug ipa/111643] __attribute__((flatten)) with -O1 runs out of memory (killed cc1)

[pinskia at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Keywords|                            |documentation

--- Comment #5 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
>It says: "For a function marked with this attribute, every call inside this function is inlined, if possible." The call to foo() is not directly inside the function bar().

The definition of "every call" is definitely ambious in the documentation. It
could mean every mentioned call or it could mean recusively every call. In this
case, it means flatten all calls including ones called from other functions
("indirectly"). It was originally added to workaround some (not so good)
inlining heurstics at the time for some C++ code (early 2000s).  inlining
heurstics has changed since then and has gotten better over the years so folks
don't use it as much.

Also if you have any code where GCC's inlining heurstics does not do what you
think it should please file that so we can improve them instead of workarounds
like always_inline and flatten don't need to be used ...

[Bug ipa/111643] __attribute__((flatten)) with -O1 runs out of memory (killed cc1)

[rguenth at gcc dot gnu.org]

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

Richard Biener <rguenth at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
     Ever confirmed|0                           |1
             Status|UNCONFIRMED                 |NEW
   Last reconfirmed|                            |2023-10-04

--- Comment #6 from Richard Biener <rguenth at gcc dot gnu.org> ---
Something like

#pragma GCC inline
  foo ();

or

 [[gnu::inline]] foo ();

was mentioned a few times but nobody bothered to implement it (setting a flag
on the CALL_EXPR).

Will post/push

diff --git a/gcc/doc/extend.texi b/gcc/doc/extend.texi
index b4770f1a149..645c76f23e9 100644
--- a/gcc/doc/extend.texi
+++ b/gcc/doc/extend.texi
@@ -3109,7 +3109,9 @@ file descriptor opened with @code{O_RDONLY}.
 @cindex @code{flatten} function attribute
 @item flatten
 Generally, inlining into a function is limited.  For a function marked with
-this attribute, every call inside this function is inlined, if possible.
+this attribute, every call inside this function is inlined including the
+calls such inlining introduces to the function (but not recursive calls
+to the function itself), if possible.
 Functions declared with attribute @code{noinline} and similar are not
 inlined.  Whether the function itself is considered for inlining depends
 on its size and the current inlining parameters.

[Bug ipa/111643] __attribute__((flatten)) with -O1 runs out of memory (killed cc1)

[cvs-commit at gcc dot gnu.org]

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

--- Comment #7 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by Richard Biener <rguenth@gcc.gnu.org>:

https://gcc.gnu.org/g:ffbd7c3d0fd1b9b10ef5a0f2b2e64bd234620167

commit r14-4404-gffbd7c3d0fd1b9b10ef5a0f2b2e64bd234620167
Author: Richard Biener <rguenther@suse.de>
Date:   Wed Oct 4 11:19:10 2023 +0200

    ipa/111643 - clarify flatten attribute documentation

    The following clarifies the flatten attribute documentation to mention
    the inlining applies also to calls formed as part of inlining earlier
    calls but not calls to the function itself.

            PR ipa/111643
            * doc/extend.texi (attribute flatten): Clarify.

[Bug ipa/111643] __attribute__((flatten)) with -O1 runs out of memory (killed cc1)

[rguenth at gcc dot gnu.org]

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

Richard Biener <rguenth at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |RESOLVED
         Resolution|---                         |WONTFIX

--- Comment #8 from Richard Biener <rguenth at gcc dot gnu.org> ---
I've amended the documentation, similar to the other duplicates we do not
intend to fix this (or rather we can't), even less so when LTO isn't involved.

[Bug ipa/111643] __attribute__((flatten)) with -O1 runs out of memory (killed cc1)

[lukas.graetz@tu-darmstadt.de]

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

--- Comment #9 from Lukas Grätz <lukas.graetz@tu-darmstadt.de> ---
Thanks for everything, it seemed to be a misunderstanding from my side anyway
and the documentation fix should help others.

I am sorry for being silent, I was sick for a few days. As for my original
problem, I am thinking of opening a new report, because I realized there could
be another solution without flatten. To explain a bit more, we have
bar_original() and bar_new(), the latter should behave identical to the former
except one additional statement, the "instrumentation". Since the
instrumentation can be done in two assembler instructions only, the overhead of
bar_new() calling bar_original() is not negligible.

int bar_original (int x) { /* CODE */ }

unsigned int trace_buffer[512];
uint8_t trace_pos;

#define FUNCTION_NUMBER_bar     0x686
int bar_new (int x) {
    trace_buffer[trace_pos++] = 0x686; // instrumentation
    return bar_original(x);
}

My idea: Do not touch the stack inside bar_new() and replace the call in
bar_new() with a jump or better a fall-through to bar_original(). This is
possible, because both functions have the same signature. It could save around
4 instructions and some stack memory. I have a lot of such functions after my
instrumentation step.

I also wondered whether

int bar_alias (void) { return bar_original(); }

could be a portable alternative to attribute alias. Except that current GCC
does not translate it that way.

[Bug ipa/111643] __attribute__((flatten)) with -O1 runs out of memory (killed cc1)

[amonakov at gcc dot gnu.org]

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

Alexander Monakov <amonakov at gcc dot gnu.org> changed:

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

--- Comment #10 from Alexander Monakov <amonakov at gcc dot gnu.org> ---
(In reply to Lukas Grätz from comment #9)
> I also wondered whether
> 
> int bar_alias (void) { return bar_original(); }
> 
> could be a portable alternative to attribute alias. Except that current GCC
> does not translate it that way.

That's because function addresses are significant and so

  &bar_alias == &bar_original

must evaluate to false, but would be true for aliases.

In theory compilers could do better by introducing fall-through aliases:
https://gcc.gnu.org/wiki/cauldron2019talks?action=AttachFile&do=view&target=fallthrough-aliases.pdf

[Bug ipa/111643] __attribute__((flatten)) with -O1 runs out of memory (killed cc1)

[lukas.graetz@tu-darmstadt.de]

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

--- Comment #11 from Lukas Grätz <lukas.graetz@tu-darmstadt.de> ---
(In reply to Alexander Monakov from comment #10)
> (In reply to Lukas Grätz from comment #9)
> > I also wondered whether
> > 
> > int bar_alias (void) { return bar_original(); }
> > 
> > could be a portable alternative to attribute alias. Except that current GCC
> > does not translate it that way.
> 
> That's because function addresses are significant and so
> 
>   &bar_alias == &bar_original
> 
> must evaluate to false, but would be true for aliases.
> 
> In theory compilers could do better by introducing fall-through aliases:
> https://gcc.gnu.org/wiki/
> cauldron2019talks?action=AttachFile&do=view&target=fallthrough-aliases.pdf

Thanks a lot! I haven't thought about function addresses. Is there hope that
fall-through aliases get into gcc? Then my perhaps my instrumentation
fall-through would also be possible to implement.

[Bug ipa/111643] __attribute__((flatten)) with -O1 runs out of memory (killed cc1)

[pinskia at gcc dot gnu.org]

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

--- Comment #12 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
Gcc does have tail call optimization which should allow the instrumentation
with less overhead. Though tail call optimization happens at -O2 and above only
(by default).

[Bug ipa/111643] __attribute__((flatten)) with -O1 runs out of memory (killed cc1)

[pinskia at gcc dot gnu.org]

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

--- Comment #13 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
(In reply to Andrew Pinski from comment #12)
> Gcc does have tail call optimization which should allow the instrumentation
> with less overhead. Though tail call optimization happens at -O2 and above
> only (by default).

The only improvement to this would be fall through alias which allows the
removal of the jump to the other function. A direct non-conditional jump is
usually predictable so the overhead should be small still.

[Bug ipa/111643] __attribute__((flatten)) with -O1 runs out of memory (killed cc1)

[lukas.graetz@tu-darmstadt.de]

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

--- Comment #14 from Lukas Grätz <lukas.graetz@tu-darmstadt.de> ---

(In reply to Andrew Pinski from comment #13)
> (In reply to Andrew Pinski from comment #12)
> > Gcc does have tail call optimization which should allow the instrumentation
> > with less overhead. Though tail call optimization happens at -O2 and above
> > only (by default).
> 
> The only improvement to this would be fall through alias which allows the
> removal of the jump to the other function. A direct non-conditional jump is
> usually predictable so the overhead should be small still.

Thanks! I thought that there was still some stack involved also causing some
overhead for every function call (in comparison to a pure non-conditional
jump). When I have time next week, I will try to look into that in detail.