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~

Reply via email to