| Message ID | 20210430011543.1017113-26-richard.henderson@linaro.org |
|---|---|
| State | New |
| Headers | show |
| Series | Base for adding PowerPC 64-bit instructions | expand |
From: Richard Henderson <richard.henderson@linaro.org> > +&D rt ra si > +@D ...... rt:5 ra:5 si:s16 &D > + > +# If a prefix is allowed, decode with default values. > +&PLS_D rt ra si:int64_t r:bool > +@PLS_D ...... rt:5 ra:5 si:s16 &PLS_D r=0 > + > +### Fixed-Point Arithmetic Instructions > + > +ADDI 001110 ..... ..... ................ @PLS_D > +ADDIS 001111 ..... ..... ................ @D > diff --git a/target/ppc/insn64.decode b/target/ppc/insn64.decode index > 9fc45d0614..f4272df724 100644 > --- a/target/ppc/insn64.decode > +++ b/target/ppc/insn64.decode > @@ -16,3 +16,18 @@ > # You should have received a copy of the GNU Lesser General Public # License > along with this library; if not, see <http://www.gnu.org/licenses/>. > # > + > +# Many all of these instruction names would be prefixed by "P", # but > +we share code with the non-prefixed instruction. > + > +# Format MLS:D and 8LS:D > +&PLS_D rt ra si:int64_t r:bool !extern > +%pls_si 32:s18 0:16 > +@PLS_D ...... .. ... r:1 .. .................. \ > + ...... rt:5 ra:5 ................ \ > + &PLS_D si=%pls_si > + > +### Fixed-Point Arithmetic Instructions > + > +ADDI 000001 10 0--.-- .................. \ > + 001110 ..... ..... ................ @PLS_D 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. For ADDI/PADDI, something along the lines of: &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 PADDI 000001 10 0--.-- .................. \ 001110 ..... ..... ................ @PLS_D ADDI 001110 ..... ..... ................ @PLS_D_32 ADDIS 001111 ..... ..... ................ @D That's where I was going with the original patch, using the varinsnwidth support from decodetree.py. 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 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. For the load functions, we would then have: %ds_si 2:s14 !function=times_4 @PLS_DS_32 ...... rt:5 ra:5 .............. .. &PLS_D si=%ds_si r=0 &X rt ra rb @X ...... rt:5 ra:5 rb:5 .......... . &X PLBZ 000001 10 0--.-- .................. \ 100010 ..... ..... ................ @PLS_D LBZ 100010 ..... ..... ................ @PLS_D_32 !impl=PLBZ LBZU 100011 ..... ..... ................ @PLS_D_32 LBZX 011111 ..... ..... ..... 0001010111 - @X LBZUX 011111 ..... ..... ..... 0001110111 - @X PLHZ 000001 10 0--.-- .................. \ 101000 ..... ..... ................ @PLS_D LHZ 101000 ..... ..... ................ @PLS_D_32 !impl=PLHZ LHZU 101001 ..... ..... ................ @PLS_D_32 LHZX 011111 ..... ..... ..... 0100010111 - @X LHZUX 011111 ..... ..... ..... 0100110111 - @X PLHA 000001 10 0--.-- .................. \ 101010 ..... ..... ................ @PLS_D LHA 101010 ..... ..... ................ @PLS_D_32 !impl=PLHA LHAU 101011 ..... ..... ................ @PLS_D_32 LHAX 011111 ..... ..... ..... 0101010111 - @X LHAXU 011111 ..... ..... ..... 0101110111 - @X PLWZ 000001 10 0--.-- .................. \ 100000 ..... ..... ................ @PLS_D LWZ 100000 ..... ..... ................ @PLS_D_32 !impl=PLWZ LWZU 100001 ..... ..... ................ @PLS_D_32 LWZX 011111 ..... ..... ..... 0000010111 - @X LWZUX 011111 ..... ..... ..... 0000110111 - @X PLWA 000001 00 0--.-- .................. \ 101001 ..... ..... ................ @PLS_D LWA 111010 ..... ..... ..............10 @PLS_DS_32 !impl=PLWA LWAX 011111 ..... ..... ..... 0101010101 - @X LWAUX 011111 ..... ..... ..... 0101110101 - @X PLD 000001 00 0--.-- .................. \ 111001 ..... ..... ................ @PLS_D LD 111010 ..... ..... ..............00 @PLS_DS_32 !impl=PLD LDU 111010 ..... ..... ..............01 @PLS_DS_32 LDX 011111 ..... ..... ..... 0000010101 - @X LDUX 011111 ..... ..... ..... 0000110101 - @X -- Luis
On 29/04/2021 22:15, Richard Henderson wrote: > Signed-off-by: Richard Henderson <richard.henderson@linaro.org> > --- > target/ppc/insn32.decode | 12 +++++++ > target/ppc/insn64.decode | 15 +++++++++ > target/ppc/translate.c | 29 ---------------- > target/ppc/translate/fixedpoint-impl.c.inc | 39 ++++++++++++++++++++++ > 4 files changed, 66 insertions(+), 29 deletions(-) > > diff --git a/target/ppc/insn32.decode b/target/ppc/insn32.decode > index b175441209..52d9b355d4 100644 > --- a/target/ppc/insn32.decode > +++ b/target/ppc/insn32.decode > @@ -16,3 +16,15 @@ > # You should have received a copy of the GNU Lesser General Public > # License along with this library; if not, see <http://www.gnu.org/licenses/>. > # > + > +&D rt ra si > +@D ...... rt:5 ra:5 si:s16 &D > + > +# If a prefix is allowed, decode with default values. > +&PLS_D rt ra si:int64_t r:bool > +@PLS_D ...... rt:5 ra:5 si:s16 &PLS_D r=0 > + > +### Fixed-Point Arithmetic Instructions > + > +ADDI 001110 ..... ..... ................ @PLS_D > +ADDIS 001111 ..... ..... ................ @D > diff --git a/target/ppc/insn64.decode b/target/ppc/insn64.decode > index 9fc45d0614..f4272df724 100644 > --- a/target/ppc/insn64.decode > +++ b/target/ppc/insn64.decode > @@ -16,3 +16,18 @@ > # You should have received a copy of the GNU Lesser General Public > # License along with this library; if not, see <http://www.gnu.org/licenses/>. > # > + > +# Many all of these instruction names would be prefixed by "P", > +# but we share code with the non-prefixed instruction. > + > +# Format MLS:D and 8LS:D > +&PLS_D rt ra si:int64_t r:bool !extern > +%pls_si 32:s18 0:16 > +@PLS_D ...... .. ... r:1 .. .................. \ > + ...... rt:5 ra:5 ................ \ > + &PLS_D si=%pls_si > + > +### Fixed-Point Arithmetic Instructions > + > +ADDI 000001 10 0--.-- .................. \ > + 001110 ..... ..... ................ @PLS_D I'm not sure about this. It's a bit surprising to find ADDI here, and the comment that explains why is likely to be ignored after the big copyright header. <snip> > diff --git a/target/ppc/translate/fixedpoint-impl.c.inc b/target/ppc/translate/fixedpoint-impl.c.inc > index b740083605..7af1b3bcf5 100644 > --- a/target/ppc/translate/fixedpoint-impl.c.inc > +++ b/target/ppc/translate/fixedpoint-impl.c.inc > @@ -16,3 +16,42 @@ > * You should have received a copy of the GNU Lesser General Public > * License along with this library; if not, see <http://www.gnu.org/licenses/>. > */ > + > +/* > + * Incorporate CIA into the constant when R=1. > + * Validate that when R=1, RA=0. > + */ > +static bool resolve_PLS_D(DisasContext *ctx, arg_PLS_D *a) > +{ > + if (a->r) { > + if (unlikely(a->ra != 0)) { > + gen_invalid(ctx); > + return false; > + } > + a->si += ctx->cia; > + } > + return true; > +} > + > +static bool trans_ADDI(DisasContext *ctx, arg_PLS_D *a) > +{ > + if (resolve_PLS_D(ctx, a)) { > + if (a->ra) { > + tcg_gen_addi_tl(cpu_gpr[a->rt], cpu_gpr[a->ra], a->si); > + } else { > + tcg_gen_movi_tl(cpu_gpr[a->rt], a->si); > + } > + } > + return true; > +} > + I'd prefer to keep a trans_PADDI like > static bool trans_PADDI(DisasContext *ctx, arg_PLS_D *a) > { > if(!resolve_PLS_D(ctx, a)) { > return false; > } > return trans_ADDI(ctx, a); > } It's the middle way between v2 and v3. trans_ADDI code is reused, it'll probably be optimized as a tail call, and resolve_PLS_D is not called when it's not needed. > +static bool trans_ADDIS(DisasContext *ctx, arg_D *a) > +{ > + int si = a->si << 16; > + if (a->ra) { > + tcg_gen_addi_tl(cpu_gpr[a->rt], cpu_gpr[a->ra], si); > + } else { > + tcg_gen_movi_tl(cpu_gpr[a->rt], si); > + } > + return true; > +} > I'd also keep this as in the last version, where trans_ADDI is called. Thanks, Matheus K. Ferst Instituto de Pesquisas ELDORADO <http://www.eldorado.org.br/> Analista de Software Júnior Aviso Legal - Disclaimer <https://www.eldorado.org.br/disclaimer.html>
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~
On 4/30/21 7:05 AM, Matheus K. Ferst wrote: >> +ADDI 000001 10 0--.-- .................. \ >> + 001110 ..... ..... ................ @PLS_D > > I'm not sure about this. It's a bit surprising to find ADDI here, and the > comment that explains why is likely to be ignored after the big copyright header. You could move the comment closer, and replicate, e.g. ADDI .... \ .... @PLS_D # PADDI > I'd prefer to keep a trans_PADDI like > > > static bool trans_PADDI(DisasContext *ctx, arg_PLS_D *a) > > { > > if(!resolve_PLS_D(ctx, a)) { > > return false; > > } > > return trans_ADDI(ctx, a); > > } But in this case ADDI probably doesn't use PLS_D. You could use static bool trans_PADDI(DisasContext *ctx, arg_PLS_D *a) { arg_D d; if (!resolve_PLS_D(ctx, &d, a)) { return false; } return trans_ADDI(ctx, &d); } making sure to use int64_t in the offset for arg_D. > It's the middle way between v2 and v3. trans_ADDI code is reused, it'll > probably be optimized as a tail call, and resolve_PLS_D is not called when it's > not needed. My version won't tail-call, because of the escaping local storage, but I don't see how you can avoid it. r~
On 30/04/2021 11:31, Richard Henderson wrote: > On 4/30/21 7:05 AM, Matheus K. Ferst wrote: >>> +ADDI 000001 10 0--.-- .................. \ >>> + 001110 ..... ..... ................ @PLS_D >> >> I'm not sure about this. It's a bit surprising to find ADDI here, and >> the comment that explains why is likely to be ignored after the big >> copyright header. > > You could move the comment closer, and replicate, e.g. > > ADDI .... \ > .... @PLS_D # PADDI > > If we keep this naming, IMHO moving the comment closer looks better. >> I'd prefer to keep a trans_PADDI like >> >> > static bool trans_PADDI(DisasContext *ctx, arg_PLS_D *a) >> > { >> > if(!resolve_PLS_D(ctx, a)) { >> > return false; >> > } >> > return trans_ADDI(ctx, a); >> > } > > But in this case ADDI probably doesn't use PLS_D. You could use > > static bool trans_PADDI(DisasContext *ctx, arg_PLS_D *a) > { > arg_D d; > if (!resolve_PLS_D(ctx, &d, a)) { > return false; > } > return trans_ADDI(ctx, &d); > } > > making sure to use int64_t in the offset for arg_D. > We'd keep trans_ADDI with the same signature to avoid creating an arg_D on the stack. Patch 4 added type specification, maybe we can define an arg_D within arg_PLD_D? I'll play a bit to see if it works. >> It's the middle way between v2 and v3. trans_ADDI code is reused, >> it'll probably be optimized as a tail call, and resolve_PLS_D is not >> called when it's not needed. > > My version won't tail-call, because of the escaping local storage, but I > don't see how you can avoid it. > > > r~ I haven't been able to test it properly yet, but at least on godbolt it seems that the compiler prefers to inline over tail call, so maybe that's not a problem. Thanks, Matheus K. Ferst Instituto de Pesquisas ELDORADO <http://www.eldorado.org.br/> Analista de Software Júnior Aviso Legal - Disclaimer <https://www.eldorado.org.br/disclaimer.html>
On 4/30/21 11:02 AM, Matheus K. Ferst wrote: >> But in this case ADDI probably doesn't use PLS_D. You could use >> >> static bool trans_PADDI(DisasContext *ctx, arg_PLS_D *a) >> { >> arg_D d; >> if (!resolve_PLS_D(ctx, &d, a)) { >> return false; >> } >> return trans_ADDI(ctx, &d); >> } >> >> making sure to use int64_t in the offset for arg_D. >> > > We'd keep trans_ADDI with the same signature to avoid creating an arg_D on the > stack. Patch 4 added type specification, maybe we can define an arg_D within > arg_PLD_D? I'll play a bit to see if it works. That starts to creep, with e.g. ADDIS now requiring arg_PLD_D. You'll want to audit the other D-form insns to see what other special cases there are. r~
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
On 4/30/21 11:45 AM, Luis Fernando Fujita Pires wrote:
> 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.
Oh, riscv does this via extra_args:.
r~
From: Richard Henderson <richard.henderson@linaro.org> > On 4/30/21 11:45 AM, Luis Fernando Fujita Pires wrote: > > 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. > > Oh, riscv does this via extra_args:. > > r~ The build system probably will fail to detect that a rebuild is needed if the file passed in through extra_args is changed though, right?
On 4/30/21 1:32 PM, Luis Fernando Fujita Pires wrote: > From: Richard Henderson <richard.henderson@linaro.org> >> On 4/30/21 11:45 AM, Luis Fernando Fujita Pires wrote: >>> 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. >> >> Oh, riscv does this via extra_args:. >> >> r~ > > The build system probably will fail to detect that a rebuild is needed if the file passed in through extra_args is changed though, right? Oh, true. Good thing there are patches for riscv to remove that. :-P r~
On 30/04/2021 15:43, Richard Henderson wrote: > On 4/30/21 11:02 AM, Matheus K. Ferst wrote: >>> But in this case ADDI probably doesn't use PLS_D. You could use >>> >>> static bool trans_PADDI(DisasContext *ctx, arg_PLS_D *a) { arg_D >>> d; if (!resolve_PLS_D(ctx, &d, a)) { return false; } return >>> trans_ADDI(ctx, &d); } >>> >>> making sure to use int64_t in the offset for arg_D. >>> >> >> We'd keep trans_ADDI with the same signature to avoid creating an >> arg_D on the stack. Patch 4 added type specification, maybe we can >> define an arg_D within arg_PLD_D? I'll play a bit to see if it >> works. > > That starts to creep, with e.g. ADDIS now requiring arg_PLD_D. You'll > want to audit the other D-form insns to see what other special cases > there are. > > r~ Well, anything that shares implementation with a prefixed instruction would use the prefixed arg. I got arg_D within arg_PLD_D using !function and calling decode_insn32_extract_D. A bit on the ugly side, and probably not worth the effort. Maybe changing decodetree would help, but that's another patch, for another series. Anyway, my compiler (GCC 9.3) is inlining trans_ADDI calls with -O3, so I'd say that your trans_PADDI from the previous message, with trans_ADDI and trans_ADDIS using arg_D, would be the cleaner solution. Thanks, Matheus K. Ferst Instituto de Pesquisas ELDORADO <http://www.eldorado.org.br/> Analista de Software Júnior Aviso Legal - Disclaimer <https://www.eldorado.org.br/disclaimer.html>
diff --git a/target/ppc/insn32.decode b/target/ppc/insn32.decode index b175441209..52d9b355d4 100644 --- a/target/ppc/insn32.decode +++ b/target/ppc/insn32.decode @@ -16,3 +16,15 @@ # You should have received a copy of the GNU Lesser General Public # License along with this library; if not, see <http://www.gnu.org/licenses/>. # + +&D rt ra si +@D ...... rt:5 ra:5 si:s16 &D + +# If a prefix is allowed, decode with default values. +&PLS_D rt ra si:int64_t r:bool +@PLS_D ...... rt:5 ra:5 si:s16 &PLS_D r=0 + +### Fixed-Point Arithmetic Instructions + +ADDI 001110 ..... ..... ................ @PLS_D +ADDIS 001111 ..... ..... ................ @D diff --git a/target/ppc/insn64.decode b/target/ppc/insn64.decode index 9fc45d0614..f4272df724 100644 --- a/target/ppc/insn64.decode +++ b/target/ppc/insn64.decode @@ -16,3 +16,18 @@ # You should have received a copy of the GNU Lesser General Public # License along with this library; if not, see <http://www.gnu.org/licenses/>. # + +# Many all of these instruction names would be prefixed by "P", +# but we share code with the non-prefixed instruction. + +# Format MLS:D and 8LS:D +&PLS_D rt ra si:int64_t r:bool !extern +%pls_si 32:s18 0:16 +@PLS_D ...... .. ... r:1 .. .................. \ + ...... rt:5 ra:5 ................ \ + &PLS_D si=%pls_si + +### Fixed-Point Arithmetic Instructions + +ADDI 000001 10 0--.-- .................. \ + 001110 ..... ..... ................ @PLS_D diff --git a/target/ppc/translate.c b/target/ppc/translate.c index d782a13d27..5a8a3c39c3 100644 --- a/target/ppc/translate.c +++ b/target/ppc/translate.c @@ -924,19 +924,6 @@ GEN_INT_ARITH_ADD(addex, 0x05, cpu_ov, 1, 1, 0); /* addze addze. addzeo addzeo.*/ GEN_INT_ARITH_ADD_CONST(addze, 0x06, 0, cpu_ca, 1, 1, 0) GEN_INT_ARITH_ADD_CONST(addzeo, 0x16, 0, cpu_ca, 1, 1, 1) -/* addi */ -static void gen_addi(DisasContext *ctx) -{ - target_long simm = SIMM(ctx->opcode); - - if (rA(ctx->opcode) == 0) { - /* li case */ - tcg_gen_movi_tl(cpu_gpr[rD(ctx->opcode)], simm); - } else { - tcg_gen_addi_tl(cpu_gpr[rD(ctx->opcode)], - cpu_gpr[rA(ctx->opcode)], simm); - } -} /* addic addic.*/ static inline void gen_op_addic(DisasContext *ctx, bool compute_rc0) { @@ -956,20 +943,6 @@ static void gen_addic_(DisasContext *ctx) gen_op_addic(ctx, 1); } -/* addis */ -static void gen_addis(DisasContext *ctx) -{ - target_long simm = SIMM(ctx->opcode); - - if (rA(ctx->opcode) == 0) { - /* lis case */ - tcg_gen_movi_tl(cpu_gpr[rD(ctx->opcode)], simm << 16); - } else { - tcg_gen_addi_tl(cpu_gpr[rD(ctx->opcode)], - cpu_gpr[rA(ctx->opcode)], simm << 16); - } -} - /* addpcis */ static void gen_addpcis(DisasContext *ctx) { @@ -7029,10 +7002,8 @@ GEN_HANDLER_E(cmpeqb, 0x1F, 0x00, 0x07, 0x00600000, PPC_NONE, PPC2_ISA300), GEN_HANDLER_E(cmpb, 0x1F, 0x1C, 0x0F, 0x00000001, PPC_NONE, PPC2_ISA205), GEN_HANDLER_E(cmprb, 0x1F, 0x00, 0x06, 0x00400001, PPC_NONE, PPC2_ISA300), GEN_HANDLER(isel, 0x1F, 0x0F, 0xFF, 0x00000001, PPC_ISEL), -GEN_HANDLER(addi, 0x0E, 0xFF, 0xFF, 0x00000000, PPC_INTEGER), GEN_HANDLER(addic, 0x0C, 0xFF, 0xFF, 0x00000000, PPC_INTEGER), GEN_HANDLER2(addic_, "addic.", 0x0D, 0xFF, 0xFF, 0x00000000, PPC_INTEGER), -GEN_HANDLER(addis, 0x0F, 0xFF, 0xFF, 0x00000000, PPC_INTEGER), GEN_HANDLER_E(addpcis, 0x13, 0x2, 0xFF, 0x00000000, PPC_NONE, PPC2_ISA300), GEN_HANDLER(mulhw, 0x1F, 0x0B, 0x02, 0x00000400, PPC_INTEGER), GEN_HANDLER(mulhwu, 0x1F, 0x0B, 0x00, 0x00000400, PPC_INTEGER), diff --git a/target/ppc/translate/fixedpoint-impl.c.inc b/target/ppc/translate/fixedpoint-impl.c.inc index b740083605..7af1b3bcf5 100644 --- a/target/ppc/translate/fixedpoint-impl.c.inc +++ b/target/ppc/translate/fixedpoint-impl.c.inc @@ -16,3 +16,42 @@ * You should have received a copy of the GNU Lesser General Public * License along with this library; if not, see <http://www.gnu.org/licenses/>. */ + +/* + * Incorporate CIA into the constant when R=1. + * Validate that when R=1, RA=0. + */ +static bool resolve_PLS_D(DisasContext *ctx, arg_PLS_D *a) +{ + if (a->r) { + if (unlikely(a->ra != 0)) { + gen_invalid(ctx); + return false; + } + a->si += ctx->cia; + } + return true; +} + +static bool trans_ADDI(DisasContext *ctx, arg_PLS_D *a) +{ + if (resolve_PLS_D(ctx, a)) { + if (a->ra) { + tcg_gen_addi_tl(cpu_gpr[a->rt], cpu_gpr[a->ra], a->si); + } else { + tcg_gen_movi_tl(cpu_gpr[a->rt], a->si); + } + } + return true; +} + +static bool trans_ADDIS(DisasContext *ctx, arg_D *a) +{ + int si = a->si << 16; + if (a->ra) { + tcg_gen_addi_tl(cpu_gpr[a->rt], cpu_gpr[a->ra], si); + } else { + tcg_gen_movi_tl(cpu_gpr[a->rt], si); + } + return true; +}
Signed-off-by: Richard Henderson <richard.henderson@linaro.org> --- target/ppc/insn32.decode | 12 +++++++ target/ppc/insn64.decode | 15 +++++++++ target/ppc/translate.c | 29 ---------------- target/ppc/translate/fixedpoint-impl.c.inc | 39 ++++++++++++++++++++++ 4 files changed, 66 insertions(+), 29 deletions(-) -- 2.25.1