| Message ID | 20211010174401.141339-8-richard.henderson@linaro.org |
|---|---|
| State | New |
| Headers | show |
| Series | tcg: support 32-bit guest addresses as signed | expand |
Hi Richard, On 2021/10/11 01:44, Richard Henderson wrote: > All 32-bit mips operations sign-extend the output, so we are easily > able to keep TCG_TYPE_I32 values sign-extended in host registers. > > Signed-off-by: Richard Henderson <richard.henderson@linaro.org> > --- > tcg/mips/tcg-target-sa32.h | 8 ++++++++ > tcg/mips/tcg-target.c.inc | 13 +++---------- > 2 files changed, 11 insertions(+), 10 deletions(-) > > diff --git a/tcg/mips/tcg-target-sa32.h b/tcg/mips/tcg-target-sa32.h > index cb185b1526..51255e7cba 100644 > --- a/tcg/mips/tcg-target-sa32.h > +++ b/tcg/mips/tcg-target-sa32.h > @@ -1 +1,9 @@ > +/* > + * Do not set TCG_TARGET_SIGNED_ADDR32 for mips32; > + * TCG expects this to only be set for 64-bit hosts. > + */ > +#ifdef __mips64 > +#define TCG_TARGET_SIGNED_ADDR32 1 > +#else > #define TCG_TARGET_SIGNED_ADDR32 0 > +#endif It looks like we never want to set TCG_TARGET_SIGNED_ADDR32 on 32-bit hosts; maybe a compile-time assert could be added somewhere for statically guaranteeing this? > diff --git a/tcg/mips/tcg-target.c.inc b/tcg/mips/tcg-target.c.inc > index d8f6914f03..cc3b4d5b90 100644 > --- a/tcg/mips/tcg-target.c.inc > +++ b/tcg/mips/tcg-target.c.inc > @@ -1161,20 +1161,13 @@ static void tcg_out_tlb_load(TCGContext *s, TCGReg base, TCGReg addrl, > tcg_out_ld(s, TCG_TYPE_I32, TCG_TMP0, TCG_TMP3, cmp_off + LO_OFF); > tcg_out_movi(s, TCG_TYPE_I32, TCG_TMP1, mask); > } else { > - tcg_out_ldst(s, (TARGET_LONG_BITS == 64 ? OPC_LD > - : TCG_TARGET_REG_BITS == 64 ? OPC_LWU : OPC_LW), > - TCG_TMP0, TCG_TMP3, cmp_off); > + tcg_out_ld(s, TCG_TYPE_TL, TCG_TMP0, TCG_TMP3, cmp_off); > tcg_out_movi(s, TCG_TYPE_TL, TCG_TMP1, mask); > /* No second compare is required here; > load the tlb addend for the fast path. */ > tcg_out_ld(s, TCG_TYPE_PTR, TCG_TMP2, TCG_TMP3, add_off); > } > > - /* Zero extend a 32-bit guest address for a 64-bit host. */ > - if (TCG_TARGET_REG_BITS > TARGET_LONG_BITS) { > - tcg_out_ext32u(s, base, addrl); > - addrl = base; > - } > tcg_out_opc_reg(s, OPC_AND, TCG_TMP1, TCG_TMP1, addrl); > > label_ptr[0] = s->code_ptr; > @@ -1456,7 +1449,7 @@ static void tcg_out_qemu_ld(TCGContext *s, const TCGArg *args, bool is_64) > data_regl, data_regh, addr_regl, addr_regh, > s->code_ptr, label_ptr); > #else > - if (TCG_TARGET_REG_BITS > TARGET_LONG_BITS) { > + if (TCG_TARGET_REG_BITS > TARGET_LONG_BITS && !guest_base_signed_addr32) { Is there precedent of extracting predicates like this into some header for reuse? However I agree that the current expression conveys enough meaning without being overly complicated. > tcg_out_ext32u(s, base, addr_regl); > addr_regl = base; > } > @@ -1559,7 +1552,7 @@ static void tcg_out_qemu_st(TCGContext *s, const TCGArg *args, bool is_64) > s->code_ptr, label_ptr); > #else > base = TCG_REG_A0; > - if (TCG_TARGET_REG_BITS > TARGET_LONG_BITS) { > + if (TCG_TARGET_REG_BITS > TARGET_LONG_BITS && !guest_base_signed_addr32) { > tcg_out_ext32u(s, base, addr_regl); > addr_regl = base; > }
On 10/10/21 9:20 PM, WANG Xuerui wrote: >> +++ b/tcg/mips/tcg-target-sa32.h >> @@ -1 +1,9 @@ >> +/* >> + * Do not set TCG_TARGET_SIGNED_ADDR32 for mips32; >> + * TCG expects this to only be set for 64-bit hosts. >> + */ >> +#ifdef __mips64 >> +#define TCG_TARGET_SIGNED_ADDR32 1 >> +#else >> #define TCG_TARGET_SIGNED_ADDR32 0 >> +#endif > It looks like we never want to set TCG_TARGET_SIGNED_ADDR32 on 32-bit > hosts; maybe a compile-time assert could be added somewhere for > statically guaranteeing this? I've placed a build-time assert in tcg/tcg.c. >> - if (TCG_TARGET_REG_BITS > TARGET_LONG_BITS) { >> + if (TCG_TARGET_REG_BITS > TARGET_LONG_BITS && !guest_base_signed_addr32) { > Is there precedent of extracting predicates like this into some header > for reuse? However I agree that the current expression conveys enough > meaning without being overly complicated. Depends on the expected scope of the predicate, I guess. If it's private to tcg-target.c.inc, I'd put it in tcg.c. If it's private to tcg in general, I'd put it in tcg-internal.h. Beyond that, I guess it depends. For this, I don't know what I'd call it that isn't more verbose than the expression itself. r~
diff --git a/tcg/mips/tcg-target-sa32.h b/tcg/mips/tcg-target-sa32.h index cb185b1526..51255e7cba 100644 --- a/tcg/mips/tcg-target-sa32.h +++ b/tcg/mips/tcg-target-sa32.h @@ -1 +1,9 @@ +/* + * Do not set TCG_TARGET_SIGNED_ADDR32 for mips32; + * TCG expects this to only be set for 64-bit hosts. + */ +#ifdef __mips64 +#define TCG_TARGET_SIGNED_ADDR32 1 +#else #define TCG_TARGET_SIGNED_ADDR32 0 +#endif diff --git a/tcg/mips/tcg-target.c.inc b/tcg/mips/tcg-target.c.inc index d8f6914f03..cc3b4d5b90 100644 --- a/tcg/mips/tcg-target.c.inc +++ b/tcg/mips/tcg-target.c.inc @@ -1161,20 +1161,13 @@ static void tcg_out_tlb_load(TCGContext *s, TCGReg base, TCGReg addrl, tcg_out_ld(s, TCG_TYPE_I32, TCG_TMP0, TCG_TMP3, cmp_off + LO_OFF); tcg_out_movi(s, TCG_TYPE_I32, TCG_TMP1, mask); } else { - tcg_out_ldst(s, (TARGET_LONG_BITS == 64 ? OPC_LD - : TCG_TARGET_REG_BITS == 64 ? OPC_LWU : OPC_LW), - TCG_TMP0, TCG_TMP3, cmp_off); + tcg_out_ld(s, TCG_TYPE_TL, TCG_TMP0, TCG_TMP3, cmp_off); tcg_out_movi(s, TCG_TYPE_TL, TCG_TMP1, mask); /* No second compare is required here; load the tlb addend for the fast path. */ tcg_out_ld(s, TCG_TYPE_PTR, TCG_TMP2, TCG_TMP3, add_off); } - /* Zero extend a 32-bit guest address for a 64-bit host. */ - if (TCG_TARGET_REG_BITS > TARGET_LONG_BITS) { - tcg_out_ext32u(s, base, addrl); - addrl = base; - } tcg_out_opc_reg(s, OPC_AND, TCG_TMP1, TCG_TMP1, addrl); label_ptr[0] = s->code_ptr; @@ -1456,7 +1449,7 @@ static void tcg_out_qemu_ld(TCGContext *s, const TCGArg *args, bool is_64) data_regl, data_regh, addr_regl, addr_regh, s->code_ptr, label_ptr); #else - if (TCG_TARGET_REG_BITS > TARGET_LONG_BITS) { + if (TCG_TARGET_REG_BITS > TARGET_LONG_BITS && !guest_base_signed_addr32) { tcg_out_ext32u(s, base, addr_regl); addr_regl = base; } @@ -1559,7 +1552,7 @@ static void tcg_out_qemu_st(TCGContext *s, const TCGArg *args, bool is_64) s->code_ptr, label_ptr); #else base = TCG_REG_A0; - if (TCG_TARGET_REG_BITS > TARGET_LONG_BITS) { + if (TCG_TARGET_REG_BITS > TARGET_LONG_BITS && !guest_base_signed_addr32) { tcg_out_ext32u(s, base, addr_regl); addr_regl = base; }
All 32-bit mips operations sign-extend the output, so we are easily able to keep TCG_TYPE_I32 values sign-extended in host registers. Signed-off-by: Richard Henderson <richard.henderson@linaro.org> --- tcg/mips/tcg-target-sa32.h | 8 ++++++++ tcg/mips/tcg-target.c.inc | 13 +++---------- 2 files changed, 11 insertions(+), 10 deletions(-)