On 22/7/26 16:08, Luc Michel wrote:
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.

Doh indeed we don't want to update these.


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(-)




Reply via email to