RE: [PATCH 3/3] target/riscv: fix the trap generation for conditional store

[Konrad, Frederic]

+CC maintainers

> -----Original Message-----
> From: qemu-devel-bounces+fkonrad=amd.com@nongnu.org 
> <qemu-devel-bounces+fkonrad=amd.com@nongnu.org> On
> Behalf Of Richard Henderson
> Sent: 11 December 2024 22:43
> To: qemu-devel@nongnu.org
> Subject: Re: [PATCH 3/3] target/riscv: fix the trap generation for 
> conditional store
> 
> On 12/11/24 15:19, Frederic Konrad wrote:
> > +    /*
> > +     * A misaligned store trap should be triggered even if the store should
> > +     * fail due to the reservation.
> > +     */
> > +    tcg_gen_andi_tl(tmp, src1, ~((uint64_t)0) << 
> > memop_alignment_bits(mop));
> 
> The constant is incorrect for testing the low bits.

Hmm, I don't get it, basically with that I'm trying to do:
  MO_8: src1 == (src1 & 0xFFFFFFFF)
  MO_16: src1 == (src1 & 0xFFFFFFFE)
  MO_32: src1 == (src1 & 0xFFFFFFFC)
  etc

what am I missing?

> 
> > +    tcg_gen_brcond_tl(TCG_COND_EQ, tmp, src1, l3);
> 
> Best to make the fallthrough path be the common case, as we will optimize 
> across the
> extended basic block.
> 
> Use test-style comparison:
> 
>      tcg_gen_brcondi_tl(TCG_COND_TSTNE, src1, memop_size(mop) - 1, 
> l_misalign);

In any case that one looks better thanks for the tips!

About making the fallthrough path be the common case, If I do it I'll need to 
jump anyway and the
end of this instruction ie:

    if not aligned go to misaligned label:
    ...
    do the normal operation
    ...
    go to done label
    misaligned label
    ...
    trigger the exception
    ...
    done label

Is that what you had in mind?

Thanks,
Fred

> 
> 
> 
> r~