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!)

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?

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).

>             * 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?


Segher

Reply via email to