On Fri, 20 Aug 2021 at 19:47, Richard Henderson
<richard.hender...@linaro.org> wrote:
>
> On 8/20/21 2:03 AM, Peter Maydell wrote:
> >> - } else if (datalo != addend) {
> >> + } else if (scratch_addend) {
> >> tcg_out_ld32_rwb(s, COND_AL, datalo, addend, addrlo);
> >> tcg_out_ld32_12(s, COND_AL, datahi, addend, 4);
> >> } else {
> >
> > I don't understand this change. Yes, we can trash the addend
> > register, but if it's the same as 'datalo' then the second load
> > is not going to DTRT... Shouldn't this be
> > if (scratch_addend && datalo != addend)
> > ?
>
> Previously, addend was *always* a scratch register, TCG_REG_TMP or such.
> Afterward, addend may be TCG_REG_GUEST_BASE, which should not be modified.
> At no point is there overlap between addend and data{hi,lo}.
So the old "datalo == addend" code path was dead code ?
Perhaps if the function now assumes that scratch_addend implies
that datalo != addend it would be worth assert()ing that, in case
some future callsite doesn't realize the restriction ?
thanks
-- PMM
