On Sat, Aug 16, 2025 at 10:24 PM H.J. Lu <hjl.to...@gmail.com> wrote:
>
> On Sat, Aug 16, 2025 at 6:43 AM Hongtao Liu <crazy...@gmail.com> wrote:
> >
> > On Sat, Aug 16, 2025 at 8:45 PM Hongtao Liu <crazy...@gmail.com> wrote:
> > >
> > > On Fri, Aug 15, 2025 at 8:48 PM H.J. Lu <hjl.to...@gmail.com> wrote:
> > > >
> > > > On Fri, Aug 15, 2025 at 12:44 AM Hongtao Liu <crazy...@gmail.com> wrote:
> > > > >
> > > > > On Fri, Aug 15, 2025 at 10:07 AM H.J. Lu <hjl.to...@gmail.com> wrote:
> > > > > >
> > > > > > Add target("80387") attribute to enable and disable x87
> > > > > > instructions in a
> > > > > > function.
> > > > > >
> > > > > > gcc/
> > > > > >
> > > > > > PR target/121541
> > > > > > * config/i386/i386-options.cc
> > > > > > (ix86_valid_target_attribute_inner_p): Add target("80387")
> > > > > > attribute. Set the mask bit in opts_set->x_target_flags if
> > > > > > the
> > > > > > mask bit in opts->x_target_flags is updated.
> > > > > > * doc/extend.texi: Document target("80387") function
> > > > > > attribute.
> > > > > >
> > > > > > gcc/testsuite/
> > > > > >
> > > > > > PR target/121541
> > > > > > * gcc.target/i386/pr121541-1a.c: New test.
> > > > > > * gcc.target/i386/pr121541-1b.c: Likewise.
> > > > > > * gcc.target/i386/pr121541-2.c: Likewise.
> > > > > > * gcc.target/i386/pr121541-3.c: Likewise.
> > > > > > * gcc.target/i386/pr121541-4.c: Likewise.
> > > > > > * gcc.target/i386/pr121541-5a.c: Likewise.
> > > > > > * gcc.target/i386/pr121541-5b.c: Likewise.
> > > > > >
> > > > > > Signed-off-by: H.J. Lu <hjl.to...@gmail.com>
> > > > > > ---
> > > > > > gcc/config/i386/i386-options.cc | 6 ++++++
> > > > > > gcc/doc/extend.texi | 5 +++++
> > > > > > gcc/testsuite/gcc.target/i386/pr121541-1a.c | 11 +++++++++++
> > > > > > gcc/testsuite/gcc.target/i386/pr121541-1b.c | 6 ++++++
> > > > > > gcc/testsuite/gcc.target/i386/pr121541-2.c | 11 +++++++++++
> > > > > > gcc/testsuite/gcc.target/i386/pr121541-3.c | 11 +++++++++++
> > > > > > gcc/testsuite/gcc.target/i386/pr121541-4.c | 11 +++++++++++
> > > > > > gcc/testsuite/gcc.target/i386/pr121541-5a.c | 11 +++++++++++
> > > > > > gcc/testsuite/gcc.target/i386/pr121541-5b.c | 6 ++++++
> > > > > > 9 files changed, 78 insertions(+)
> > > > > > create mode 100644 gcc/testsuite/gcc.target/i386/pr121541-1a.c
> > > > > > create mode 100644 gcc/testsuite/gcc.target/i386/pr121541-1b.c
> > > > > > create mode 100644 gcc/testsuite/gcc.target/i386/pr121541-2.c
> > > > > > create mode 100644 gcc/testsuite/gcc.target/i386/pr121541-3.c
> > > > > > create mode 100644 gcc/testsuite/gcc.target/i386/pr121541-4.c
> > > > > > create mode 100644 gcc/testsuite/gcc.target/i386/pr121541-5a.c
> > > > > > create mode 100644 gcc/testsuite/gcc.target/i386/pr121541-5b.c
> > > > > >
> > > > > > diff --git a/gcc/config/i386/i386-options.cc
> > > > > > b/gcc/config/i386/i386-options.cc
> > > > > > index 136c0f2e966..abb5dd7700e 100644
> > > > > > --- a/gcc/config/i386/i386-options.cc
> > > > > > +++ b/gcc/config/i386/i386-options.cc
> > > > > > @@ -1172,6 +1172,10 @@ ix86_valid_target_attribute_inner_p (tree
> > > > > > fndecl, tree args, char *p_strings[],
> > > > > > OPT_mrecip,
> > > > > > MASK_RECIP),
> > > > > >
> > > > > > + IX86_ATTR_YES ("80387",
> > > > > > + OPT_m80387,
> > > > > > + MASK_80387),
> > > > > > +
> > > > > > IX86_ATTR_IX86_YES ("general-regs-only",
> > > > > > OPT_mgeneral_regs_only,
> > > > > > OPTION_MASK_GENERAL_REGS_ONLY),
> > > > > > @@ -1281,6 +1285,8 @@ ix86_valid_target_attribute_inner_p (tree
> > > > > > fndecl, tree args, char *p_strings[],
> > > > > >
> > > > > > else if (type == ix86_opt_yes || type == ix86_opt_no)
> > > > > > {
> > > > > > + opts_set->x_target_flags |= mask;
> > >
> > > I c, should we restrict the |= mask only for if (type == ix86_opt_yes
> > > && mask == MASK_80387)?
>
> Setting opts_set->x_target_flags is the right thing to do even though
> it only affects MASK_80387 today.
Ok for the original patch.
>
> > > > > I don't think we should set opts_set here, opts_set is more readable
> > > > > only.
> > > > > I think we need to call ix86_handle_option here to rewrite opts when
> > > > > x87 is enabled(disable general regs in ix86_handle_optiion)
> > > > > Would that also solve the problem below?
> > > >
> > > > ix86_handle_option always clears MASK_80387 for -mgeneral-regs-only.
> > > > But __attribute__ ((target("80387"))) overrides it by the code below.
> > > >
> > > > For
> > > >
> > > > gcc -O2 -mno-80387 -mgeneral-regs-only ...
> > > >
> > > > we got here by
> > > >
> > > > (gdb) bt
> > > > #0 ix86_option_override_internal (main_args_p=true,
> > > > opts=0x4898d40 <global_options>, opts_set=0x489a700
> > > > <global_options_set>)
> > > > at
> > > > /export/gnu/import/git/gitlab/x86-gcc/gcc/config/i386/i386-options.cc:2383
> > > > #1 0x000000000159c69c in ix86_option_override ()
> > > > at
> > > > /export/gnu/import/git/gitlab/x86-gcc/gcc/config/i386/i386-options.cc:3188
> > > > #2 0x0000000000e9f179 in process_options ()
> > > > at /export/gnu/import/git/gitlab/x86-gcc/gcc/toplev.cc:1293
> > > >
> > > > global_options_set is set by
> > > >
> > > > (gdb) bt
> > > > #0 set_option (opts=0x4898d40 <global_options>,
> > > > opts_set=0x489a700 <global_options_set>, opt_index=2062, value=0,
> > > > arg=0x0, kind=0, loc=0, dc=0x489c180 <global_diagnostic_context>,
> > > > mask=0)
> > > > at /export/gnu/import/git/gitlab/x86-gcc/gcc/opts-common.cc:1763
> > > > #1 0x0000000002cb1041 in handle_option (opts=0x4898d40
> > > > <global_options>,
> > > > opts_set=0x489a700 <global_options_set>, decoded=0x48b5180,
> > > > lang_mask=8,
> > > > kind=0, loc=0, handlers=0x7fffffffd990, generated_p=false,
> > > > dc=0x489c180 <global_diagnostic_context>)
> > > > at /export/gnu/import/git/gitlab/x86-gcc/gcc/opts-common.cc:1313
> > > >
> > > > For __attribute__ ((target("80387"))), we got here by
> > > >
> > > > #0 ix86_option_override_internal (main_args_p=false,
> > > > opts=0x7fffffffb6f0,
> > > > opts_set=0x7fffffff9d30)
> > > > at
> > > > /export/gnu/import/git/gitlab/x86-gcc/gcc/config/i386/i386-options.cc:2383
> > > > #1 0x000000000158f1d9 in ix86_valid_target_attribute_tree (
> > > > fndecl=0x7fffe99cd900, args=0x7fffe99cb398, opts=0x7fffffffb6f0,
> > > > opts_set=0x7fffffff9d30, target_clone_attr=false)
> > > > at
> > > > /export/gnu/import/git/gitlab/x86-gcc/gcc/config/i386/i386-options.cc:1468
> > > > #2 0x000000000158f756 in ix86_valid_target_attribute_p (
> > > > fndecl=0x7fffe99cd900, name=0x7fffe99c9b40, args=0x7fffe99cb398,
> > > > flags=0)
> > > > at
> > > > /export/gnu/import/git/gitlab/x86-gcc/gcc/config/i386/i386-options.cc:1569
> > > > (gdb) f 2
> > > > #2 0x000000000158f756 in ix86_valid_target_attribute_p (
> > > > fndecl=0x7fffe99cd900, name=0x7fffe99c9b40, args=0x7fffe99cb398,
> > > > flags=0)
> > > > at
> > > > /export/gnu/import/git/gitlab/x86-gcc/gcc/config/i386/i386-options.cc:1569
> > > > 1569 = ix86_valid_target_attribute_tree (fndecl, args,
> > > > &func_options,
> > > > (gdb) list
> > > > 1564 cl_target_option_restore (&func_options, &func_options_set,
> > > > 1565 TREE_TARGET_OPTION (old_target));
> > > > 1566
> > > > 1567 /* FLAGS == 1 is used for target_clones attribute. */
> > > > 1568 new_target
> > > > 1569 = ix86_valid_target_attribute_tree (fndecl, args,
> > > > &func_options,
> > > > 1570 &func_options_set, flags == 1);
> > > > ^^^^^^^^^^^^^^^^^^^^^^^^^^ opts_set.
> > > > 1571
> > > > 1572 new_optimize = build_optimization_node (&func_options,
> > > > &func_options_set);
> > > > 1573
> > > >
> > > > func_options_set was set by my patch earlier:
> > > >
> > > > (gdb) bt
> > > > #0 ix86_valid_target_attribute_inner_p (fndecl=0x7fffe99cd900,
> > > > args=0x7fffe99bea40, p_strings=0x7fffffff9ca0, opts=0x7fffffffb6f0,
> > > > opts_set=0x7fffffff9d30, enum_opts_set=0x7fffffff82e0,
> > > > target_clone_attr=false)
> > > > at
> > > > /export/gnu/import/git/gitlab/x86-gcc/gcc/config/i386/i386-options.cc:1288
> > > > #1 0x000000000158e806 in ix86_valid_target_attribute_inner_p (
> > > > fndecl=0x7fffe99cd900, args=0x7fffe99cb398,
> > > > p_strings=0x7fffffff9ca0,
> > > > opts=0x7fffffffb6f0, opts_set=0x7fffffff9d30,
> > > > enum_opts_set=0x7fffffff82e0, target_clone_attr=false)
> > > > at
> > > > /export/gnu/import/git/gitlab/x86-gcc/gcc/config/i386/i386-options.cc:1197
> > > > #2 0x000000000158f072 in ix86_valid_target_attribute_tree (
> > > > fndecl=0x7fffe99cd900, args=0x7fffe99cb398, opts=0x7fffffffb6f0,
> > > > opts_set=0x7fffffff9d30, target_clone_attr=false)
> > > > at
> > > > /export/gnu/import/git/gitlab/x86-gcc/gcc/config/i386/i386-options.cc:1426
> > > > #3 0x000000000158f756 in ix86_valid_target_attribute_p (
> > > > fndecl=0x7fffe99cd900, name=0x7fffe99c9b40, args=0x7fffe99cb398,
> > > > flags=0)
> > > > at
> > > > /export/gnu/import/git/gitlab/x86-gcc/gcc/config/i386/i386-options.cc:1569
> > > >
> > > > It was a mistake for ix86_valid_target_attribute_inner_p not to set
> > > > func_options_set when updating func_options when called from
> > > >
> > > > /* FLAGS == 1 is used for target_clones attribute. */
> > > > new_target
> > > > = ix86_valid_target_attribute_tree (fndecl, args, &func_options,
> > > > &func_options_set, flags == 1);
> > > >
> > > > It wasn't a problem before target("80387") is added since
> > > > ix86_handle_option ignores opts_set:
> > > >
> > > > bool
> > > > ix86_handle_option (struct gcc_options *opts,
> > > > struct gcc_options *opts_set ATTRIBUTE_UNUSED,
> > > > const struct cl_decoded_option *decoded,
> > > > location_t loc)
> > > >
> > > > and opts_set is checked only for MASK_80387.
> > > >
> > > > > /* Don't enable x87 instructions if only general registers are
> > > > > allowed by target("general-regs-only") function attribute
> > > > > or
> > > > > -mgeneral-regs-only. */
> > > > > if (!(opts->x_ix86_target_flags &
> > > > > OPTION_MASK_GENERAL_REGS_ONLY)
> > > > > && !(opts_set->x_target_flags & MASK_80387))
> > Or add adjust this to
> >
> > if (!(opts->x_ix86_target_flags & OPTION_MASK_GENERAL_REGS_ONLY)
> > && !(opts->x_ix86_target_flags & MASK_80387)
>
> It is wrong since the MASK_80387 bit
>
> #define MASK_80387 (1U << 1)
>
> in opts->x_ix86_target_flags is for
>
> #define OPTION_MASK_ISA_64BIT (HOST_WIDE_INT_1U << 1)
>
> > && !(opts_set->x_target_flags & MASK_80387))
> >
> > Does this solve the problem?
> >
> > > > > When MASK_80387 in opts_set and opts are cleared by target("80387")
> > > > > function attribute, MASK_80387 will be turned on again without
> > > > > target("general-regs-only") function attribute nor
> > > > > -mgeneral-regs-only.
> > > > >
> > > > > {
> > > > > if (((processor_alias_table[i].flags & PTA_NO_80387) !=
> > > > > 0))
> > > > > opts->x_target_flags &= ~MASK_80387;
> > > > > else
> > > > > opts->x_target_flags |= MASK_80387;
> > > > >
> > > > >
> > > > > > +
> > > > > > if (type == ix86_opt_no)
> > > > > > opt_set_p = !opt_set_p;
> > > > > >
> > > > > > diff --git a/gcc/doc/extend.texi b/gcc/doc/extend.texi
> > > > > > index 224d6197d63..e2771d7e36a 100644
> > > > > > --- a/gcc/doc/extend.texi
> > > > > > +++ b/gcc/doc/extend.texi
> > > > > > @@ -6798,6 +6798,11 @@ Enable/disable the generation of RCPSS,
> > > > > > RCPPS, RSQRTSS and RSQRTPS
> > > > > > instructions followed an additional Newton-Raphson step instead of
> > > > > > doing a floating-point division.
> > > > > >
> > > > > > +@cindex @code{target("80387")} function attribute, x86
> > > > > > +@item 80387
> > > > > > +@itemx no-80387
> > > > > > +Generate code containing 80387 instructions for floating point.
> > > > > > +
> > > > > > @cindex @code{target("general-regs-only")} function attribute, x86
> > > > > > @item general-regs-only
> > > > > > Generate code which uses only the general registers.
> > > > > > diff --git a/gcc/testsuite/gcc.target/i386/pr121541-1a.c
> > > > > > b/gcc/testsuite/gcc.target/i386/pr121541-1a.c
> > > > > > new file mode 100644
> > > > > > index 00000000000..83884a7b15c
> > > > > > --- /dev/null
> > > > > > +++ b/gcc/testsuite/gcc.target/i386/pr121541-1a.c
> > > > > > @@ -0,0 +1,11 @@
> > > > > > +/* { dg-do compile { target { ! ia32 } } } */
> > > > > > +/* { dg-options "-O2 -march=x86-64" } */
> > > > > > +
> > > > > > +extern long double d;
> > > > > > +
> > > > > > +__attribute__ ((target("no-80387")))
> > > > > > +void
> > > > > > +func1 (void)
> > > > > > +{
> > > > > > + d *= 3; /* { dg-error "x87 register return with x87 disabled" }
> > > > > > */
> > > > > > +}
> > > > > > diff --git a/gcc/testsuite/gcc.target/i386/pr121541-1b.c
> > > > > > b/gcc/testsuite/gcc.target/i386/pr121541-1b.c
> > > > > > new file mode 100644
> > > > > > index 00000000000..f440b14ca71
> > > > > > --- /dev/null
> > > > > > +++ b/gcc/testsuite/gcc.target/i386/pr121541-1b.c
> > > > > > @@ -0,0 +1,6 @@
> > > > > > +/* { dg-do compile { target ia32 } } */
> > > > > > +/* { dg-options "-O2" } */
> > > > > > +
> > > > > > +#include "pr121541-1a.c"
> > > > > > +
> > > > > > +/* { dg-final { scan-assembler "call\[\\t \]+_?__mulxf3" } } */
> > > > > > diff --git a/gcc/testsuite/gcc.target/i386/pr121541-2.c
> > > > > > b/gcc/testsuite/gcc.target/i386/pr121541-2.c
> > > > > > new file mode 100644
> > > > > > index 00000000000..281341e9bb9
> > > > > > --- /dev/null
> > > > > > +++ b/gcc/testsuite/gcc.target/i386/pr121541-2.c
> > > > > > @@ -0,0 +1,11 @@
> > > > > > +/* { dg-do compile } */
> > > > > > +/* { dg-options "-O2 -mno-80387" } */
> > > > > > +
> > > > > > +extern long double d;
> > > > > > +
> > > > > > +__attribute__ ((target("80387")))
> > > > > > +void
> > > > > > +func1 (void)
> > > > > > +{
> > > > > > + d *= 3;
> > > > > > +}
> > > > > > diff --git a/gcc/testsuite/gcc.target/i386/pr121541-3.c
> > > > > > b/gcc/testsuite/gcc.target/i386/pr121541-3.c
> > > > > > new file mode 100644
> > > > > > index 00000000000..380fab2aad9
> > > > > > --- /dev/null
> > > > > > +++ b/gcc/testsuite/gcc.target/i386/pr121541-3.c
> > > > > > @@ -0,0 +1,11 @@
> > > > > > +/* { dg-do compile } */
> > > > > > +/* { dg-options "-O2 -mgeneral-regs-only" } */
> > > > > > +
> > > > > > +extern long double d;
> > > > > > +
> > > > > > +__attribute__ ((target("80387")))
> > > > > > +void
> > > > > > +func1 (void)
> > > > > > +{
> > > > > > + d *= 3;
> > > > > > +}
> > > > > > diff --git a/gcc/testsuite/gcc.target/i386/pr121541-4.c
> > > > > > b/gcc/testsuite/gcc.target/i386/pr121541-4.c
> > > > > > new file mode 100644
> > > > > > index 00000000000..1f4381a52b0
> > > > > > --- /dev/null
> > > > > > +++ b/gcc/testsuite/gcc.target/i386/pr121541-4.c
> > > > > > @@ -0,0 +1,11 @@
> > > > > > +/* { dg-do compile } */
> > > > > > +/* { dg-options "-O2" } */
> > > > > > +
> > > > > > +extern long double d;
> > > > > > +
> > > > > > +__attribute__ ((target("general-regs-only","80387")))
> > > > > > +void
> > > > > > +func1 (void)
> > > > > > +{
> > > > > > + d *= 3;
> > > > > > +}
> > > > > > diff --git a/gcc/testsuite/gcc.target/i386/pr121541-5a.c
> > > > > > b/gcc/testsuite/gcc.target/i386/pr121541-5a.c
> > > > > > new file mode 100644
> > > > > > index 00000000000..e6137e22e98
> > > > > > --- /dev/null
> > > > > > +++ b/gcc/testsuite/gcc.target/i386/pr121541-5a.c
> > > > > > @@ -0,0 +1,11 @@
> > > > > > +/* { dg-do compile { target { ! ia32 } } } */
> > > > > > +/* { dg-options "-O2 -march=x86-64" } */
> > > > > > +
> > > > > > +extern long double d;
> > > > > > +
> > > > > > +__attribute__ ((target("80387","general-regs-only")))
> > > > > > +void
> > > > > > +func1 (void)
> > > > > > +{
> > > > > > + d *= 3; /* { dg-error "x87 register return with x87 disabled" }
> > > > > > */
> > > > > > +}
> > > > > > diff --git a/gcc/testsuite/gcc.target/i386/pr121541-5b.c
> > > > > > b/gcc/testsuite/gcc.target/i386/pr121541-5b.c
> > > > > > new file mode 100644
> > > > > > index 00000000000..b61a7fe6a3f
> > > > > > --- /dev/null
> > > > > > +++ b/gcc/testsuite/gcc.target/i386/pr121541-5b.c
> > > > > > @@ -0,0 +1,6 @@
> > > > > > +/* { dg-do compile { target ia32 } } */
> > > > > > +/* { dg-options "-O2" } */
> > > > > > +
> > > > > > +#include "pr121541-5a.c"
> > > > > > +
> > > > > > +/* { dg-final { scan-assembler "call\[\\t \]+_?__mulxf3" } } */
> > > > > > --
> > > > > > 2.50.1
> > > > > >
> > > > >
> > > > >
> > > > > --
> > > > > BR,
> > > > > Hongtao
> > > >
> > > >
> > > >
> > > > --
> > > > H.J.
> > >
> > >
> > >
> > > --
> > > BR,
> > > Hongtao
> >
> >
> >
> > --
> > BR,
> > Hongtao
>
>
>
> --
> H.J.
--
BR,
Hongtao
