From 57e113cf7c94a682c29566cb3e0e85955904fd35 Mon Sep 17 00:00:00 2001 From: Jakub Jelinek Date: Thu, 10 Sep 2020 17:39:00 +0200 Subject: [PATCH] arm: Fix up arm_override_options_after_change [PR96939] 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). 2020-09-10 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 | 6 ++---- gcc/testsuite/gcc.target/arm/lto/pr96939_0.c | 15 +++++++++++++++ gcc/testsuite/gcc.target/arm/lto/pr96939_1.c | 10 ++++++++++ 3 files changed, 27 insertions(+), 4 deletions(-) create mode 100644 gcc/testsuite/gcc.target/arm/lto/pr96939_0.c create mode 100644 gcc/testsuite/gcc.target/arm/lto/pr96939_1.c diff --git a/gcc/config/arm/arm.c b/gcc/config/arm/arm.c index dd78141519e..37fc15c837f 100644 --- a/gcc/config/arm/arm.c +++ b/gcc/config/arm/arm.c @@ -3037,10 +3037,6 @@ arm_override_options_after_change_1 (struct gcc_options *opts) 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. */ diff --git a/gcc/testsuite/gcc.target/arm/lto/pr96939_0.c b/gcc/testsuite/gcc.target/arm/lto/pr96939_0.c new file mode 100644 index 00000000000..241ffd5da0a --- /dev/null +++ b/gcc/testsuite/gcc.target/arm/lto/pr96939_0.c @@ -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; +} diff --git a/gcc/testsuite/gcc.target/arm/lto/pr96939_1.c b/gcc/testsuite/gcc.target/arm/lto/pr96939_1.c new file mode 100644 index 00000000000..53c6093e803 --- /dev/null +++ b/gcc/testsuite/gcc.target/arm/lto/pr96939_1.c @@ -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); +} -- 2.30.2