| Message ID | 20240607144921.726730-1-rathc@linux.ibm.com |
|---|---|
| Headers | show |
| Series | target/ppc: Move VSX storage access and compare | expand |
On 6/7/24 07:49, Chinmay Rath wrote: > +static bool do_ld_st_vl(DisasContext *ctx, arg_X *a, > + void (*helper)(TCGv_ptr, TCGv, TCGv_ptr, TCGv)) > +{ > + TCGv EA; > + TCGv_ptr xt; > + if (a->rt < 32) { > + REQUIRE_VSX(ctx); > + } else { > + REQUIRE_VECTOR(ctx); > + } > + xt = gen_vsr_ptr(a->rt); > + gen_set_access_type(ctx, ACCESS_INT); > + > + if (a->ra) { > + EA = tcg_temp_new(); > + tcg_gen_mov_tl(EA, cpu_gpr[a->ra]); > + } else { > + EA = tcg_constant_tl(0); > + } > + if (NARROW_MODE(ctx)) { > + tcg_gen_ext32u_tl(EA, EA); ra == 0, narrow mode, will crash, due to write into constant 0. Obviously 0 does not need extending, so this could be if (!a->ra) { ea = constant 0; } else if (narrow mode) { ea = tcg_temp_new(); tcg_gen_ext32u_tl(ea, cpu_gpr[a->ra]); } else { ra = cpu_gpr[a->ra]; } Aren't there existing helper functions for computing this address? And if not, better to create one. r~
Hi Richard, On 6/7/24 21:11, Richard Henderson wrote: > On 6/7/24 07:49, Chinmay Rath wrote: >> +static bool do_ld_st_vl(DisasContext *ctx, arg_X *a, >> + void (*helper)(TCGv_ptr, TCGv, TCGv_ptr, TCGv)) >> +{ >> + TCGv EA; >> + TCGv_ptr xt; >> + if (a->rt < 32) { >> + REQUIRE_VSX(ctx); >> + } else { >> + REQUIRE_VECTOR(ctx); >> + } >> + xt = gen_vsr_ptr(a->rt); >> + gen_set_access_type(ctx, ACCESS_INT); >> + >> + if (a->ra) { >> + EA = tcg_temp_new(); >> + tcg_gen_mov_tl(EA, cpu_gpr[a->ra]); >> + } else { >> + EA = tcg_constant_tl(0); >> + } >> + if (NARROW_MODE(ctx)) { >> + tcg_gen_ext32u_tl(EA, EA); > > ra == 0, narrow mode, will crash, due to write into constant 0. > Obviously 0 does not need extending, so this could be > > if (!a->ra) { > ea = constant 0; > } else if (narrow mode) { > ea = tcg_temp_new(); > tcg_gen_ext32u_tl(ea, cpu_gpr[a->ra]); > } else { > ra = cpu_gpr[a->ra]; > } > ^ Thank you Richard, will take care in v2. > > Aren't there existing helper functions for computing this address? > And if not, better to create one. ^ The calculation of effective address in these instructions is slightly different than the others, for which helper function exist : EA for these insns : EA ← (RA=0) ? 0 : GPR[RA] EA for rest storage access insns : EA ← ((RA=0) ? 0 : GPR[RA]) + GPR[RB] This is why I could not reuse that function. Also, this calculation of EA is limited to these 4 insns above, and only 2 others (prefixed insns), which is why I did not create a new function for this, considering it won't be reused for any other insn. Please let me know if I should create a new function in this case as well. Thanks and Regards, Chinmay > > > r~ >
On 6/9/24 11:11, Chinmay Rath wrote: > The calculation of effective address in these instructions is slightly different than the > others, > for which helper function exist : > > EA for these insns : EA ← (RA=0) ? 0 : GPR[RA] > EA for rest storage access insns : EA ← ((RA=0) ? 0 : GPR[RA]) + GPR[RB] > > This is why I could not reuse that function. Also, this calculation of EA is limited to these > 4 insns above, and only 2 others (prefixed insns), which is why I did not create a new > function > for this, considering it won't be reused for any other insn. > > Please let me know if I should create a new function in this case as well. If you expect this to be used just once, then leaving it inline is perfectly fine. r~
Moving all remaining VSX storage access instructions and all VSX compare instructions of XX3 form with RC field, to decodetree specification. Chinmay Rath (4): target/ppc: Moving VSX scalar storage access insns to decodetree. target/ppc: Move VSX vector with length storage access insns to decodetree. target/ppc: Move VSX vector storage access insns to decodetree. target/ppc: Move VSX fp compare insns to decodetree. target/ppc/helper.h | 24 +- target/ppc/insn32.decode | 41 +++ target/ppc/fpu_helper.c | 16 +- target/ppc/mem_helper.c | 8 +- target/ppc/translate/vsx-impl.c.inc | 430 ++++++++++++++-------------- target/ppc/translate/vsx-ops.c.inc | 49 ---- 6 files changed, 286 insertions(+), 282 deletions(-)