On Tue, Feb 20, 2024 at 05:27:07PM +0800, Kewen.Lin wrote:
> > -mcpu=power8 implies -mvsx already.
>
> Yes, but users can specify -mno-vsx in RUNTESTFLAGS, dejagnu
> framework can have different behaviors (options order) for
> different versions, this explicit -mvsx is mainly for the
> consistency between the checking and the actual testing.
It is not supported at all. It is better to assume users do not try
to hang themselves.
> > It is mostly a testsuite patch, and testcase patches are fine (and much
> > wanted!) in stage 4. The actual compiler options remain, and behaviour
> > does not change for anyone who used the option as intended,
>
> Yes, excepting for one unexpected use that users having one cpu type which
> doesn't support power8/power9 capability but meanwhile specifies option
> -mpower{8,9}-vector to gain power8/power9 capability (as currently these
> options can enable the corresponding flags). But I don't think it's an
> expected use case.
Yeah, it is why we do not want such options at all :-)
> >> * config/rs6000/rs6000.opt: Make option power{8,9}-vector as
> >> WarnRemoved.
> >
> > Do we want this, or do we want it silent? Should we remove the options
> > later, if we now warn for it?
>
> Good question, it mainly follows the practice of option direct-move here.
> IMHO at least for power8-vector we want WarnRemoved for now as it's
> documented before, and we can probably make it (or them) removed later on
> trunk once all active branch releases don't support it any more.
>
> What's your opinion on this?
Originally I did
Warn(%qs is deprecated)
which already was a mistake. It then changed to
Deprecated
and then to
WarnRemoved
which make it clearer that it is a bad plan.
If it is okay to remove an option, we should not talk about it at all
anymore. Well maybe warn about it for another release or so, but not
longer.
> >> (define_register_constraint "we"
> >> "rs6000_constraints[RS6000_CONSTRAINT_we]"
> >> - "@internal Like @code{wa}, if @option{-mpower9-vector} and
> >> @option{-m64} are
> >> - used; otherwise, @code{NO_REGS}.")
> >> + "@internal Like @code{wa}, if the cpu type is power9 or up, meanwhile
> >> + @option{-mvsx} and @option{-m64} are used; otherwise, @code{NO_REGS}.")
> >
> > "if this is a POWER9 or later and @option{-mvsx} and @option{-m64} are
> > used". How clumsy. Maybe we should make the patterns that use "we"
> > work without mtvsrdd as well? Hrm, they will still require 64-bit GPRs
> > of course, unless we can do something tricky.
> >
> > We do not need the special constraint at all of course (we can add these
> > conditions to all patterns that use it: all *two* patterns). So maybe
> > that's what we should do :-)
>
> Not sure the original intention introducing it (Mike might know it best), but
> removing it sounds doable.
It is for mtvsrdd.
> btw, it seems more than two patterns using it?
> like (if I didn't miss something):
> - vsx_concat_<mode>
> - vsx_splat_<mode>_reg
> - vsx_splat_v4si_di
> - vsx_mov<mode>_64bit
Yes, it isn't clear we should use this contraint in those last two. It
looks like those do not even need the restriction to 64 bit systems.
Well the last one obviously has that already, but then it could just use
"wa", no?
> > -mcpu=power8 implies -mvsx (power7 already). You can disable VSX, or
> > VMX as well, but by default it is enabled.
>
> Yes, it's meant to consider the explicitly -mno-vsx, which suffers the option
> order issue. But considering we raise error for -mno-vsx -mpower{8,9}-vector
> before, without specifying -mvsx is closer to the previous.
>
> I'll adjust it and the below similar ones, thanks!
It is never supported to do unsupported things :-)
We need to be able to rely on defaults. Otherwise, we will have to
implement all of GCC recursively, in itself, in the testsuite, and in
individual tests. Let's not :-)
Cheers,
Segher
