Hi gcc-patches mailing list, Christopher Bazley via Sourceware Forge <[email protected]> has requested that the following forgejo pull request be published on the mailing list.
Created on: 2026-04-29 15:41:47+00:00 Latest update: 2026-05-15 15:34:34+00:00 Changes: 3 changed files, 50 additions, 6 deletions Head revision: chris.bazley/gcc ref prevent_optimize_attribute_from_undoing_target_attribute commit fa47ae32cb5a08cd03333d46e642b41b3cb0eb74 Base revision: gcc/gcc-TEST ref trunk commit 03bf757085091d95a9fe4ab964ee62da157ef563 r17-473-g03bf757085091d Merge base: 03bf757085091d95a9fe4ab964ee62da157ef563 Full diff url: https://forge.sourceware.org/gcc/gcc-TEST/pulls/150.diff Discussion: https://forge.sourceware.org/gcc/gcc-TEST/pulls/150 Requested Reviewers: 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. Changed files: - A: gcc/testsuite/gcc.target/powerpc/pr125327.c - M: gcc/c-family/c-attribs.cc - M: gcc/d/d-attribs.cc Christopher Bazley (1): c-family: Prevent optimize attribute from undoing target attribute 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 Range-diff against v1: 1: 2cf351ca3dec ! 1: fa47ae32cb5a Prevent optimize attribute from undoing target attribute @@ Metadata Author: Christopher Bazley <[email protected]> ## Commit message ## - Prevent optimize attribute from undoing target attribute + c-family: Prevent optimize attribute from undoing target attribute + + This patch fixes a bug reported as PR c/125327. Previously, function attributes such as - __attribute__ ((target ("+sve"), optimize ("O2"))) could + __attribute__ ((target ("vsx"), optimize("O2"))) could be applied wrongly because correct processing of the - optimize attribute relied on one of the following to be - true: + 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. + 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. - Assumption 2 is not generally guaranteed to be true, and - the implementation of the handle_optimize_attribute function + 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 @@ Commit message 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: @@ Commit message 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 ## @@ gcc/c-family/c-attribs.cc: handle_optimize_attribute (tree *node, tree name, tree args, else @@ gcc/d/d-attribs.cc: d_handle_optimize_attribute (tree *node, tree name, tree arg if (saved_global_options != NULL) { cl_optimization_compare (saved_global_options, &global_options); + + ## gcc/testsuite/gcc.target/powerpc/pr125327.c (new) ## +@@ ++/* { 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
