From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-ed1-x541.google.com (mail-ed1-x541.google.com [IPv6:2a00:1450:4864:20::541]) by sourceware.org (Postfix) with ESMTPS id C61E5398547A for ; Tue, 3 Nov 2020 13:28:04 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org C61E5398547A Received: by mail-ed1-x541.google.com with SMTP id t11so18150235edj.13 for ; Tue, 03 Nov 2020 05:28:04 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc:content-transfer-encoding; bh=5/qzjq9GKeEdLExVMM3jpxt3ydbl2Nv51I+j6izUFU0=; b=dibqo7DWW1lbqToorBigIcK4FlCusisQYgLXQnxl0tReIyc0n2PzDSBa/lsifKq68e TpNGNhGtPLjxjgfpxP2eW7Y5XegysnRN8W56Af3hZ6ZGBpWdkhHcngscUuBGw2nhLo/G gGBYk+bCUmKwUoa44JvLiBtkpJlqsziisEuSvLwNjM674lM2GpT/WgLHa0JR31pCkXRk kX9voaAQCoOWu73wLq3+oEj7RUlTsA8bTc0ropUkBjMH2/d3vlRaGI2jwbqOLoIP2IUB Nm8UZ/b8fvEsE6EhgUAAqWAs3a/3Oj2la3N4TLZa8xVxt2lJObKaQftGhKDDupFM9a/C FHog== X-Gm-Message-State: AOAM533cR99okI0BCVXwokrqCYdivq/KiJISQyz2Kev+U2XWoEb3JZMZ hHZS5yun2QnUo9Kg3UTUboLiJDwRU5rSUixM4GQ= X-Google-Smtp-Source: ABdhPJzkf/aGL/Vh25Xm8kbmCQCxvmz6pTfsHS+D4rgLCj1Tj9kqdgdayv7fR1xwFJnlK0ws0TxHNEBQoV4/E8s3ODM= X-Received: by 2002:aa7:cd56:: with SMTP id v22mr22427658edw.245.1604410083850; Tue, 03 Nov 2020 05:28:03 -0800 (PST) MIME-Version: 1.0 References: <82e71ebf-7b2e-67e7-1f08-ea525deee4cb@suse.cz> In-Reply-To: <82e71ebf-7b2e-67e7-1f08-ea525deee4cb@suse.cz> From: Richard Biener Date: Tue, 3 Nov 2020 14:27:52 +0100 Message-ID: Subject: Re: [PATCH] Optimize macro: make it more predictable To: =?UTF-8?Q?Martin_Li=C5=A1ka?= Cc: GCC Patches , Jakub Jelinek , Michael Matz Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-Spam-Status: No, score=-8.0 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, FREEMAIL_FROM, GIT_PATCH_0, KAM_SHORT, RCVD_IN_DNSWL_NONE, SPF_HELO_NONE, SPF_PASS, TXREP autolearn=ham autolearn_force=no version=3.4.2 X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on server2.sourceware.org X-BeenThere: gcc-patches@gcc.gnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gcc-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 03 Nov 2020 13:28:24 -0000 On Fri, Oct 23, 2020 at 1:47 PM Martin Li=C5=A1ka wrote: > > Hey. > > This is a follow-up of the discussion that happened in thread about no_st= ack_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 optimizat= ion 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-poin= ter 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 s= ame as appending attribute value > to the command line. So far so good. We should also reflect that in docum= entation entry which is quite > vague right now: > > """ > The optimize attribute is used to specify that a function is to be compil= ed 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 a= nd not > with -ftree-vectorize -O1 ? Hmm. I guess the only two reasonable options are to append to the active s= et 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=3D97469 > > 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 =3D j; > + > + /* Merge the decoded options with save_decoded_options. */ > + unsigned save_opt_count =3D save_opt_decoded_options.length (); > + unsigned merged_decoded_options_count =3D save_opt_count + decoded_opt= ions_count; > + cl_decoded_option *merged_decoded_options > + =3D XNEWVEC (cl_decoded_option, merged_decoded_options_count); > + > + for (unsigned i =3D 0; i < save_opt_count; ++i) > + merged_decoded_options[i] =3D save_opt_decoded_options[i]; > + for (unsigned i =3D 0; i < decoded_options_count; ++i) > + merged_decoded_options[save_opt_count + i] =3D 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 save_opt_decoded_options; > + > /* Used to enable -fvar-tracking, -fweb and -frename-registers accordin= g > 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_cou= nt); > > + /* Save Optimization decoded options. */ > + for (unsigned i =3D 0; i < save_decoded_options_count; ++i) > + if (cl_options[save_decoded_options[i].opt_index].flags & CL_OPTIMIZ= ATION) > + 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_opt= ions); > > 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 save_opt_decoded_options; > > class timer; > > -- > 2.28.0 >