From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mx2.suse.de (mx2.suse.de [195.135.220.15]) by sourceware.org (Postfix) with ESMTPS id 1B5553987958 for ; Fri, 23 Oct 2020 11:47:39 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org 1B5553987958 Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=suse.cz Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=mliska@suse.cz X-Virus-Scanned: by amavisd-new at test-mx.suse.de Received: from relay2.suse.de (unknown [195.135.221.27]) by mx2.suse.de (Postfix) with ESMTP id 1793CACBF; Fri, 23 Oct 2020 11:47:38 +0000 (UTC) From: =?UTF-8?Q?Martin_Li=c5=a1ka?= Subject: [PATCH] Optimize macro: make it more predictable To: gcc-patches@gcc.gnu.org Cc: Richard Biener , Jakub Jelinek , Michael Matz Message-ID: <82e71ebf-7b2e-67e7-1f08-ea525deee4cb@suse.cz> Date: Fri, 23 Oct 2020 13:47:37 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.3.2 MIME-Version: 1.0 Content-Type: text/plain; charset=windows-1252; format=flowed Content-Language: en-US Content-Transfer-Encoding: 7bit X-Spam-Status: No, score=-10.8 required=5.0 tests=BAYES_00, GIT_PATCH_0, KAM_DMARC_STATUS, KAM_SHORT, RCVD_IN_MSPIKE_H3, RCVD_IN_MSPIKE_WL, 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: Fri, 23 Oct 2020 11:47:40 -0000 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. --- 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 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 save_opt_decoded_options; class timer; -- 2.28.0