Re: [PATCH] target/hppa: Allow, but diagnose, LDCW aligned only mod 4

[Philippe Mathieu-Daudé]

On 1/17/20 6:30 PM, Richard Henderson wrote:
> On 1/17/20 6:13 AM, Philippe Mathieu-Daudé wrote:
> > > > -    MemOp mop = MO_TEUL | MO_ALIGN_16 | a->size;
> > > > +    MemOp mop = MO_TE | MO_ALIGN | a->size;
> >
> >
> > Hmmm you changed MO_TEUL -> MO_TE, so from MO_32 to MO_8.
> >
> > Per your description, shouldn't this be MO_TEUL | MO_ALIGN_4?
>
> The "UL" part is also being added by a->size.  This code was written this way
> in preparation for the 64-bit ldc, and the bug was not noticable because we
> don't have that yet.

Ah I missed the a->size.

So on 32-bit the hw trap doesn't trap on unaligned 16, but traps on 
unaligned 4.

On 64-bit we don't know yet, but IIUC we expect to not trap on unaligned 8.

Looking at target/hppa/insns.decode:

&ldst           t b x disp sp m scale size

@ldstx          ...... b:5 x:5 sp:2 scale:1 ....... m:1 t:5     &ldst disp=0
@ldim5          ...... b:5 ..... sp:2 ......... t:5     \
                &ldst disp=%im5_16 x=0 scale=0 m=%ma_to_m

ldc             000011 ..... ..... .. . 1 -- 0111      ......   @ldim5 
size=2
ldc             000011 ..... ..... .. . 0 -- 0111      ......   @ldstx 
size=2

We have a->size = 2 = MO_32 = MO_UL.

So do you plan to add LDCD from PA2.0 using size=3, OK.

From "exec/memop.h":

     * MO_ALIGN supposes the alignment size is the size of a memory access.
     *
     * There are three options:
     * - unaligned access permitted (MO_UNALN).
     * - an alignment to the size of an access (MO_ALIGN);

Ah so with LDCW we already access a word, so MO_ALIGN -> MO_ALIGN_4.

With LDLD we'll have MO_ALIGN -> MO_ALIGN_8.

Now "MemOp mop = MO_TE | MO_ALIGN | a->size;" makes sense!

The power of your decodetree script amazed me again :)

Reviewed-by: Philippe Mathieu-Daudé <address@hidden>