On 14:02 Wed 22 Jul , Philippe Mathieu-Daudé wrote:
> Hi Luc,
>
> On 10/7/26 09:23, Luc Michel wrote:
> > Since 36a9529e60e0 the TCGv returned by compute_ldst_addr_type{a,b} for
> > the address can be a target register or a constant. However
> > do_{load,store} might modify it in the case of a reversed load/store.
>
> Register are modifiable, aren't they? IIUC compute_ldst_addr_typeb()
> is not used by the reversed opcodes and the only issue is the 0 constant
> in compute_ldst_addr_typea(). Is that analysis correct?
Hi Phil,
The address registers are never modified by a load/store instruction.
(See e.g., https://docs.amd.com/r/en-US/ug984-vivado-microblaze-ref/lw )
36a9529e60e0 introduced an optimization that can return the global TCGv
used for rA or rB storage instead of copying it.
The bug appears in the reversed load/store case, where the
do_load/do_store functions treat the `addr' TCGv_i32 as a temp and
modify it. Since 36a9529e60e0, addr can alias rA, rB, or a 0 TCG
constant. Those 3 cases are buggy.
Thanks
Luc
>
> > Fix this by factoring out the reversed access logic and copying the
> > address in a new TCGv_i32 if it needs to be modified.
> >
> > Fixes: 36a9529e60e0 (target/microblaze: Simplify
> > compute_ldst_addr_type{a,b})
> > Signed-off-by: Luc Michel <[email protected]>
> > ---
> > target/microblaze/translate.c | 56 ++++++++++++++++++++---------------
> > 1 file changed, 32 insertions(+), 24 deletions(-)
>
--
