Re: [PATCH 1/5] target/mips: Replace GET_OFFSET() macro by get_offset() function

[Richard Henderson]

On 8/18/21 11:31 AM, Philippe Mathieu-Daudé wrote:
> >    I think you should drop
> > get_offset() entirely and replace it with
> >
> >      int dir = cpu_is_bigendian(env) ? 1 : -1;
> >
> >      stb(env, arg2 + 1 * dir, data);
> >
> >      stb(env, arg2 + 2 * dir, data);
> >
> > Alternately, bite the bullet and split the function(s) into two,
> > explicitly endian versions: helper_swl_be, helper_swl_le, etc.
>
> I'll go for the easier path ;)

It's not really more difficult.

static inline void do_swl(env, uint32_t val, target_ulong addr, int midx,
                          int dir, unsigned lmask, uintptr_t ra)
{
    cpu_stb_mmuidx_ra(env, addr, val >> 24, midx, ra);

    if (lmask <= 2) {
        cpu_stb_mmuidx_ra(env, addr + 1 * dir, val >> 16, midx, ra);
    }
    if (lmask <= 1) {
        cpu_stb_mmuidx_ra(env, addr + 1 * dir, val >> 8, midx, ra);
    }
    if (lmask == 0) {
        cpu_stb_mmuidx_ra(env, addr + 1 * dir, val, midx, ra);
    }
}

void helper_swl_be(env, val, addr, midx)
{
    do_swl(env, val, addr, midx, 1, addr & 3, GETPC());
}

void helper_swl_le(env, val, addr, midx)
{
    do_swl(env, val, addr, midx, -1, ~addr & 3, GETPC());
}

Although I do wonder if this is strictly correct with respect to atomicity.  In my 
tcg/mips unaligned patch set, I assumed that lwl+lwr of an aligned address produces two 
atomic 32-bit loads, which result in a complete atomic load at the end.

Should we be doing something like

void helper_swl_be(env, val, addr, midx)
{
    uintptr_t ra = GETPC();

    switch (addr & 3) {
    case 0:
        cpu_stl_be_mmuidx_ra(env, val, addr, midx, ra);
        break;
    case 1:
        cpu_stb_mmuidx_ra(env, val >> 24, addr, midx, ra);
        cpu_stw_be_mmuidx_ra(env, val >> 16, addr + 1, midx, ra);
        break;
    case 2:
        cpu_stw_be_mmuidx_ra(env, val >> 16, addr, midx, ra);
        break;
    case 3:
        cpu_stb_mmuidx_ra(env, val >> 24, addr, midx, ra);
        break;
    }
}

void helper_swl_le(env, val, addr, midx)
{
    uintptr_t ra = GETPC();

    /*
     * We want to use stw and stl for atomicity, but want any
     * fault to report ADDR, not the aligned address.
     */
    probe_write(env, addr, 0, midx, ra);

    switch (addr & 3) {
    case 3:
        cpu_stl_le_mmuidx_ra(env, val, addr - 3, midx, ra);
        break;
    case 1:
        cpu_stw_le_mmuidx_ra(env, val >> 16, addr - 1, midx, ra);
        break;
    case 2:
        cpu_stw_le_mmuidx_ra(env, val >> 8, addr - 2, midx, ra);
        /* fall through */
    case 0:
        cpu_stb_mmuidx_ra(env, val >> 24, addr, midx, ra);
        break;
    }
}

etc.


r~