| Message ID | 20231023160944.10692-8-philmd@linaro.org |
|---|---|
| State | New |
| Headers | show |
| Series | tcg: Use tcg_gen_[s]extract_{i32,i64,tl} | expand |
On 10/23/23 09:09, Philippe Mathieu-Daudé wrote: > Inspired-by: Richard Henderson <richard.henderson@linaro.org> > Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org> > --- > target/mips/tcg/mxu_translate.c | 6 ++---- > 1 file changed, 2 insertions(+), 4 deletions(-) > > diff --git a/target/mips/tcg/mxu_translate.c b/target/mips/tcg/mxu_translate.c > index c517258ac5..6eb73256b2 100644 > --- a/target/mips/tcg/mxu_translate.c > +++ b/target/mips/tcg/mxu_translate.c > @@ -3840,8 +3840,7 @@ static void gen_mxu_Q16SAT(DisasContext *ctx) > tcg_gen_movi_tl(t0, 255); > > gen_set_label(l_lo); > - tcg_gen_shli_tl(t1, mxu_gpr[XRb - 1], 16); > - tcg_gen_sari_tl(t1, t1, 16); > + tcg_gen_sextract_tl(t1, mxu_gpr[XRb - 1], 0, 16); The most simple replacement here is tcg_gen_ext16s_tl. I'll also note that the entire function should be replaced, e.g. TCGv min = tcg_constant_tl(0); TCGv max = tcg_constant_tl(0xff); TCGv tmp[2]; tmp[0] = tcg_temp_new(); tmp[1] = tcg_temp_new(); for (i = 0; i < 4; i++) { int rs = i & 2 ? XRc : XRb; TCGv t = tmp[i & 1]; gen_load_mxu_gpr(t, rs); tcg_gen_sextract_tl(t, t, (i & 1) * 16, 16); tcg_gen_smax_tl(t, t, min); tcg_gen_smin_tl(t, t, max); if (i != 0) { tcg_gen_shli_tl(t, t, i * 8); tcg_gen_or_tl(t, t, tmp[(i - 1) & 1]; } } gen_store_mxu_gpr(tmp[1], XRa); And lots of other similar work within this file. :-/ r~
On 24/10/23 02:14, Richard Henderson wrote: > On 10/23/23 09:09, Philippe Mathieu-Daudé wrote: >> Inspired-by: Richard Henderson <richard.henderson@linaro.org> >> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org> >> --- >> target/mips/tcg/mxu_translate.c | 6 ++---- >> 1 file changed, 2 insertions(+), 4 deletions(-) >> >> diff --git a/target/mips/tcg/mxu_translate.c >> b/target/mips/tcg/mxu_translate.c >> index c517258ac5..6eb73256b2 100644 >> --- a/target/mips/tcg/mxu_translate.c >> +++ b/target/mips/tcg/mxu_translate.c >> @@ -3840,8 +3840,7 @@ static void gen_mxu_Q16SAT(DisasContext *ctx) >> tcg_gen_movi_tl(t0, 255); >> gen_set_label(l_lo); >> - tcg_gen_shli_tl(t1, mxu_gpr[XRb - 1], 16); >> - tcg_gen_sari_tl(t1, t1, 16); >> + tcg_gen_sextract_tl(t1, mxu_gpr[XRb - 1], 0, 16); > > The most simple replacement here is tcg_gen_ext16s_tl. > > I'll also note that the entire function should be replaced, e.g. > > TCGv min = tcg_constant_tl(0); > TCGv max = tcg_constant_tl(0xff); > TCGv tmp[2]; > > tmp[0] = tcg_temp_new(); > tmp[1] = tcg_temp_new(); > > for (i = 0; i < 4; i++) { > int rs = i & 2 ? XRc : XRb; > TCGv t = tmp[i & 1]; > > gen_load_mxu_gpr(t, rs); > tcg_gen_sextract_tl(t, t, (i & 1) * 16, 16); > tcg_gen_smax_tl(t, t, min); > tcg_gen_smin_tl(t, t, max); > if (i != 0) { > tcg_gen_shli_tl(t, t, i * 8); > tcg_gen_or_tl(t, t, tmp[(i - 1) & 1]; > } > } > gen_store_mxu_gpr(tmp[1], XRa); > > > And lots of other similar work within this file. :-/ Yeah, this code is upported from some old tree. It should use the TCG GVec API. See: https://lore.kernel.org/qemu-devel/781894e9-2565-b54f-8df3-9cbd1cf68e2a@linaro.org/
On 24/10/23 10:57, Philippe Mathieu-Daudé wrote: > On 24/10/23 02:14, Richard Henderson wrote: >> On 10/23/23 09:09, Philippe Mathieu-Daudé wrote: >>> Inspired-by: Richard Henderson <richard.henderson@linaro.org> >>> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org> >>> --- >>> target/mips/tcg/mxu_translate.c | 6 ++---- >>> 1 file changed, 2 insertions(+), 4 deletions(-) >>> >>> diff --git a/target/mips/tcg/mxu_translate.c >>> b/target/mips/tcg/mxu_translate.c >>> index c517258ac5..6eb73256b2 100644 >>> --- a/target/mips/tcg/mxu_translate.c >>> +++ b/target/mips/tcg/mxu_translate.c >>> @@ -3840,8 +3840,7 @@ static void gen_mxu_Q16SAT(DisasContext *ctx) >>> tcg_gen_movi_tl(t0, 255); >>> gen_set_label(l_lo); >>> - tcg_gen_shli_tl(t1, mxu_gpr[XRb - 1], 16); >>> - tcg_gen_sari_tl(t1, t1, 16); >>> + tcg_gen_sextract_tl(t1, mxu_gpr[XRb - 1], 0, 16); >> >> The most simple replacement here is tcg_gen_ext16s_tl. >> >> I'll also note that the entire function should be replaced, e.g. >> >> TCGv min = tcg_constant_tl(0); >> TCGv max = tcg_constant_tl(0xff); >> TCGv tmp[2]; >> >> tmp[0] = tcg_temp_new(); >> tmp[1] = tcg_temp_new(); >> >> for (i = 0; i < 4; i++) { >> int rs = i & 2 ? XRc : XRb; >> TCGv t = tmp[i & 1]; >> >> gen_load_mxu_gpr(t, rs); >> tcg_gen_sextract_tl(t, t, (i & 1) * 16, 16); >> tcg_gen_smax_tl(t, t, min); >> tcg_gen_smin_tl(t, t, max); >> if (i != 0) { >> tcg_gen_shli_tl(t, t, i * 8); >> tcg_gen_or_tl(t, t, tmp[(i - 1) & 1]; >> } >> } >> gen_store_mxu_gpr(tmp[1], XRa); I'll tag your suggestion for later, thanks! >> And lots of other similar work within this file. :-/ > > Yeah, this code is upported from some old tree. It should > use the TCG GVec API. See: > https://lore.kernel.org/qemu-devel/781894e9-2565-b54f-8df3-9cbd1cf68e2a@linaro.org/ >
diff --git a/target/mips/tcg/mxu_translate.c b/target/mips/tcg/mxu_translate.c index c517258ac5..6eb73256b2 100644 --- a/target/mips/tcg/mxu_translate.c +++ b/target/mips/tcg/mxu_translate.c @@ -3840,8 +3840,7 @@ static void gen_mxu_Q16SAT(DisasContext *ctx) tcg_gen_movi_tl(t0, 255); gen_set_label(l_lo); - tcg_gen_shli_tl(t1, mxu_gpr[XRb - 1], 16); - tcg_gen_sari_tl(t1, t1, 16); + tcg_gen_sextract_tl(t1, mxu_gpr[XRb - 1], 0, 16); tcg_gen_brcondi_tl(TCG_COND_LT, t1, 0, l_less_lo); tcg_gen_brcondi_tl(TCG_COND_GT, t1, 255, l_greater_lo); tcg_gen_br(l_done); @@ -3876,8 +3875,7 @@ static void gen_mxu_Q16SAT(DisasContext *ctx) tcg_gen_movi_tl(t0, 255); gen_set_label(l_lo); - tcg_gen_shli_tl(t1, mxu_gpr[XRc - 1], 16); - tcg_gen_sari_tl(t1, t1, 16); + tcg_gen_sextract_tl(t1, mxu_gpr[XRc - 1], 0, 16); tcg_gen_brcondi_tl(TCG_COND_LT, t1, 0, l_less_lo); tcg_gen_brcondi_tl(TCG_COND_GT, t1, 255, l_greater_lo); tcg_gen_br(l_done);
Inspired-by: Richard Henderson <richard.henderson@linaro.org> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org> --- target/mips/tcg/mxu_translate.c | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-)