On Tue, 2022-09-20 at 16:14 -0500, Segher Boessenkool wrote: > Hi! > > On Mon, Sep 19, 2022 at 06:19:15PM -0500, will schmidt wrote: > > This is the first of a batch of changes that eliminate a number > > of define TARGET_foo entries we have collected over time. > > Good good :-) > > > TARGET_CTZ is defined as TARGET_MODULO, and has a low number > > of uses. References to TARGET_CTZ should be safe to replace > > with TARGET_MODULO throughout. > > No, please don't. This has nothing to with "modulo". If you want to > say this is just whether we have ISA 3.0 or p9, make a new target > macro > for *that* and use that everywhere. > > This is a general issue, that will make the code much more sane if > you > can fix it!
> > > TARGET_FCTIDZ is entirely unused, and safe to remove. > > Please make separate patches for separate issues. This makes it much > easier to review, and MUCH easier for all other ways we need to > handle > it (backports, reverts, everything else). With Git it is *easier* to > keep separate patches separate than it is to lump it all > together. So, > the trick is to keep things in separate commits during development > already (and you will find more benefits doing that, too!) Yup, I actually developed these three (plus a bunch more) separately, but combined the first three for posting. I'll split them back out and repost after a bit. > > TARGET_FCTIDZ was never used, it always used TARGET_FCFID directly. > > The original PEM mistakenly said this insn is "64-bit only". This > was > fixed in ISA 2.01 . > > > TARGET_FCTIWUZ has a low number of uses, and can be directly > > replaced with TARGET_POPCNTD. > > It is a p7 (ISA 2.06) insn. Please make a TARGET_P7 or such? Yes. I do have a change later in the (unposted) series to replace POPCNTD with POWER7, at a glance thats #17 down the line. In review I agree with your comment that the in-between changes aren't the best choices. I'll see about skipping the in-between values and going straight for POPCNTD->POWER7. I am looking at the TARGET_POWER10 notation as the target style, versus TARGET_P7, but I can go that direction if we think that would be preferred. Maybe it is since this is a retro-fix versus new. :-) > > In the current situation target macros like TARGET_POPCNTD are abused > to > mean either "can we use the popcntd insn", or to mean "can we use > insn > new on p7". Or sometimes something in between, or something in this > general neighbourhood. It is never clear which is meant, which makes > it > very hard to untangle this. But thanks for trying! :-) > > (Don't let me dicsourage you btw, most is pretty straightforward). Absolutely.. I do have this mostly covered locally, I just need to refine a few parts. :-) > > > > * config/rs6000/rs6000.h (TARGET_CTZ): Replace with > > TARGET_MODULO. > > Changelogs are indented with tabs, and this fits on one line. > > So, please make TARGET_P7 and such, and OPTION_MASKs for those in > rs6000-cpus.def? willdo, thanks -Will > > > Segher
