| Message ID | 20230424054105.1579315-7-richard.henderson@linaro.org |
|---|---|
| State | Superseded |
| Headers | show |
| Series | tcg: Simplify calls to load/store helpers | expand |
On 24/4/23 07:40, Richard Henderson wrote: > Test for both base and index; use datahi as a temporary, overwritten > by the final load. Always perform the loads in ascending order, so > that any (user-only) fault sees the correct address. > > Signed-off-by: Richard Henderson <richard.henderson@linaro.org> > --- > tcg/i386/tcg-target.c.inc | 31 +++++++++++++++---------------- > 1 file changed, 15 insertions(+), 16 deletions(-) > > diff --git a/tcg/i386/tcg-target.c.inc b/tcg/i386/tcg-target.c.inc > index b986109d77..794d440a9e 100644 > --- a/tcg/i386/tcg-target.c.inc > +++ b/tcg/i386/tcg-target.c.inc > @@ -2223,23 +2223,22 @@ static void tcg_out_qemu_ld_direct(TCGContext *s, TCGReg datalo, TCGReg datahi, > if (TCG_TARGET_REG_BITS == 64) { > tcg_out_modrm_sib_offset(s, movop + P_REXW + seg, datalo, > base, index, 0, ofs); > + break; > + } > + if (use_movbe) { > + TCGReg t = datalo; > + datalo = datahi; > + datahi = t; > + } > + if (base == datalo || index == datalo) { > + tcg_out_modrm_sib_offset(s, OPC_LEA, datahi, base, index, 0, ofs); > + tcg_out_modrm_offset(s, movop + seg, datalo, datahi, 0); > + tcg_out_modrm_offset(s, movop + seg, datahi, datahi, 4); LGTM but I'd rather have someone fluent with x86 review this one... > } else { > - if (use_movbe) { > - TCGReg t = datalo; > - datalo = datahi; > - datahi = t; > - } > - if (base != datalo) { > - tcg_out_modrm_sib_offset(s, movop + seg, datalo, > - base, index, 0, ofs); > - tcg_out_modrm_sib_offset(s, movop + seg, datahi, > - base, index, 0, ofs + 4); > - } else { > - tcg_out_modrm_sib_offset(s, movop + seg, datahi, > - base, index, 0, ofs + 4); > - tcg_out_modrm_sib_offset(s, movop + seg, datalo, > - base, index, 0, ofs); > - } > + tcg_out_modrm_sib_offset(s, movop + seg, datalo, > + base, index, 0, ofs); > + tcg_out_modrm_sib_offset(s, movop + seg, datahi, > + base, index, 0, ofs + 4); > } > break; > default:
On 4/29/23 14:01, Philippe Mathieu-Daudé wrote: > On 24/4/23 07:40, Richard Henderson wrote: >> Test for both base and index; use datahi as a temporary, overwritten >> by the final load. Always perform the loads in ascending order, so >> that any (user-only) fault sees the correct address. >> >> Signed-off-by: Richard Henderson <richard.henderson@linaro.org> >> --- >> tcg/i386/tcg-target.c.inc | 31 +++++++++++++++---------------- >> 1 file changed, 15 insertions(+), 16 deletions(-) >> >> diff --git a/tcg/i386/tcg-target.c.inc b/tcg/i386/tcg-target.c.inc >> index b986109d77..794d440a9e 100644 >> --- a/tcg/i386/tcg-target.c.inc >> +++ b/tcg/i386/tcg-target.c.inc >> @@ -2223,23 +2223,22 @@ static void tcg_out_qemu_ld_direct(TCGContext *s, TCGReg datalo, >> TCGReg datahi, >> if (TCG_TARGET_REG_BITS == 64) { >> tcg_out_modrm_sib_offset(s, movop + P_REXW + seg, datalo, >> base, index, 0, ofs); >> + break; >> + } >> + if (use_movbe) { >> + TCGReg t = datalo; >> + datalo = datahi; >> + datahi = t; >> + } >> + if (base == datalo || index == datalo) { >> + tcg_out_modrm_sib_offset(s, OPC_LEA, datahi, base, index, 0, ofs); >> + tcg_out_modrm_offset(s, movop + seg, datalo, datahi, 0); >> + tcg_out_modrm_offset(s, movop + seg, datahi, datahi, 4); > > LGTM but I'd rather have someone fluent with x86 review this one... The original address is (base + (index << 0) + ofs). If datalo overlaps either base or index, then we can't use the same form of address for the second load for datahi. So we "Load Effective Address" to perform the computation of the original address once, storing into datahi as temporary (we are guaranteed that datalo != datahi because they're both outputs). After that, the two addresses that we want are (datahi + 0) and (datahi + 4). r~
diff --git a/tcg/i386/tcg-target.c.inc b/tcg/i386/tcg-target.c.inc index b986109d77..794d440a9e 100644 --- a/tcg/i386/tcg-target.c.inc +++ b/tcg/i386/tcg-target.c.inc @@ -2223,23 +2223,22 @@ static void tcg_out_qemu_ld_direct(TCGContext *s, TCGReg datalo, TCGReg datahi, if (TCG_TARGET_REG_BITS == 64) { tcg_out_modrm_sib_offset(s, movop + P_REXW + seg, datalo, base, index, 0, ofs); + break; + } + if (use_movbe) { + TCGReg t = datalo; + datalo = datahi; + datahi = t; + } + if (base == datalo || index == datalo) { + tcg_out_modrm_sib_offset(s, OPC_LEA, datahi, base, index, 0, ofs); + tcg_out_modrm_offset(s, movop + seg, datalo, datahi, 0); + tcg_out_modrm_offset(s, movop + seg, datahi, datahi, 4); } else { - if (use_movbe) { - TCGReg t = datalo; - datalo = datahi; - datahi = t; - } - if (base != datalo) { - tcg_out_modrm_sib_offset(s, movop + seg, datalo, - base, index, 0, ofs); - tcg_out_modrm_sib_offset(s, movop + seg, datahi, - base, index, 0, ofs + 4); - } else { - tcg_out_modrm_sib_offset(s, movop + seg, datahi, - base, index, 0, ofs + 4); - tcg_out_modrm_sib_offset(s, movop + seg, datalo, - base, index, 0, ofs); - } + tcg_out_modrm_sib_offset(s, movop + seg, datalo, + base, index, 0, ofs); + tcg_out_modrm_sib_offset(s, movop + seg, datahi, + base, index, 0, ofs + 4); } break; default:
Test for both base and index; use datahi as a temporary, overwritten by the final load. Always perform the loads in ascending order, so that any (user-only) fault sees the correct address. Signed-off-by: Richard Henderson <richard.henderson@linaro.org> --- tcg/i386/tcg-target.c.inc | 31 +++++++++++++++---------------- 1 file changed, 15 insertions(+), 16 deletions(-)