Re: [PATCH] Optimize macro: make it more predictable

[Jeff Law <law@redhat.com>]

On 10/23/20 5:47 AM, Martin Liška 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 ?
>
> 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.
So you XNEWVEC and store the result into "merge_decoded_options".  But
you free "decoded_options".  Was that intentional?

This seems to bring a bit more predictability, but I suspect there's
more to do here.


jeff