Re: [PATCH v3 13/14] tcg/arm: Reserve a register for guest_base

[Peter Maydell <peter.maydell@linaro.org>]

On Fri, 20 Aug 2021 at 19:47, Richard Henderson
<richard.henderson@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