[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
RE: [PATCH v3 25/30] target/ppc: Move ADDI, ADDIS to decodetree, impleme
From: |
Luis Fernando Fujita Pires |
Subject: |
RE: [PATCH v3 25/30] target/ppc: Move ADDI, ADDIS to decodetree, implement PADDI |
Date: |
Fri, 30 Apr 2021 18:45:35 +0000 |
From: Richard Henderson <richard.henderson@linaro.org>
> 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.
I tried to solve this with one of the previous decodetree patches ("decodetree:
Allow custom var width load functions"), whose goal was to allow us to
implement a custom instruction load function (in reality, the only effect it
had inside decodetree.py was to not generate the _load function).
So the instruction load would still be handled by a simple function inside
translate.c, but we would use the auto-generated decode() function to call the
trans_XX() functions.
> 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.
Right. In the current patch we're already using separate definitions for 'si'
depending on the format (%pls_si and %ds_si below):
&PLS_D rt ra si:int64_t r:bool
%pls_si 32:s18 0:16
@PLS_D ...... .. ... r:1 .. .................. \
...... rt:5 ra:5 ................ \
&PLS_D si=%pls_si
@PLS_D_32 ...... rt:5 ra:5 si:s16 &PLS_D r=0
%ds_si 2:s14 !function=times_4
@PLS_DS_32 ...... rt:5 ra:5 .............. .. &PLS_D si=%ds_si r=0
And I also had to create separate @formats for 32- and 64-bit versions (@PLS_D,
@PLS_D_32, etc.), which isn't that nice either.
> 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.)
I think we can already pass multiple files to decodetree.py and it will handle
them correctly. I just didn't find a way to do that from the meson build files,
which assume decodetree will always use a single input file.
Another option would be to allow files to be included from inside other .decode
files.
> > 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.
Not a big deal. I was just referring to the fact that, in the current patch,
you noted that the instruction names in insn64.decode were not prefixed by "P"
due to the code sharing with the 32-bit instructions.
Thanks!
Luis
- [PATCH v3 21/30] target/ppc: Use translator_loop_temp_check, (continued)
- [PATCH v3 21/30] target/ppc: Use translator_loop_temp_check, Richard Henderson, 2021/04/29
- [PATCH v3 19/30] target/ppc: Tidy exception vs exit_tb, Richard Henderson, 2021/04/29
- [PATCH v3 24/30] target/ppc: Move page crossing check to ppc_tr_translate_insn, Richard Henderson, 2021/04/29
- [PATCH v3 28/30] target/ppc: Implement prefixed integer load instructions, Richard Henderson, 2021/04/29
- [PATCH v3 29/30] target/ppc: Move D/DS/X-form integer stores to decodetree, Richard Henderson, 2021/04/29
- [PATCH v3 27/30] target/ppc: Move D/DS/X-form integer loads to decodetree, Richard Henderson, 2021/04/29
- [PATCH v3 25/30] target/ppc: Move ADDI, ADDIS to decodetree, implement PADDI, Richard Henderson, 2021/04/29
- RE: [PATCH v3 25/30] target/ppc: Move ADDI, ADDIS to decodetree, implement PADDI, Luis Fernando Fujita Pires, 2021/04/30
- Re: [PATCH v3 25/30] target/ppc: Move ADDI, ADDIS to decodetree, implement PADDI, Richard Henderson, 2021/04/30
- RE: [PATCH v3 25/30] target/ppc: Move ADDI, ADDIS to decodetree, implement PADDI,
Luis Fernando Fujita Pires <=
- Re: [PATCH v3 25/30] target/ppc: Move ADDI, ADDIS to decodetree, implement PADDI, Richard Henderson, 2021/04/30
- RE: [PATCH v3 25/30] target/ppc: Move ADDI, ADDIS to decodetree, implement PADDI, Luis Fernando Fujita Pires, 2021/04/30
- Re: [PATCH v3 25/30] target/ppc: Move ADDI, ADDIS to decodetree, implement PADDI, Richard Henderson, 2021/04/30
Re: [PATCH v3 25/30] target/ppc: Move ADDI, ADDIS to decodetree, implement PADDI, Matheus K. Ferst, 2021/04/30
- Re: [PATCH v3 25/30] target/ppc: Move ADDI, ADDIS to decodetree, implement PADDI, Richard Henderson, 2021/04/30
- Re: [PATCH v3 25/30] target/ppc: Move ADDI, ADDIS to decodetree, implement PADDI, Matheus K. Ferst, 2021/04/30
- Re: [PATCH v3 25/30] target/ppc: Move ADDI, ADDIS to decodetree, implement PADDI, Richard Henderson, 2021/04/30
- Re: [PATCH v3 25/30] target/ppc: Move ADDI, ADDIS to decodetree, implement PADDI, Matheus K. Ferst, 2021/04/30
[PATCH v3 26/30] target/ppc: Implement PNOP, Richard Henderson, 2021/04/29
[PATCH v3 30/30] target/ppc: Implement prefixed integer store instructions, Richard Henderson, 2021/04/29