On Tue, 2022-06-07 at 23:16 -0400, Michael Meissner wrote: > On Tue, Jun 07, 2022 at 07:59:34PM -0500, Peter Bergner wrote: > > On 6/7/22 4:24 PM, Segher Boessenkool wrote: > > > On Tue, Jun 07, 2022 at 04:17:04PM -0500, Peter Bergner wrote: > > > > I think I mentioned this offline, but I'd prefer a negative target flag, > > > > something like TARGET_NO_STORE_VECTOR_PAIR that defaults to off, > > > > meaning we'd > > > > generate stxvp by default. > > > > > > NAK. All negatives should be -mno-xxx with -mxxx the corresponding > > > positive. All of them. > > > > That's not what I was asking for. I totally agree that > > -mno-store-vector-pair > > should disable generating stxvp and that -mstore-vector-pair should enable > > generating it. What I asked for was that the internal flag we use to enable > > and disable it should be a negative flag, where TARGET_NO_STORE_VECTOR_PAIR > > is > > true when we use -mno-store-vector-pair and false when using > > -mstore-vector-pair. > > That way we can add that flag to power10's rs6000-cpu.def entry and then > > we're > > done. What I don't want to have to do is that if/when power87 is released, > > we > > still have to add TARGET_STORE_VECTOR_PAIR its rs6000-cpu.def entry just to > > get stxvp insns generated. That adds a cost to every cpu after power10 > > since > > we'd have to remember to add that flag to every follow-on cpu. > > FWIW, I really dislike having negative flags like that (just talking about the > option mask internals, not the user option).
I can't tell there is agreement in either direction, i'll throw some comments out and see if that helps make a decision. I agree with avoiding the negative flags. Whenever I run across a code snippet reading "if (! TARGET_NOT_FOO) ... " it's time to double-check everything. :-) If the proposal is to have "TARGET_NO_STORE_VECTOR_PAIR" set to "off", I'd counter propose whatever variation possible to drop the "NO" from the string. i.e. "TARGET_STORE_VECTOR_PAIR" set to however it makes sense to indicate enabled, or not. All that said, .. with a strong preference to have the internal flags matching the option flags as closely as possible. > > I don't view the cost to add one postive flag to the next CPU as bad, as it > will be a one time cost. Presumably it would be set also next++ CPU. This is > like power8 is all of the power7 flags + new flags. Power9 is all of the > power8 flags + new flags. I.e. in general it is cumulative. Yes, I'm aware > there are times when there are breaks, but hopefully those are rare. This sounds reasonable. Some weight could be added for which way to bias the flag based on a guess of what the 'power87' release will allow, but ultimately that shouldn't really matter. And no, power87 isnt' real AFAIK,.. I'm just repeating the example provided by Peter :-) Thanks -Will > > Otherwise it is like the mess with -mpower8-fusion, where going from power8 to > power9 we have to clear the fusion flag. If store vector pair is a postive > flag, then it isn't set in power10 flags, but it might be set in next cpu > flags. But if it is a negative flag, we have to explicitly clear it. > > We can do it, but I just prefer to go with the positive flag approach. >
