From: Christopher Bazley <[email protected]>

This patch fixes a bug reported as PR c/125327.

Previously, function attributes such as
__attribute__ ((target ("vsx"), optimize("O2"))) could
be applied wrongly because correct processing of the
optimize attribute relied on at least one of the following to
be true:

1. The function had no function-specific target options, or
2. changing optimization options did not have the side-effect
   of modifying target options, or
3. the target option modified as a side-effect of changing
   optimization options was not saved in the cl_target_option
   struct.

Assumptions 2 and 3 are not generally guaranteed to be true.
The implementation of the handle_optimize_attribute function
already implicitly acknowledged that by rebuilding the target
options after parsing optimization options, and replacing the
current target option node if that rebuilding resulted in
a fresh target option node. However, any target options
already associated with the function being modified were not
applied before parsing optimization options, therefore they
were lost if that function's target option node was replaced.
Moreover, the decision about whether to replace any existing
target option node was flawed because it was based on a
comparison with a fake node built from ambient global state
instead of a comparison with the current target node.

Assumption 3 is true for i386 because the i386 definition of
munroll-only-small-loops does not have the "Save" tag,
therefore its value cannot differ from the ambient global state
and the target node of the function was not replaced.
The rs6000 and s390 definitions of munroll-only-small-loops
(which is likewise implicitly enabled at -O2 and above)
*do* have the "Save" tag but the 'target' attribute is not
supported on s390, therefore the bug can only be reproduced
on rs6000.

For the bug to manifest, the target option node needed to be
replaced. For rs6000, that could happen if the newly-saved
value of munroll-only-small-loops differed from the ambient
global state (which was wrongly used as the baseline for the
comparison) even if the newly-saved value did not differ from
the value previously saved in the target option node of the
function whose 'optimize' attribute was being applied.

gcc/c-family/ChangeLog:

        * c-attribs.cc (handle_optimize_attribute):
        Save the global target options before modifying
        them. If the function with the optimize attribute
        already has a target option node then restore those
        target options as the global options, so that they
        are used as the basis for any new optimization and
        target options attached to the function.

gcc/d/ChangeLog:

        * d-attribs.cc (d_handle_optimize_attribute):
        Save the global target options before modifying
        them. If the function with the optimize attribute
        already has a target option node then restore those
        target options as the global options, so that they
        are used as the basis for any new optimization and
        target options attached to the function.

gcc/testsuite/ChangeLog:

        * gcc.target/powerpc/pr125327.c: New test.
---
 gcc/c-family/c-attribs.cc                   | 16 +++++++++++---
 gcc/d/d-attribs.cc                          | 16 +++++++++++---
 gcc/testsuite/gcc.target/powerpc/pr125327.c | 24 +++++++++++++++++++++
 3 files changed, 50 insertions(+), 6 deletions(-)
 create mode 100644 gcc/testsuite/gcc.target/powerpc/pr125327.c

diff --git a/gcc/c-family/c-attribs.cc b/gcc/c-family/c-attribs.cc
index d437c55285e3..99ce04abff56 100644
--- a/gcc/c-family/c-attribs.cc
+++ b/gcc/c-family/c-attribs.cc
@@ -6468,12 +6468,18 @@ handle_optimize_attribute (tree *node, tree name, tree 
args,
   else
     {
       struct cl_optimization cur_opts;
+      struct cl_target_option cur_target;
       tree old_opts = DECL_FUNCTION_SPECIFIC_OPTIMIZATION (*node);
+      tree old_target = DECL_FUNCTION_SPECIFIC_TARGET (*node);
 
       /* Save current options.  */
       cl_optimization_save (&cur_opts, &global_options, &global_options_set);
-      tree prev_target_node = build_target_option_node (&global_options,
-                                                       &global_options_set);
+      cl_target_option_save (&cur_target, &global_options, 
&global_options_set);
+
+      tree prev_target_node
+       = old_target
+           ? old_target
+           : build_target_option_node (&global_options, &global_options_set);
 
       /* If we previously had some optimization options, use them as the
         default.  */
@@ -6492,6 +6498,10 @@ handle_optimize_attribute (tree *node, tree name, tree 
args,
        cl_optimization_restore (&global_options, &global_options_set,
                                 TREE_OPTIMIZATION (old_opts));
 
+      if (old_target)
+       cl_target_option_restore (&global_options, &global_options_set,
+                                 TREE_TARGET_OPTION (old_target));
+
       /* Parse options, and update the vector.  */
       parse_optimize_options (args, true);
       DECL_FUNCTION_SPECIFIC_OPTIMIZATION (*node)
@@ -6509,7 +6519,7 @@ handle_optimize_attribute (tree *node, tree name, tree 
args,
       cl_optimization_restore (&global_options, &global_options_set,
                               &cur_opts);
       cl_target_option_restore (&global_options, &global_options_set,
-                               TREE_TARGET_OPTION (prev_target_node));
+                               &cur_target);
 
       if (saved_global_options != NULL)
        {
diff --git a/gcc/d/d-attribs.cc b/gcc/d/d-attribs.cc
index 3f748b7bef3e..30c1cb320ec2 100644
--- a/gcc/d/d-attribs.cc
+++ b/gcc/d/d-attribs.cc
@@ -923,12 +923,18 @@ d_handle_optimize_attribute (tree *node, tree name, tree 
args, int,
   else
     {
       struct cl_optimization cur_opts;
+      struct cl_target_option cur_target;
       tree old_opts = DECL_FUNCTION_SPECIFIC_OPTIMIZATION (*node);
+      tree old_target = DECL_FUNCTION_SPECIFIC_TARGET (*node);
 
       /* Save current options.  */
       cl_optimization_save (&cur_opts, &global_options, &global_options_set);
-      tree prev_target_node = build_target_option_node (&global_options,
-                                                       &global_options_set);
+      cl_target_option_save (&cur_target, &global_options, 
&global_options_set);
+
+      tree prev_target_node
+       = old_target
+           ? old_target
+           : build_target_option_node (&global_options, &global_options_set);
 
       /* If we previously had some optimization options, use them as the
         default.  */
@@ -943,6 +949,10 @@ d_handle_optimize_attribute (tree *node, tree name, tree 
args, int,
        cl_optimization_restore (&global_options, &global_options_set,
                                 TREE_OPTIMIZATION (old_opts));
 
+      if (old_target)
+       cl_target_option_restore (&global_options, &global_options_set,
+                                 TREE_TARGET_OPTION (old_target));
+
       /* Parse options, and update the vector.  */
       parse_optimize_options (args);
       DECL_FUNCTION_SPECIFIC_OPTIMIZATION (*node)
@@ -956,7 +966,7 @@ d_handle_optimize_attribute (tree *node, tree name, tree 
args, int,
       cl_optimization_restore (&global_options, &global_options_set,
                               &cur_opts);
       cl_target_option_restore (&global_options, &global_options_set,
-                               TREE_TARGET_OPTION (prev_target_node));
+                               &cur_target);
       if (saved_global_options != NULL)
        {
          cl_optimization_compare (saved_global_options, &global_options);
diff --git a/gcc/testsuite/gcc.target/powerpc/pr125327.c 
b/gcc/testsuite/gcc.target/powerpc/pr125327.c
new file mode 100644
index 000000000000..b7bde8532e09
--- /dev/null
+++ b/gcc/testsuite/gcc.target/powerpc/pr125327.c
@@ -0,0 +1,24 @@
+/* { dg-do compile } */
+/* { dg-options "-O1 -mcpu=power8 -mno-vsx" } */
+
+typedef double v2df __attribute__ ((vector_size (16)));
+
+/* This is a test for preservation of target-specific options when optimization
+   options are changed.  If the optimize() attribute is not applied correctly,
+   then the values of any target-specific options already set by the target()
+   attribute can be lost.  The vector/scalar (VSX) instruction enablement flag
+   associated with foo() should be preserved when the optimization level is
+   changed from 1 to 2, because neither -mvsx nor -mno-vsx is implied by -O2;
+   however, an unrelated target-specific option, -munroll-only-small-loops, 
*is*
+   implied by -O2, which requires the function's target option node to be
+   rebuilt.  When that happens, the existing value of the VSX flag must be
+   preserved.
+ */
+
+v2df __attribute__ ((target ("vsx"), optimize("O2")))
+foo (double a, double b)
+{
+  return (v2df){a, b};
+}
+
+/* { dg-final { scan-assembler {\txxpermdi } } } */
-- 
2.54.0

Reply via email to