On Fri, Oct 23, 2020 at 1:47 PM Martin Liška <mli...@suse.cz> wrote:
>
> Hey.
>
> This is a follow-up of the discussion that happened in thread about
> no_stack_protector
> attribute: https://gcc.gnu.org/pipermail/gcc-patches/2020-May/545916.html
>
> The current optimize attribute works in the following way:
> - 1) we take current global_options as base
> - 2) maybe_default_options is called for the currently selected optimization
> level, which
> means all rules in default_options_table are executed
> - 3) attribute values are applied (via decode_options)
>
> So the step 2) is problematic: in case of -O2 -fno-omit-frame-pointer and
> __attribute__((optimize("-fno-stack-protector")))
> ends basically with -O2 -fno-stack-protector because -fno-omit-frame-pointer
> is default:
> /* -O1 and -Og optimizations. */
> { OPT_LEVELS_1_PLUS, OPT_fomit_frame_pointer, NULL, 1 },
>
> My patch handled and the current optimize attribute really behaves that same
> as appending attribute value
> to the command line. So far so good. We should also reflect that in
> documentation entry which is quite
> vague right now:
>
> """
> The optimize attribute is used to specify that a function is to be compiled
> with different optimization options than specified on the command line.
> """
>
> and we may want to handle -Ox in the attribute in a special way. I guess many
> macro/pragma users expect that
>
> -O2 -ftree-vectorize and __attribute__((optimize(1))) will end with -O1 and
> not
> with -ftree-vectorize -O1 ?
Hmm. I guess the only two reasonable options are to append to the active set
and thus end up with -ftree-vectorize -O1 or to start from an empty set and thus
end up with -O1.
Maybe we can have
@item optimize (@var{level}, @dots{})
reset everything to plain -On and
@item optimize (@var{string}, @dots{})
append? So optimize("O1") will end up with -O2 -ftree-vectorize -O1 and
optimize(1) with -O1? How do we handle
void __attribute__((optimize(1),optimize("ftree-vectorize")))
thus two optimize attributes?
> I'm also planning to take a look at the target macro/attribute, I expect
> similar problems:
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=97469
>
> Thoughts?
> Thanks,
> Martin
>
> gcc/c-family/ChangeLog:
>
> * c-common.c (parse_optimize_options): Decoded attribute options
> with the ones that were already set on the command line.
>
> gcc/ChangeLog:
>
> * toplev.c (toplev::main): Save decoded Optimization options.
> * toplev.h (save_opt_decoded_options): New.
>
> gcc/testsuite/ChangeLog:
>
> * gcc.target/i386/avx512er-vrsqrt28ps-3.c: Disable -ffast-math.
> * gcc.target/i386/avx512er-vrsqrt28ps-5.c: Likewise.
> ---
> gcc/c-family/c-common.c | 15 ++++++++++++++-
> .../gcc.target/i386/avx512er-vrsqrt28ps-3.c | 2 +-
> .../gcc.target/i386/avx512er-vrsqrt28ps-5.c | 2 +-
> gcc/toplev.c | 8 ++++++++
> gcc/toplev.h | 1 +
> 5 files changed, 25 insertions(+), 3 deletions(-)
>
> diff --git a/gcc/c-family/c-common.c b/gcc/c-family/c-common.c
> index e16ca3894bc..d4342e93d0a 100644
> --- a/gcc/c-family/c-common.c
> +++ b/gcc/c-family/c-common.c
> @@ -5727,10 +5727,23 @@ parse_optimize_options (tree args, bool attr_p)
> j++;
> }
> decoded_options_count = j;
> +
> + /* Merge the decoded options with save_decoded_options. */
> + unsigned save_opt_count = save_opt_decoded_options.length ();
> + unsigned merged_decoded_options_count = save_opt_count +
> decoded_options_count;
> + cl_decoded_option *merged_decoded_options
> + = XNEWVEC (cl_decoded_option, merged_decoded_options_count);
> +
> + for (unsigned i = 0; i < save_opt_count; ++i)
> + merged_decoded_options[i] = save_opt_decoded_options[i];
> + for (unsigned i = 0; i < decoded_options_count; ++i)
> + merged_decoded_options[save_opt_count + i] = decoded_options[i];
> +
> /* And apply them. */
> decode_options (&global_options, &global_options_set,
> - decoded_options, decoded_options_count,
> + merged_decoded_options, merged_decoded_options_count,
> input_location, global_dc, NULL);
> + free (decoded_options);
>
> targetm.override_options_after_change();
>
> diff --git a/gcc/testsuite/gcc.target/i386/avx512er-vrsqrt28ps-3.c
> b/gcc/testsuite/gcc.target/i386/avx512er-vrsqrt28ps-3.c
> index 1ba8172d6e3..40aefb50844 100644
> --- a/gcc/testsuite/gcc.target/i386/avx512er-vrsqrt28ps-3.c
> +++ b/gcc/testsuite/gcc.target/i386/avx512er-vrsqrt28ps-3.c
> @@ -8,7 +8,7 @@
> #define MAX 1000
> #define EPS 0.00001
>
> -__attribute__ ((noinline, optimize (1)))
> +__attribute__ ((noinline, optimize (1, "-fno-fast-math")))
> void static
> compute_rsqrt_ref (float *a, float *r)
> {
> diff --git a/gcc/testsuite/gcc.target/i386/avx512er-vrsqrt28ps-5.c
> b/gcc/testsuite/gcc.target/i386/avx512er-vrsqrt28ps-5.c
> index e067a81e562..498f4d50aa5 100644
> --- a/gcc/testsuite/gcc.target/i386/avx512er-vrsqrt28ps-5.c
> +++ b/gcc/testsuite/gcc.target/i386/avx512er-vrsqrt28ps-5.c
> @@ -8,7 +8,7 @@
> #define MAX 1000
> #define EPS 0.00001
>
> -__attribute__ ((noinline, optimize (1)))
> +__attribute__ ((noinline, optimize (1, "-fno-fast-math")))
> void static
> compute_sqrt_ref (float *a, float *r)
> {
> diff --git a/gcc/toplev.c b/gcc/toplev.c
> index 8c1e1e1f44f..27e4a1bc485 100644
> --- a/gcc/toplev.c
> +++ b/gcc/toplev.c
> @@ -123,6 +123,9 @@ static bool no_backend;
> struct cl_decoded_option *save_decoded_options;
> unsigned int save_decoded_options_count;
>
> +/* Vector of saved Optimization decoded command line options. */
> +auto_vec<cl_decoded_option> save_opt_decoded_options;
> +
> /* Used to enable -fvar-tracking, -fweb and -frename-registers according
> to optimize in process_options (). */
> #define AUTODETECT_VALUE 2
> @@ -2430,6 +2433,11 @@ toplev::main (int argc, char **argv)
> &save_decoded_options,
> &save_decoded_options_count);
>
> + /* Save Optimization decoded options. */
> + for (unsigned i = 0; i < save_decoded_options_count; ++i)
> + if (cl_options[save_decoded_options[i].opt_index].flags &
> CL_OPTIMIZATION)
> + save_opt_decoded_options.safe_push (save_decoded_options[i]);
> +
> /* Perform language-specific options initialization. */
> lang_hooks.init_options (save_decoded_options_count,
> save_decoded_options);
>
> diff --git a/gcc/toplev.h b/gcc/toplev.h
> index d6c316962b0..627312f85f5 100644
> --- a/gcc/toplev.h
> +++ b/gcc/toplev.h
> @@ -23,6 +23,7 @@ along with GCC; see the file COPYING3. If not see
> /* Decoded options, and number of such options. */
> extern struct cl_decoded_option *save_decoded_options;
> extern unsigned int save_decoded_options_count;
> +extern auto_vec<cl_decoded_option> save_opt_decoded_options;
>
> class timer;
>
> --
> 2.28.0
>
