Re: [PATCH v3 25/30] target/ppc: Move ADDI, ADDIS to decodetree, implement PADDI

[Richard Henderson]

On 4/30/21 4:23 AM, Luis Fernando Fujita Pires wrote:
> I think we should reconsider using the same .decode file for both 32- and
> 64-bit instructions, to avoid duplicating argument set definitions, and to
> keep the prefixed instructions close to their non-prefixed counterparts.
varinsnwidth assumes there is no easy way to determine, before decoding, the 
width of the instruction.  The way this is implemented in decodetree is vastly 
less optimal than what we can do with a few lines for ppc.

In addition, there's a rough spot in %field definitions.  You can't share those 
between patterns of different sizes, which can get confusing.  Have a look at 
target/rx, and the definitions of %b[23]_r_0, which is the same field for 2 and 
3-byte insns.

The replication of argument set definitions is unfortunate, but in the end will 
only be a handful of lines.  We could probably come up with a way to avoid that 
too, via a decodetree extension, if you really insist.  (My vague idea there 
would put the argument set definitions into a 3rd file, included on the 
decodetree command-line.)

> And, in order to share the trans_PADDI/ADDI implementation, maybe add something 
> to decodetree.py to allow us to specify that an instruction shares the 
> trans_XX() implementation from another one, such as:
> ADDI            001110 ..... ..... ................     @PLS_D_32 !impl=PADDI

This is done by using the same name up front.
If you like, add a comment to give the real instruction name.

PADDI   001110 ..... ..... ................     @PLS_D_32 # ADDI


> This way, we could (and would need to, in fact) keep the 'P' in the prefixed 
> instruction names, but at the same time avoid having extra trans_XX functions 
> just calling another one without any additional code.

I don't understand this at all.



r~