[Bug c++/96535] New: GCC 10 ignoring function __attribute__ optimize for all x86

[Bug c++/96535] New: GCC 10 ignoring function __attribute__ optimize for all x86

[danielhanchen at gmail dot com]

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

            Bug ID: 96535
           Summary: GCC 10 ignoring function __attribute__ optimize for
                    all x86
           Product: gcc
           Version: unknown
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: c++
          Assignee: unassigned at gcc dot gnu.org
          Reporter: danielhanchen at gmail dot com
  Target Milestone: ---

Hey GCC team!

In GCC 10.x, it seems like any argument to __attribute__((optimize(...)) is
ignored at the function level. GCC 9.x and previous do not have this issue. [Or
maybe only -funroll-loops is ignored not 100% sure]

Detailed example at: https://gcc.godbolt.org/z/PTK4WE

3 Scenarios

1. [GCC 10.2: -O2 -ffast-math -march=haswell -std=c++2a -fopenmp] +
[__attribute__((optimize("O2","fast-math","unroll-loops")))] DOES NOT unroll.

2. [GCC 10.2: -funroll-loops -O2 -ffast-math -march=haswell -std=c++2a
-fopenmp] + [__attribute__((optimize("O2","fast-math","unroll-loops")))] DOES
unroll.

3. [GCC 9.3:  -O2 -ffast-math -march=haswell -std=c++2a -fopenmp] +
[__attribute__((optimize("O2","fast-math","unroll-loops")))] DOES unroll.

It seems that in GCC 10.x, you have to place -funroll-loops in the compilation
string, and function level __attribute__s are ignored?

PS: Code in godbolt is a matrix multiplication kernel. It multiplies 1 column *
1 row of a matrix.

[Bug c++/96535] GCC 10 ignoring function __attribute__ optimize for all x86

[crazylht at gmail dot com]

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

--- Comment #1 from Hongtao.liu <crazylht at gmail dot com> ---
for cmdline option, it's handled in process_options which will enable
flag_cunroll_grow_size which is the real effective flag to unroll the loop in
testcase.

cut from toplev.c
---
  /* Unrolling all loops implies that standard loop unrolling must also
     be done.  */
  if (flag_unroll_all_loops)
    flag_unroll_loops = 1;

  /* Allow cunroll to grow size accordingly.  */
  if (flag_cunroll_grow_size == AUTODETECT_VALUE)
    flag_cunroll_grow_size
      = flag_unroll_loops || flag_peel_loops || optimize >= 3;

  /* web and rename-registers help when run after loop unrolling.  */
  if (flag_web == AUTODETECT_VALUE)
    flag_web = flag_unroll_loops;

  if (flag_rename_registers == AUTODETECT_VALUE)
    flag_rename_registers = flag_unroll_loops;

---

But for attribute ((optimize ("unroll-loops")))
it's handled in handle_optimize_attribute which doesn't enable
flag_cunroll_grow_size.

[Bug c++/96535] [10/11 Regression] GCC 10 ignoring function __attribute__ optimize for all x86 since r11-1019

[jakub at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
            Summary|GCC 10 ignoring function    |[10/11 Regression] GCC 10
                   |__attribute__ optimize for  |ignoring function
                   |all x86                     |__attribute__ optimize for
                   |                            |all x86 since r11-1019
   Last reconfirmed|                            |2020-08-11
                 CC|                            |guojiufu at gcc dot gnu.org,
                   |                            |jakub at gcc dot gnu.org
             Status|UNCONFIRMED                 |NEW
   Target Milestone|---                         |10.3
     Ever confirmed|0                           |1
           Priority|P3                          |P2

--- Comment #2 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Caused by r11-1019-g71489d8e7572388c6e9528a45e4942b7c7b67e4f and its
r10-8327-g60bd3f20baebeeddd60f8a2b85927e7da7c6016e backport to 10 branch.

[Bug c++/96535] [10/11 Regression] GCC 10 ignoring function __attribute__ optimize for all x86 since r11-1019

[danielhanchen at gmail dot com]

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

--- Comment #3 from Daniel Han-Chen <danielhanchen at gmail dot com> ---
Oh lolll I was just about to add a comment about further experimentation

Seems like Jakub and Hongtao have found the root cause of the issues?

Anyways what I was gonna write [probs not necessary anymore so no need to read]

"""
Anyways from more experimentation, it seems like O1, O2, O3 are not ignored,
but the unrolling only gets turned on via O3. So if one passes O1, O2 in
__attribute__, but the command line is O3, the function still unrolls.

For eg, when commandline is O3, in GCC 9, __attribute__((optimize("O1 / 2"))
causes code to use VMULPS and VADDPS with an unroll factor of 1.

However in GCC 10.x, when the commandline is O3, VMULPS and VADDPS is used
(optimize("O1/2")), however, unrolling is still done??? Passing
"no-unroll-loops" in attribute also does not work.

It seems like the commandline O3 overrides unrolling or something? The
resulting assembly does use VMULPS/VADDPS and not VFMADDPS for O1/O2, but O3
causes an unrolling factor of 6 or so [it should be 1]

https://gcc.godbolt.org/z/qb3d5M for new example.
"""

[Bug tree-optimization/96535] [10/11 Regression] GCC 10 ignoring function __attribute__ optimize for all x86 since r11-1019

[jakub at gcc dot gnu.org]

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

--- Comment #4 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Created attachment 49039
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=49039&action=edit
gcc11-pr96535.patch

Ugh, process_options is called only once and thus I believe processing of
options with Optimization attribute doesn't really belong there, but should be
done elsewhere (finish_options?).
I think processing of even some other options might need moving, but am not
ready to try that.
In any case, such a patch is definitely not suitable for backporting, so
perhaps
for this PR we should just move the flag_cunroll_grow_size handling in there
for now (and with opts->x_flag_unroll_all_loops || added to it because
process_options does the if flag_unroll_all_loops then set flag_unroll_loops),
and deal with the rest separately.

[Bug tree-optimization/96535] [10/11 Regression] GCC 10 ignoring function __attribute__ optimize for all x86 since r11-1019

[jakub at gcc dot gnu.org]

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

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

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

--- Comment #5 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Martin, you've been fixing Optimization option processing lately, your thoughts
on that?

[Bug tree-optimization/96535] [10/11 Regression] GCC 10 ignoring function __attribute__ optimize for all x86 since r11-1019

[marxin at gcc dot gnu.org]

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

Martin Liška <marxin at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           See Also|                            |https://gcc.gnu.org/bugzill
                   |                            |a/show_bug.cgi?id=92860

--- Comment #6 from Martin Liška <marxin at gcc dot gnu.org> ---
(In reply to Jakub Jelinek from comment #5)
> Martin, you've been fixing Optimization option processing lately, your
> thoughts on that?

There was quite nice discussion here:
https://gcc.gnu.org/pipermail/gcc-patches/2020-May/545916.html

I like what Richi suggested:

"""
I think the only sensible way is to save the original decoded options
from the gcc invocation (and have #pragma optimize push/pop append
accordingly) and append the current attribute/pragmas optimization
string to them and run that whole thing on a default option state.

That makes semantics equivalent to appending more options to the
command-line.  Well, hopefully.  Interaction with the target attribute
might be interesting (that likely also needs to append to that
"current decoded options" set).
"""

Anyway, it's a quite complicated topic. I may take a look at this in Fall this
year.

[Bug tree-optimization/96535] [10/11 Regression] GCC 10 ignoring function __attribute__ optimize for all x86 since r11-1019

[jakub at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Assignee|unassigned at gcc dot gnu.org      |jakub at gcc dot gnu.org
             Status|NEW                         |ASSIGNED

--- Comment #7 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Created attachment 49043
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=49043&action=edit
gcc11-pr96535.patch

Updated patch to only move handling of the loop unrolling options (but I need
changes on the rs6000 side too to match that).

[Bug tree-optimization/96535] [10/11 Regression] GCC 10 ignoring function __attribute__ optimize for all x86 since r11-1019

[guojiufu at gcc dot gnu.org]

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

--- Comment #8 from Jiu Fu Guo <guojiufu at gcc dot gnu.org> ---
(In reply to Jakub Jelinek from comment #7)
> Created attachment 49043 [details]
> gcc11-pr96535.patch
> 
> Updated patch to only move handling of the loop unrolling options (but I
> need changes on the rs6000 side too to match that).

I did not aware attribute(optimize) issue in previous patch. Thanks a lot for
handle this!

[Bug tree-optimization/96535] [10/11 Regression] GCC 10 ignoring function __attribute__ optimize for all x86 since r11-1019

[cvs-commit at gcc dot gnu.org]

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

--- Comment #9 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by Jakub Jelinek <jakub@gcc.gnu.org>:

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

commit r11-2672-gfe9458c280dbd6e8b892db4ca3b64185049c376b
Author: Jakub Jelinek <jakub@redhat.com>
Date:   Wed Aug 12 17:00:41 2020 +0200

    Fix up flag_cunroll_grow_size handling in presence of optimize attr
[PR96535]

    As the testcase in the PR shows (not included in the patch, as
    it seems quite fragile to observe unrolling in the IL), the introduction of
    flag_cunroll_grow_size broke optimize attribute related to loop unrolling.
    The problem is that the new option flag is set (if not set explicitly) only
    in process_options and in rs6000_option_override_internal (and there only
if
    global_init_p).  So, this means that while it is Optimization option, it
    will only be set based on the command line -funroll-loops/-O3/-fpeel-loops
    or -funroll-all-loops, which means that if command line does include any of
    those, it is enabled even for functions that will through optimize
attribute
    have all of those disabled, and if command line does not include those,
    it will not be enabled for functions that will through optimize attribute
    have any of those enabled.

    process_options is called just once, so IMHO it should be handling only
    non-Optimization option adjustments (various other options suffer from that
    too, but as this is a regression from 10.1 on the 10 branch, changing those
    is not appropriate).  Similarly, rs6000_option_override_internal is called
    only once (with global_init_p) and then for target attribute handling, but
    not for optimize attribute handling.

    This patch moves the unrolling related handling from process_options into
    finish_options which is invoked whenever the options are being finalized,
    and the rs6000 specific parts into the override_options_after_change hook
    which is called for optimize attribute handling (and unfortunately also
    th cfun changes, but what the hook does is cheap) and I've added a call to
    that from rs6000_override_options_internal, so it is also called on cmdline
    processing and for target attribute.

    Furthermore, it stops using AUTODETECT_VALUE, which can work only once,
    and instead uses the global_options_set.x_... flags.

    2020-08-12  Jakub Jelinek  <jakub@redhat.com>

            PR tree-optimization/96535
            * toplev.c (process_options): Move flag_unroll_loops and
            flag_cunroll_grow_size handling from here to ...
            * opts.c (finish_options): ... here.  For flag_cunroll_grow_size,
            don't check for AUTODETECT_VALUE, but instead check
            opts_set->x_flag_cunroll_grow_size.
            * common.opt (funroll-completely-grow-size): Default to 0.
            * config/rs6000/rs6000.c (TARGET_OVERRIDE_OPTIONS_AFTER_CHANGE):
            Redefine.
            (rs6000_override_options_after_change): New function.
            (rs6000_option_override_internal): Call it.  Move there the
            flag_cunroll_grow_size, unroll_only_small_loops and
            flag_rename_registers handling.

[Bug tree-optimization/96535] [10 Regression] GCC 10 ignoring function __attribute__ optimize for all x86 since r11-1019

[cvs-commit at gcc dot gnu.org]

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

--- Comment #10 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The releases/gcc-10 branch has been updated by Jakub Jelinek
<jakub@gcc.gnu.org>:

https://gcc.gnu.org/g:7151b3d700efcd0994bc1a6cfc36f5abc53c7f5a

commit r10-8667-g7151b3d700efcd0994bc1a6cfc36f5abc53c7f5a
Author: Jakub Jelinek <jakub@redhat.com>
Date:   Wed Aug 12 17:00:41 2020 +0200

    Fix up flag_cunroll_grow_size handling in presence of optimize attr
[PR96535]

    As the testcase in the PR shows (not included in the patch, as
    it seems quite fragile to observe unrolling in the IL), the introduction of
    flag_cunroll_grow_size broke optimize attribute related to loop unrolling.
    The problem is that the new option flag is set (if not set explicitly) only
    in process_options and in rs6000_option_override_internal (and there only
if
    global_init_p).  So, this means that while it is Optimization option, it
    will only be set based on the command line -funroll-loops/-O3/-fpeel-loops
    or -funroll-all-loops, which means that if command line does include any of
    those, it is enabled even for functions that will through optimize
attribute
    have all of those disabled, and if command line does not include those,
    it will not be enabled for functions that will through optimize attribute
    have any of those enabled.

    process_options is called just once, so IMHO it should be handling only
    non-Optimization option adjustments (various other options suffer from that
    too, but as this is a regression from 10.1 on the 10 branch, changing those
    is not appropriate).  Similarly, rs6000_option_override_internal is called
    only once (with global_init_p) and then for target attribute handling, but
    not for optimize attribute handling.

    This patch moves the unrolling related handling from process_options into
    finish_options which is invoked whenever the options are being finalized,
    and the rs6000 specific parts into the override_options_after_change hook
    which is called for optimize attribute handling (and unfortunately also
    th cfun changes, but what the hook does is cheap) and I've added a call to
    that from rs6000_override_options_internal, so it is also called on cmdline
    processing and for target attribute.

    Furthermore, it stops using AUTODETECT_VALUE, which can work only once,
    and instead uses the global_options_set.x_... flags.

    2020-08-12  Jakub Jelinek  <jakub@redhat.com>

            PR tree-optimization/96535
            * toplev.c (process_options): Move flag_unroll_loops and
            flag_cunroll_grow_size handling from here to ...
            * opts.c (finish_options): ... here.  For flag_cunroll_grow_size,
            don't check for AUTODETECT_VALUE, but instead check
            opts_set->x_flag_cunroll_grow_size.
            * common.opt (funroll-completely-grow-size): Default to 0.
            * config/rs6000/rs6000.c (TARGET_OVERRIDE_OPTIONS_AFTER_CHANGE):
            Redefine.
            (rs6000_override_options_after_change): New function.
            (rs6000_option_override_internal): Call it.  Move there the
            flag_cunroll_grow_size, unroll_only_small_loops and
            flag_rename_registers handling.

    (cherry picked from commit fe9458c280dbd6e8b892db4ca3b64185049c376b)

[Bug tree-optimization/96535] [10 Regression] GCC 10 ignoring function __attribute__ optimize for all x86 since r11-1019

[jakub at gcc dot gnu.org]

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

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

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

--- Comment #11 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Fixed for 10.3+ too.