On Fri, May 28, 2021 at 11:48 AM Martin Liška <mli...@suse.cz> wrote: > > Hi. > > There's a fallout after my revision ebd5e86c0f41dc1d692f9b2b68a510b1f6835a3e. > I would like to analyze > all case and discuss possible solution. To be honest it's a can of worms and > reverting the commit > is an option on the table. > > So the cases: > > 1) PR100759 - ppc64le > > $ cat pr.C > #pragma GCC optimize 0 > void main(); > > $ ./xgcc -B. -Os pr.C > pr.C:2:11: internal compiler error: ‘global_options’ are modified in local > context > 2 | void main(); > > What happens: we change from -Os to -O0 and rs6000_isa_flags differ in > cl_optimization_compare. > Problem is that OPTION_MASK_SAVE_TOC_INDIRECT is set based on optimize flag: > > /* If we can shrink-wrap the TOC register save separately, then use > -msave-toc-indirect unless explicitly disabled. */ > if ((rs6000_isa_flags_explicit & OPTION_MASK_SAVE_TOC_INDIRECT) == 0 > && flag_shrink_wrap_separate > && optimize_function_for_speed_p (cfun)) > rs6000_isa_flags |= OPTION_MASK_SAVE_TOC_INDIRECT;
So that means that /* Restore current options. */ cl_optimization_restore (&global_options, &global_options_set, &cur_opts); cl_target_option_restore (&global_options, &global_options_set, TREE_TARGET_OPTION (prev_target_node)); does not result in the same outcome as the original command-line processing? Given both restore processes could interact (not sure if that's the issue here) shouldn't we just have a single restore operation and a single target hook instead of both targetm.override_options_after_change and targetm.target_option.restore? Likewise we should probably _always_ set both, DECL_FUNCTION_SPECIFIC_OPT and _TARGET as a step towards unifying them. That said, for the above case a more detailed run-down as to how things go wrong would be nice to see. > Suggested solution is doing: > > if ((rs6000_isa_flags_explicit & OPTION_MASK_SAVE_TOC_INDIRECT) == 0 > && flag_shrink_wrap_separate > rs6000_isa_flags |= OPTION_MASK_SAVE_TOC_INDIRECT; > > and add '&& optimize_function_for_speed_p (cfun)' to the place where the > option mask is used. > > 2) Joseph's case: > > $ cat ~/Programming/testcases/opts-bug.i > extern unsigned long int x; > extern float f (float); > extern __typeof (f) f_power8; > extern __typeof (f) f_power9; > extern __typeof (f) f __attribute__ ((ifunc ("f_ifunc"))); > static __attribute__ ((optimize ("-fno-stack-protector"))) __typeof (f) * > f_ifunc (void) > { > __typeof (f) *res = x ? f_power9 : f_power8; > return res; > } > > $ ./xgcc -B. ~/Programming/testcases/opts-bug.i -c -S -O2 -mlong-double-128 > -mabi=ibmlongdouble > /home/marxin/Programming/testcases/opts-bug.i:8:1: error: > ‘-mabi=ibmlongdouble’ requires ‘-mlong-double-128’ > > This is caused by a weird option override: > > else if (rs6000_long_double_type_size == 128) > rs6000_long_double_type_size = FLOAT_PRECISION_TFmode; (it's 127) > > later when rs6000_option_override_internal is called for saved target flags > (127), it complains. > Possible fix: > > else if (rs6000_long_double_type_size == 128 > || rs6000_long_double_type_size == FLOAT_PRECISION_TFmode) > > 3) ARM issue reported here: > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=98636#c20 > > arm_fp16_inst = bitmap_bit_p (arm_active_target.isa, isa_bit_fp16); > if (arm_fp16_inst) > { > if (arm_fp16_format == ARM_FP16_FORMAT_ALTERNATIVE) > error ("selected fp16 options are incompatible"); > arm_fp16_format = ARM_FP16_FORMAT_IEEE; > } > > there's likely missing else branch which would reset when arm_fp16_inst is > null. > Anyway, can be moved again to the ignored list > > 4) Jeff reported the following for v850-elf: > > $ cat ~/Programming/testcases/j.c > typedef __SIZE_TYPE__ size_t; > > extern inline __attribute__ ((__always_inline__, __gnu_inline__, > __artificial__, __nothrow__, __leaf__)) void * > memcpy (void *__restrict __dest, const void *__restrict __src, size_t __len) > { > return __builtin___memcpy_chk (__dest, __src, __len, __builtin_object_size > (__dest, 0)); > } > > __attribute__((optimize ("Ofast"))) void > bar (void *d, void *s, size_t l) > { > memcpy (d, s, l); > } > > $ ./xgcc -B. ~/Programming/testcases/j.c -S > /home/marxin/Programming/testcases/j.c: In function ‘bar’: > /home/marxin/Programming/testcases/j.c:4:1: error: inlining failed in call to > ‘always_inline’ ‘memcpy’: target specific option mismatch > 4 | memcpy (void *__restrict __dest, const void *__restrict __src, > size_t __len) > | ^~~~~~ > /home/marxin/Programming/testcases/j.c:12:3: note: called from here > 12 | memcpy (d, s, l); > | ^~~~~~~~~~~~~~~~ > > This one is pretty clear. The target does: > > { OPT_LEVELS_1_PLUS, OPT_mprolog_function, NULL, 1 }, > > So it sets a target option based on optimize level. > This one will need: > > diff --git a/gcc/config/v850/v850.c b/gcc/config/v850/v850.c > index e0e5005d865..49f91f12766 100644 > --- a/gcc/config/v850/v850.c > +++ b/gcc/config/v850/v850.c > @@ -3140,6 +3140,11 @@ v850_option_override (void) > /* The RH850 ABI does not (currently) support the use of the CALLT > instruction. */ > if (! TARGET_GCC_ABI) > target_flags |= MASK_DISABLE_CALLT; > + > + /* Save the initial options in case the user does function specific > + options. */ > + target_option_default_node = target_option_current_node > + = build_target_option_node (&global_options, &global_options_set); > } > > plus a custom can_inline_p target hook where the MASK_PROLOG_FUNCTION is > ignored because > caller does not have it set, while callee has. > > What target maintainers thing about it? > > Martin