From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from us-smtp-1.mimecast.com (us-smtp-2.mimecast.com [207.211.31.81]) by sourceware.org (Postfix) with ESMTP id 61E823857C5A for ; Tue, 8 Sep 2020 08:45:22 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org 61E823857C5A Received: from mimecast-mx01.redhat.com (mimecast-mx01.redhat.com [209.132.183.4]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-129-auQ3vINuONSqahGCDMntOQ-1; Tue, 08 Sep 2020 04:45:18 -0400 X-MC-Unique: auQ3vINuONSqahGCDMntOQ-1 Received: from smtp.corp.redhat.com (int-mx08.intmail.prod.int.phx2.redhat.com [10.5.11.23]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id 80DBC18B9F4C; Tue, 8 Sep 2020 08:45:16 +0000 (UTC) Received: from tucnak.zalov.cz (ovpn-112-247.ams2.redhat.com [10.36.112.247]) by smtp.corp.redhat.com (Postfix) with ESMTPS id 1802527CB7; Tue, 8 Sep 2020 08:45:15 +0000 (UTC) Received: from tucnak.zalov.cz (localhost [127.0.0.1]) by tucnak.zalov.cz (8.15.2/8.15.2) with ESMTP id 0888jDaI015209; Tue, 8 Sep 2020 10:45:13 +0200 Received: (from jakub@localhost) by tucnak.zalov.cz (8.15.2/8.15.2/Submit) id 0888jCp7015208; Tue, 8 Sep 2020 10:45:12 +0200 Date: Tue, 8 Sep 2020 10:45:12 +0200 From: Jakub Jelinek To: Richard Earnshaw , Kyrylo Tkachov Cc: gcc-patches@gcc.gnu.org Subject: [PATCH] arm: Fix up arm_override_options_after_change [PR96939] Message-ID: <20200908084512.GH18149@tucnak> Reply-To: Jakub Jelinek MIME-Version: 1.0 User-Agent: Mutt/1.11.3 (2019-02-01) X-Scanned-By: MIMEDefang 2.84 on 10.5.11.23 X-Mimecast-Spam-Score: 0.003 X-Mimecast-Originator: redhat.com Content-Type: text/plain; charset=us-ascii Content-Disposition: inline X-Spam-Status: No, score=-6.8 required=5.0 tests=BAYES_00, DKIMWL_WL_HIGH, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, KAM_SHORT, RCVD_IN_DNSWL_NONE, RCVD_IN_MSPIKE_H2, 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, 08 Sep 2020 08:45:23 -0000 Hi! As mentioned in the PR, the testcase fails to link, because when set_cfun is being called on the crc function, arm_override_options_after_change is called from set_cfun -> invoke_set_current_function_hook: /* Change optimization options if needed. */ if (optimization_current_node != opts) { optimization_current_node = opts; cl_optimization_restore (&global_options, TREE_OPTIMIZATION (opts)); } and at that point target_option_default_node actually matches even the current state of options, so this means armv7 (or whatever) arch is set as arm_active_target, then targetm.set_current_function (fndecl); is called later in that function, which because the crc function's DECL_FUNCTION_SPECIFIC_TARGET is different from the current one will do: cl_target_option_restore (&global_options, TREE_TARGET_OPTION (new_tree)); which calls arm_option_restore and sets arm_active_target to armv8-a+crc (so far so good). Later arm_set_current_function calls: save_restore_target_globals (new_tree); which in this case calls: /* Call target_reinit and save the state for TARGET_GLOBALS. */ TREE_TARGET_GLOBALS (new_tree) = save_target_globals_default_opts (); which because optimization_current_node != optimization_default_node (the testcase is LTO, so all functions have their DECL_FUNCTION_SPECIFIC_TARGET and TREE_OPTIMIZATION nodes) will call: cl_optimization_restore (&global_options, TREE_OPTIMIZATION (optimization_default_node)); and cl_optimization_restore (&global_options, TREE_OPTIMIZATION (opts)); The problem is that these call arm_override_options_after_change again, and that one uses the target_option_default_node as what to set the arm_active_target to (i.e. back to armv7 or whatever, but not to the armv8-a+crc that should be the active target for the crc function). That means we then error on the builtin call in that function. Now, the targetm.override_options_after_change hook is called always at the end of cl_optimization_restore, i.e. when we change the Optimization marked generic options. So it seems unnecessary to call arm_configure_build_target at that point (nothing it depends on changed), and additionally incorrect (because it uses the target_option_default_node, rather than the current set of options; we'd need to revert https://gcc.gnu.org/legacy-ml/gcc-patches/2016-12/msg01390.html otherwise so that it works again with global_options otherwise). The options that arm_configure_build_target cares about will change only during option parsing (which is where it is called already), or during arm_set_current_function, where it is done during the cl_target_option_restore. Now, arm_override_options_after_change_1 wants to adjust the str_align_functions, which depends on the current Optimization options (e.g. optimize_size and flag_align_options and str_align_functions) as well as the target options target_flags, so IMHO needs to be called both when the Optimization options (possibly) change, i.e. from the targetm.override_options_after_change hook, and from when the target options change (set_current_function hook). Bootstrapped/regtested on armv7hl-linux-gnueabi, ok for trunk? Looking further at arm_override_options_after_change_1, it also seems to be incorrect, rather than testing !opts->x_str_align_functions it should be really testing !opts_set->x_str_align_functions and get &global_options_set or similar passed to it as additional opts_set argument. That is because otherwise the decision will be sticky, while it should be done whenever use provided -falign-functions but didn't provide -falign-functions= (either on the command line, or through optimize attribute or pragma). 2020-09-08 Jakub Jelinek PR target/96939 * config/arm/arm.c (arm_override_options_after_change): Don't call arm_configure_build_target here. (arm_set_current_function): Call arm_override_options_after_change_1 at the end. * gcc.target/arm/lto/pr96939_0.c: New test. * gcc.target/arm/lto/pr96939_1.c: New file. --- gcc/config/arm/arm.c.jj 2020-07-30 15:04:38.136293101 +0200 +++ gcc/config/arm/arm.c 2020-09-07 10:43:54.809561852 +0200 @@ -3037,10 +3037,6 @@ arm_override_options_after_change_1 (str static void arm_override_options_after_change (void) { - arm_configure_build_target (&arm_active_target, - TREE_TARGET_OPTION (target_option_default_node), - &global_options_set, false); - arm_override_options_after_change_1 (&global_options); } @@ -32338,6 +32334,8 @@ arm_set_current_function (tree fndecl) cl_target_option_restore (&global_options, TREE_TARGET_OPTION (new_tree)); save_restore_target_globals (new_tree); + + arm_override_options_after_change_1 (&global_options); } /* Implement TARGET_OPTION_PRINT. */ --- gcc/testsuite/gcc.target/arm/lto/pr96939_0.c.jj 2020-09-07 11:26:45.909937609 +0200 +++ gcc/testsuite/gcc.target/arm/lto/pr96939_0.c 2020-09-07 11:29:18.722706535 +0200 @@ -0,0 +1,15 @@ +/* PR target/96939 */ +/* { dg-lto-do link } */ +/* { dg-require-effective-target arm_arch_v8a_ok } */ +/* { dg-lto-options { { -flto -O2 } } } */ + +extern unsigned crc (unsigned, const void *); +typedef unsigned (*fnptr) (unsigned, const void *); +volatile fnptr fn; + +int +main () +{ + fn = crc; + return 0; +} --- gcc/testsuite/gcc.target/arm/lto/pr96939_1.c.jj 2020-09-07 11:26:49.365887153 +0200 +++ gcc/testsuite/gcc.target/arm/lto/pr96939_1.c 2020-09-07 11:25:13.885281180 +0200 @@ -0,0 +1,10 @@ +/* PR target/96939 */ +/* { dg-options "-march=armv8-a+crc" } */ + +#include + +unsigned +crc (unsigned x, const void *y) +{ + return __crc32cw (x, *(unsigned *) y); +} Jakub