Re: [PATCH] rs6000: Add OPTION_MASK_POWER8 [PR101865]

[Segher Boessenkool <segher@kernel.crashing.org>]

Hi!

On Thu, Apr 11, 2024 at 11:23:02PM -0500, Peter Bergner wrote:
> On 4/11/24 10:31 PM, Kewen.Lin wrote:
> >> +;; This option exists only to create its MASK.  It is not intended for users.
> >> +mdo-not-use-this-option
> >> +Target RejectNegative Mask(POWER8) Var(rs6000_isa_flags) WarnRemoved
> >> +
> > 
> > I can understand the given name is to avoid users to use it, but it looks odd, personally
> > I'm inclined to mpower8 (or even mpower8-internal) even if it's more likely to be used but
> > it's a bit more meaningful (especially we already have mpower10), theoretically speaking
> > it's undocumented users shouldn't use it at all.
> 
> Sorry, I should have mentioned this, but I originally had it -mpower8, but given
> it was an option we don't want users to use, Segher mentioned offline to give it
> a name something like the above and not -mpower8.  I kind of like -mpower8-internal
> now that you mention it, but I'd like Segher's input here whether he prefers
> -mdo-not-use-this-option or -mpower8-internal or something else???

-mpower8-internal is fine.  Anyone who thinks this would be a good thing
to us, well, we cannot stop them from hurting themselves I guess.  Esp.
with a nice help text it is fine.

Going forward we need something like this for most ISA levels (we
currently often use the existence of some more-or-less random insn for
this, but we need the same test to actually test for *that* insn, not a
good thing at all).  But we do not want the user to be able to use such
options at all, so we really shouln't make command line options for it.

So it should not use an option flag at all for this, but something else.
Maybe something new even, we have had problems around this forever, that
suggests we have insufficient abstractions around this :-)

> I'll make the changes above, modulo leaving the option name unchanged until
> we hear from Segher on that and report back on the LE and BE testing.

-mpower8-internal should dissuade users from using it, certainly people
who actually read the documentation as well.  It is unfortunate we need
to tell people to not use tools we provide ourselves, but this is
temporary, right :-)  (Right?!)

Thanks guys,


Segher