[Bug target/103515] New: Unexpected OPTION_MASK_SAVE_TOC_INDIRECT of rs6000_isa_flag

[Bug target/103515] New: Unexpected OPTION_MASK_SAVE_TOC_INDIRECT of rs6000_isa_flag

[linkw at gcc dot gnu.org]

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

            Bug ID: 103515
           Summary: Unexpected OPTION_MASK_SAVE_TOC_INDIRECT of
                    rs6000_isa_flag
           Product: gcc
           Version: 12.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: target
          Assignee: unassigned at gcc dot gnu.org
          Reporter: linkw at gcc dot gnu.org
  Target Milestone: ---

Test case:
gcc/testsuite/g++.dg/torture/pr81360.C

Option:
-fno-early-inlining -Os

For function rs6000_can_inline_p, I tried to test if the rs6000_isa_flag is
always the same (expected) as the TREE_TARGET_OPTION
(target_option_default_node)->x_rs6000_isa_flags) when the caller_tree is NULL.

The regression testing on LE/BE showed up this failure. By investigating it, I
noticed it's a bug. Since the command line option is -Os, we should not set
this 
OPTION_MASK_SAVE_TOC_INDIRECT for most functions there:

  /* If we can shrink-wrap the TOC register save separately, then use
     -msave-toc-indirect unless explicitly disabled.  */
  if ((rs6000_isa_flags_explicit & OPTION_MASK_SAVE_TOC_INDIRECT) == 0
      && flag_shrink_wrap_separate
      && optimize_function_for_speed_p (cfun))
    rs6000_isa_flags |= OPTION_MASK_SAVE_TOC_INDIRECT;

But after compiler parsing the function main with "#pragma GCC optimize
("-O0")", the rs6000_isa_flags would always get this
OPTION_MASK_SAVE_TOC_INDIRECT on, even if the context is not for the function
main.

[Bug target/103515] Unexpected OPTION_MASK_SAVE_TOC_INDIRECT of rs6000_isa_flag

[linkw at gcc dot gnu.org]

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

Kewen Lin <linkw at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Target|                            |powerpc

--- Comment #1 from Kewen Lin <linkw at gcc dot gnu.org> ---
Put the below code at the beginning of function rs6000_can_inline_p can make it
ICE.

    tree caller_tree = DECL_FUNCTION_SPECIFIC_TARGET (caller);

    if (!caller_tree) {
      caller_tree = target_option_default_node;
      gcc_assert (rs6000_isa_flags
                  == TREE_TARGET_OPTION (caller_tree)->x_rs6000_isa_flags);
    }

#1  0x0000000011f1ff7c in rs6000_can_inline_p (caller=<function_decl
0x7ffff5c44700 __ct_base >, callee=<function_decl 0x7ffff5c44400 __ct_base >)

(gdb) p /x (rs6000_isa_flags ^ TREE_TARGET_OPTION
(caller_tree)->x_rs6000_isa_flags) & OPTION_MASK_SAVE_TOC_INDIRECT
$2 = 0x10000000000000

[Bug target/103515] Unexpected OPTION_MASK_SAVE_TOC_INDIRECT of rs6000_isa_flag

[marxin at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |marxin at gcc dot gnu.org
   Last reconfirmed|                            |2021-12-01
             Status|UNCONFIRMED                 |NEW
     Ever confirmed|0                           |1

[Bug target/103515] Unexpected OPTION_MASK_SAVE_TOC_INDIRECT of rs6000_isa_flag

[linkw at gcc dot gnu.org]

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

--- Comment #2 from Kewen Lin <linkw at gcc dot gnu.org> ---
Here I assumed that the current cl optimization/option save and restore scheme
wants to keep the global_option/global_option_set same as the one from the
initial option processing.  After we parsing all attributes/pragmas, we can
expect the rs6000_isa_flags back to the default one.

The fix seems to require us to take this OPTION_MASK_SAVE_TOC_INDIRECT as one
option which would be affected by optimize level.

diff --git a/gcc/config/rs6000/rs6000.c b/gcc/config/rs6000/rs6000.c
index 327822e5357..f9ee7044889 100644
--- a/gcc/config/rs6000/rs6000.c
+++ b/gcc/config/rs6000/rs6000.c
@@ -3478,6 +3478,16 @@ rs6000_override_options_after_change (void)
     }
   else if (!OPTION_SET_P (flag_cunroll_grow_size))
     flag_cunroll_grow_size = flag_peel_loops || optimize >= 3;
+  /* If we can shrink-wrap the TOC register save separately, then use
+     -msave-toc-indirect unless explicitly disabled.  */
+  if ((rs6000_isa_flags_explicit & OPTION_MASK_SAVE_TOC_INDIRECT) == 0
+      && flag_shrink_wrap_separate
+      && optimize_function_for_speed_p (cfun))
+    rs6000_isa_flags |= OPTION_MASK_SAVE_TOC_INDIRECT;
+  else
+    rs6000_isa_flags &= ~OPTION_MASK_SAVE_TOC_INDIRECT;


Also require us to build one target_node when we are going to save one new
optimization_node which isn't the same as the default one, since at that time
the optimization level changes and the option is possible to be changed as
well.

diff --git a/gcc/attribs.c b/gcc/attribs.c
index c252f5af07b..3382c095fa8 100644
--- a/gcc/attribs.c
+++ b/gcc/attribs.c
@@ -607,7 +607,14 @@ decl_attributes (tree *node, tree attributes, int flags,
   if (TREE_CODE (*node) == FUNCTION_DECL
       && optimization_current_node != optimization_default_node
       && !DECL_FUNCTION_SPECIFIC_OPTIMIZATION (*node))
-    DECL_FUNCTION_SPECIFIC_OPTIMIZATION (*node) = optimization_current_node;
+    {
+      DECL_FUNCTION_SPECIFIC_OPTIMIZATION (*node) = optimization_current_node;
+      tree target_node
+        = build_target_option_node (&global_options, &global_options_set);
+      if (!DECL_FUNCTION_SPECIFIC_TARGET (*node)
+          && target_node != target_option_default_node)
+        DECL_FUNCTION_SPECIFIC_TARGET (*node) = target_node;
+    }

   /* If this is a function and the user used #pragma GCC target, add the
      options to the attribute((target(...))) list.  */

[Bug target/103515] Unexpected OPTION_MASK_SAVE_TOC_INDIRECT of rs6000_isa_flag

[linkw at gcc dot gnu.org]

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

Kewen Lin <linkw at gcc dot gnu.org> changed:

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

--- Comment #3 from Kewen Lin <linkw at gcc dot gnu.org> ---
Note that if we changed the pragma syntax by attribute syntax, the ICE would be
gone.

[Bug target/103515] Unexpected OPTION_MASK_SAVE_TOC_INDIRECT of rs6000_isa_flag

[cvs-commit at gcc dot gnu.org]

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

--- Comment #4 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by Kewen Lin <linkw@gcc.gnu.org>:

https://gcc.gnu.org/g:01ad8c54fdca1db3d71bf6c4b861a9d1db3c2a59

commit r12-5920-g01ad8c54fdca1db3d71bf6c4b861a9d1db3c2a59
Author: Kewen Lin <linkw@linux.ibm.com>
Date:   Sun Dec 12 23:27:51 2021 -0600

    pragma: Update target option node when optimization changes [PR103515]

    For a function with optimize pragma, it's possible that the target
    options change as optimization options change.  Now we create one
    optimization option node when optimize pragma parsing, but don't
    create target option node for possible target option changes.  It
    makes later processing not detect the target options can actually
    change and further doesn't update the target options accordingly.

    This patch is to check whether target options have changed when
    creating one optimization option node for pragma optimize, and
    make one target option node if needed.  The associated test case
    shows the difference.  Without this patch, the function foo1 will
    perform unrolling which is unexpected.  The reason is that flag
    unroll_only_small_loops isn't correctly set for it.  The value
    is updated after parsing function foo2, but doesn't get restored
    later since both decls don't have DECL_FUNCTION_SPECIFIC_TARGET
    set and the hook thinks we don't need to switch.  With this patch,
    there is no unrolling for foo1, which is also consistent with the
    behavior by replacing pragma by attribute whether w/ and w/o this
    patch.

    As Martin noted, this change does the similar thing like what his
    previous commit r12-1039 did.

    gcc/ChangeLog:

            PR target/103515
            * attribs.c (decl_attributes): Check if target options change and
            create one node if so.

    gcc/testsuite/ChangeLog:

            PR target/103515
            * gcc.target/powerpc/pr103515.c: New test.

[Bug target/103515] Unexpected OPTION_MASK_SAVE_TOC_INDIRECT of rs6000_isa_flag

[bergner at gcc dot gnu.org]

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

--- Comment #5 from Peter Bergner <bergner at gcc dot gnu.org> ---
(In reply to CVS Commits from comment #4)
> The master branch has been updated by Kewen Lin <linkw@gcc.gnu.org>:

Kewen, can we mark this as FIXED?

[Bug target/103515] Unexpected OPTION_MASK_SAVE_TOC_INDIRECT of rs6000_isa_flag

[linkw at gcc dot gnu.org]

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

--- Comment #6 from Kewen Lin <linkw at gcc dot gnu.org> ---
(In reply to Peter Bergner from comment #5)
> (In reply to CVS Commits from comment #4)
> > The master branch has been updated by Kewen Lin <linkw@gcc.gnu.org>:
> 
> Kewen, can we mark this as FIXED?

Sorry, no. The issue isn't fixed completely, it still needs some more handlings
in rs6000. It's detected by my previous patch changing rs6000_can_inline_p
(https://gcc.gnu.org/pipermail/gcc-patches/2021-September/578552.html).
PR102059 is resolved, I'll split/re-post some fix from that patch for some
issues found when investigating it, then revisit this.