On Mon, Mar 31, 2025 at 9:52 PM Richard Biener <rguent...@suse.de> wrote:
>
> On Mon, 31 Mar 2025, Jakub Jelinek wrote:
>
> > On Mon, Mar 31, 2025 at 03:33:34PM +0200, Richard Biener wrote:
> > > On Mon, 31 Mar 2025, Jakub Jelinek wrote:
> > >
> > > > On Mon, Mar 31, 2025 at 03:12:56PM +0200, Richard Biener wrote:
> > > > > The following fixes ix86_handle_option to properly handle -mno-sse4
> > > > > which is always handled as -msse4 with value unset. I've verified
> > > > > no other OPT_mno_* is left around.
> > > > >
> > > > > Bootstrap and regtest running on x86_64-unknown-linux-gnu.
> > > > >
> > > > > This error goes back quite some time, I'm not sure how prevalent
> > > > > the use of -mno-sse4 is. In fact I have no idea why we have
> > > > > OPT_mno_sse4 at all...
> > > >
> > > > That is because SSE4 is SSE4.1 + SSE4.2.
> > > > So for -msse4 we want it to act as -msse4.2 and for -mno-sse4
> > > > to act as -mno-sse4.1
> > >
> > > Yes, but it seems ix86_handle_option handles this via
> > > OPTION_MASK_ISA_SSE4_SET vs. ~OPTION_MASK_ISA_SSE4_UNSET. So the
> > > question is what the -mno-sse4 options entry does. I'll note
> >
> > But
> > i386-common.cc:#define OPTION_MASK_ISA_SSE4_SET OPTION_MASK_ISA_SSE4_2_SET
> > i386-common.cc:#define OPTION_MASK_ISA_SSE4_UNSET
> > OPTION_MASK_ISA_SSE4_1_UNSET
> > i386-common.cc:#define OPTION_MASK_ISA2_SSE4_UNSET
> > OPTION_MASK_ISA2_SSE4_1_UNSET
> > which is exactly what it wants.
> >
> > > that ix86_handle_option also unsets flags in ix86_isa_flags2
> > > with -mno-sse4 but the mno-sse4 options entry does not indicate
> > > that.
> >
> > It does, that is the
> > opts->x_ix86_isa_flags2 &= ~OPTION_MASK_ISA2_SSE4_UNSET;
> > opts->x_ix86_isa_flags2_explicit |= OPTION_MASK_ISA2_SSE4_UNSET;
> > There is no OPTION_MASK_ISA2_SSE4_SET (like there is no
> > OPTION_MASK_ISA2_SSE{,[234],4_{1,2}}_SET, that isn't needed because
> > all the requirements of SSE4 still fit into ix86_isa_flags.
>
> Yes, all I say is that ix86_handle_option looks OK, I'm not sure
> why we need the
>
> mno-sse4
> Target RejectNegative InverseMask(ISA_SSE4_1) Var(ix86_isa_flags) Save
> Do not support SSE4.1 and SSE4.2 built-in functions and code generation.
>
> options entry. Or whether that means ix86_valid_target_attribute_inner_p
> should have created OPT_mno_sse4 from -mno-sse4. At least, having
> both i386.opt entries looks like unnecessary complication to me.
>
> And maybe I broke -mno-sse4 on the command-line with my change. Indeed
>
> > ./xgcc -B. t.c -mno-sse4
>
> hits the code with OPT_mno_sse4. I'll update the patch to leave
> the old OPT_mno_sse4 handling in place. The other option is
> to fixup ix86_valid_target_attribute_inner_p to when finding
> -mno-sse4 to exactly special case that and use OPT_mno_sse4?
I guess
1) removing RejectNegative in msse4 entry
2) removing mno-sse4 entry in i386.opt
3) Adjust code in ix86_handle_option(what your patch did)
Should fix all the isse?
>
> Again, I don't really understand why it was done in this complicated
> way. At least there seems to be some test coverage.
>
>
> Richard.
>
>Changing ix86_valid_target_attribute_inner_p might be even better because
>OPT_msse4 is RejectNegative option, so !value for it looks weird.
msse4 is defined as ix86_opt_isa in ix86_valid_target_attribute_inner_p
1055 IX86_ATTR_ISA ("sse4", OPT_msse4),
and would be handled in ix86_handle_option
1282 else if (type == ix86_opt_isa)
1283 {
1284 struct cl_decoded_option decoded;
1285
1286 generate_option (opt, NULL, opt_set_p, CL_TARGET,
&decoded);
1287 ix86_handle_option (opts, opts_set,
1288 &decoded, input_location);
1289 }
So I think it's already correct here.
--
BR,
Hongtao
