On Tue, Dec 8, 2020 at 10:36 AM Jakub Jelinek <ja...@redhat.com> wrote:
>
> Hi!
>
> The documentation says
> For a named pattern, the condition may not depend on the data in
> the insn being matched, but only the target-machine-type flags.
> The i386 backend violates that by using flag_excess_precision and
> flag_unsafe_math_optimizations in the conditions too, which is bad
> when optimize attribute or pragmas are used. The problem is that the
> middle-end caches the enabled conditions for the optabs for a particular
> switchable target, but multiple functions can share the same
> TARGET_OPTION_NODE, but have different TREE_OPTIMIZATION_NODE with different
> flag_excess_precision or flag_unsafe_math_optimizations, so the enabled
> conditions then match only one of those.
>
> I think best would be to just have a single options node for both the
> generic and target options, then such problems wouldn't exist, but that
> would be very risky at this point and quite large change.
>
> So, instead the following patch just shadows flag_excess_precision and
> flag_unsafe_math_optimizations values for uses in the instruction conditions
> in TargetVariable and during set_cfun artificially creates new
> TARGET_OPTION_NODE if flag_excess_precision and/or
> flag_unsafe_math_optimizations change from what is recorded in their
> TARGET_OPTION_NODE. The target nodes are hashed, so worst case we can get 4
> times as many target option nodes if one would for each unique target option
> try all the flag_excess_precision and flag_unsafe_math_optimizations values.
>
> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
>
> 2020-12-08 Jakub Jelinek <ja...@redhat.com>
>
> PR target/94440
> * config/i386/i386.opt (ix86_excess_precision,
> ix86_unsafe_math_optimizations): New TargetVariables.
> * config/i386/i386.h (X87_ENABLE_ARITH, X87_ENABLE_FLOAT): Use
> ix86_unsafe_math_optimizations instead of
> flag_unsafe_math_optimizations and ix86_excess_precision instead of
> flag_excess_precision.
> * config/i386/i386.c (ix86_excess_precision): Rename to ...
> (ix86_get_excess_precision): ... this.
> (TARGET_C_EXCESS_PRECISION): Define to ix86_get_excess_precision.
> * config/i386/i386-options.c (ix86_valid_target_attribute_tree,
> ix86_option_override_internal): Update ix86_unsafe_math_optimization
> from flag_unsafe_math_optimizations and ix86_excess_precision
> from flag_excess_precision when constructing target option nodes.
> (ix86_set_current_function): If flag_unsafe_math_optimizations
> or flag_excess_precision is different from the one recorded
> in TARGET_OPTION_NODE, create a new target option node for the
> current function and switch to that.
LGTM, but I'm not really experienced in option processing functionality.
Thanks,
Uros.
> --- gcc/config/i386/i386.opt.jj 2020-12-02 14:42:52.195054633 +0100
> +++ gcc/config/i386/i386.opt 2020-12-07 16:05:16.898814331 +0100
> @@ -49,6 +49,16 @@ int recip_mask_explicit
> TargetSave
> int x_recip_mask_explicit
>
> +;; A copy of flag_excess_precision as a target variable that should
> +;; force a different DECL_FUNCTION_SPECIFIC_TARGET upon
> +;; flag_excess_precision changes.
> +TargetVariable
> +enum excess_precision ix86_excess_precision = EXCESS_PRECISION_DEFAULT
> +
> +;; Similarly for flag_unsafe_math_optimizations.
> +TargetVariable
> +bool ix86_unsafe_math_optimizations = false
> +
> ;; Definitions to add to the cl_target_option structure
> ;; -march= processor
> TargetSave
> --- gcc/config/i386/i386.h.jj 2020-12-05 11:37:19.817423434 +0100
> +++ gcc/config/i386/i386.h 2020-12-07 16:17:13.051866670 +0100
> @@ -829,15 +829,15 @@ extern const char *host_detect_local_cpu
> SFmode, DFmode and XFmode) in the current excess precision
> configuration. */
> #define X87_ENABLE_ARITH(MODE) \
> - (flag_unsafe_math_optimizations \
> - || flag_excess_precision == EXCESS_PRECISION_FAST \
> + (ix86_unsafe_math_optimizations \
> + || ix86_excess_precision == EXCESS_PRECISION_FAST \
> || (MODE) == XFmode)
>
> /* Likewise, whether to allow direct conversions from integer mode
> IMODE (HImode, SImode or DImode) to MODE. */
> #define X87_ENABLE_FLOAT(MODE, IMODE) \
> - (flag_unsafe_math_optimizations \
> - || flag_excess_precision == EXCESS_PRECISION_FAST \
> + (ix86_unsafe_math_optimizations \
> + || ix86_excess_precision == EXCESS_PRECISION_FAST \
> || (MODE) == XFmode \
> || ((MODE) == DFmode && (IMODE) == SImode) \
> || (IMODE) == HImode)
> --- gcc/config/i386/i386.c.jj 2020-12-05 11:37:19.000000000 +0100
> +++ gcc/config/i386/i386.c 2020-12-07 16:34:39.460252324 +0100
> @@ -23001,7 +23001,7 @@ ix86_init_libfuncs (void)
> apparently at random. */
>
> static enum flt_eval_method
> -ix86_excess_precision (enum excess_precision_type type)
> +ix86_get_excess_precision (enum excess_precision_type type)
> {
> switch (type)
> {
> @@ -23527,7 +23527,7 @@ ix86_run_selftests (void)
> #define TARGET_MD_ASM_ADJUST ix86_md_asm_adjust
>
> #undef TARGET_C_EXCESS_PRECISION
> -#define TARGET_C_EXCESS_PRECISION ix86_excess_precision
> +#define TARGET_C_EXCESS_PRECISION ix86_get_excess_precision
> #undef TARGET_PROMOTE_PROTOTYPES
> #define TARGET_PROMOTE_PROTOTYPES hook_bool_const_tree_true
> #undef TARGET_SETUP_INCOMING_VARARGS
> --- gcc/config/i386/i386-options.c.jj 2020-12-05 11:37:19.806423559 +0100
> +++ gcc/config/i386/i386-options.c 2020-12-07 16:30:15.719179795 +0100
> @@ -1374,6 +1374,14 @@ ix86_valid_target_attribute_tree (tree f
> /* Add any builtin functions with the new isa if any. */
> ix86_add_new_builtins (opts->x_ix86_isa_flags,
> opts->x_ix86_isa_flags2);
>
> + enum excess_precision orig_ix86_excess_precision
> + = opts->x_ix86_excess_precision;
> + bool orig_ix86_unsafe_math_optimizations
> + = opts->x_ix86_unsafe_math_optimizations;
> + opts->x_ix86_excess_precision = opts->x_flag_excess_precision;
> + opts->x_ix86_unsafe_math_optimizations
> + = opts->x_flag_unsafe_math_optimizations;
> +
> /* Save the current options unless we are validating options for
> #pragma. */
> t = build_target_option_node (opts, opts_set);
> @@ -1382,6 +1390,9 @@ ix86_valid_target_attribute_tree (tree f
> opts->x_ix86_tune_string = orig_tune_string;
> opts_set->x_ix86_fpmath = orig_fpmath_set;
> opts_set->x_prefer_vector_width_type = orig_pvw_set;
> + opts->x_ix86_excess_precision = orig_ix86_excess_precision;
> + opts->x_ix86_unsafe_math_optimizations
> + = orig_ix86_unsafe_math_optimizations;
>
> release_options_strings (option_strings);
> }
> @@ -3019,8 +3030,14 @@ ix86_option_override_internal (bool main
> /* Save the initial options in case the user does function specific
> options. */
> if (main_args_p)
> - target_option_default_node = target_option_current_node
> - = build_target_option_node (opts, opts_set);
> + {
> + opts->x_ix86_excess_precision
> + = opts->x_flag_excess_precision;
> + opts->x_ix86_unsafe_math_optimizations
> + = opts->x_flag_unsafe_math_optimizations;
> + target_option_default_node = target_option_current_node
> + = build_target_option_node (opts, opts_set);
> + }
>
> if (opts->x_flag_cf_protection != CF_NONE)
> opts->x_flag_cf_protection
> @@ -3322,6 +3339,24 @@ ix86_set_current_function (tree fndecl)
> if (TREE_TARGET_GLOBALS (new_tree))
> restore_target_globals (TREE_TARGET_GLOBALS (new_tree));
> else if (new_tree == target_option_default_node)
> + restore_target_globals (&default_target_globals);
> + else
> + TREE_TARGET_GLOBALS (new_tree) = save_target_globals_default_opts ();
> + }
> + else if (flag_unsafe_math_optimizations
> + != TREE_TARGET_OPTION (new_tree)->x_ix86_unsafe_math_optimizations
> + || (flag_excess_precision
> + != TREE_TARGET_OPTION (new_tree)->x_ix86_excess_precision))
> + {
> + cl_target_option_restore (&global_options, &global_options_set,
> + TREE_TARGET_OPTION (new_tree));
> + ix86_excess_precision = flag_excess_precision;
> + ix86_unsafe_math_optimizations = flag_unsafe_math_optimizations;
> + DECL_FUNCTION_SPECIFIC_TARGET (fndecl) = new_tree
> + = build_target_option_node (&global_options, &global_options_set);
> + if (TREE_TARGET_GLOBALS (new_tree))
> + restore_target_globals (TREE_TARGET_GLOBALS (new_tree));
> + else if (new_tree == target_option_default_node)
> restore_target_globals (&default_target_globals);
> else
> TREE_TARGET_GLOBALS (new_tree) = save_target_globals_default_opts ();
> --- gcc/testsuite/gcc.target/i386/pr94440-1.c.jj 2020-12-07
> 16:37:48.216157159 +0100
> +++ gcc/testsuite/gcc.target/i386/pr94440-1.c 2020-12-07 16:37:15.586519343
> +0100
> @@ -0,0 +1,23 @@
> +/* PR target/94440 */
> +/* { dg-do compile { target ia32 } } */
> +/* { dg-options "-mfpmath=sse,387 -msse2 -Ofast -fexcess-precision=standard
> -fno-graphite --param=scev-max-expr-size=0" } */
> +
> +int b;
> +double c, d;
> +
> +void
> +foo (void)
> +{
> + int i;
> + for (i = 0; i < b; i++)
> + c += i;
> +}
> +
> +int __attribute__((optimize(1)))
> +main ()
> +{
> + double a[9];
> + int i;
> + for (i = 0; i < 9; i++)
> + d += a[i];
> +}
> --- gcc/testsuite/gcc.target/i386/pr94440-2.c.jj 2020-12-07
> 16:45:14.920196739 +0100
> +++ gcc/testsuite/gcc.target/i386/pr94440-2.c 2020-12-07 16:45:10.877241650
> +0100
> @@ -0,0 +1,19 @@
> +/* PR target/94440 */
> +/* { dg-do compile } */
> +/* { dg-options "-O0 -msse2 -ffast-math" } */
> +
> +double a;
> +int b;
> +long double c;
> +
> +void
> +foo (void)
> +{
> + float d = (double)(long double)b;
> +}
> +
> +__attribute__((optimize("O0"))) void
> +bar (void)
> +{
> + a = c;
> +}
>
> Jakub
>
