arm: couple of finalize_memop related oddities

arm: couple of finalize_memop related oddities

[Peter Maydell]

I noticed a couple of finalize_memop related oddities while I
was rebasing my decodetree series:

(1) in disas_ldst_reg_imm9(), we calculate a memop, but then
when we call gen_mte_check1_mmuidx() we don't pass the memop
as that function's memop argument, we just pass size. Everywhere
else that calls gen_mte_check* functions passes memop. Intentional?

(2) disas_ldst_reg_roffset() and disas_ldst_reg_unsigned_imm()
use finalize_memop() for both vector and normal register
loads/stores. Should they be using finalize_memop_asimd()
for the vector versions?

(3) disas_ldst_multiple_struct() and disas_ldst_single_struct()
use finalize_memop() even though they always load/store
vector registers. Should they be using finalize_memop_asimd() ?

thanks
-- PMM

Re: arm: couple of finalize_memop related oddities

[Richard Henderson]

On 6/9/23 03:47, Peter Maydell wrote:
> I noticed a couple of finalize_memop related oddities while I
> was rebasing my decodetree series:
> 
> (1) in disas_ldst_reg_imm9(), we calculate a memop, but then
> when we call gen_mte_check1_mmuidx() we don't pass the memop
> as that function's memop argument, we just pass size. Everywhere
> else that calls gen_mte_check* functions passes memop. Intentional?

No, looks like a bug.

> (2) disas_ldst_reg_roffset() and disas_ldst_reg_unsigned_imm()
> use finalize_memop() for both vector and normal register
> loads/stores. Should they be using finalize_memop_asimd()
> for the vector versions?
> 
> (3) disas_ldst_multiple_struct() and disas_ldst_single_struct()
> use finalize_memop() even though they always load/store
> vector registers. Should they be using finalize_memop_asimd() ?

Yes.

Everywhere that uses CreateAccDescASIMD in the pseudocode should use finalize_memop_asimd 
for clarity.  (If size is never 128-bit then it will be same as just finalize_memop, so 
some of those places were not actual bugs.  But definitely unclear.)


r~

Re: arm: couple of finalize_memop related oddities

[Peter Maydell]

On Fri, 9 Jun 2023 at 17:03, Richard Henderson
<richard.henderson@linaro.org> wrote:
>
> On 6/9/23 03:47, Peter Maydell wrote:
> > I noticed a couple of finalize_memop related oddities while I
> > was rebasing my decodetree series:
> >
> > (1) in disas_ldst_reg_imm9(), we calculate a memop, but then
> > when we call gen_mte_check1_mmuidx() we don't pass the memop
> > as that function's memop argument, we just pass size. Everywhere
> > else that calls gen_mte_check* functions passes memop. Intentional?
>
> No, looks like a bug.
>
> > (2) disas_ldst_reg_roffset() and disas_ldst_reg_unsigned_imm()
> > use finalize_memop() for both vector and normal register
> > loads/stores. Should they be using finalize_memop_asimd()
> > for the vector versions?
> >
> > (3) disas_ldst_multiple_struct() and disas_ldst_single_struct()
> > use finalize_memop() even though they always load/store
> > vector registers. Should they be using finalize_memop_asimd() ?
>
> Yes.
>
> Everywhere that uses CreateAccDescASIMD in the pseudocode should use finalize_memop_asimd
> for clarity.  (If size is never 128-bit then it will be same as just finalize_memop, so
> some of those places were not actual bugs.  But definitely unclear.)

OK, thanks. The rebase of my decodetree patches for ld/st
turned out hairy enough that I think I'll resend a v2 of
that, so I'll add in some patches at the start that fix
these finalize_memop bits before the conversion.

-- PMM