Re: [Qemu-devel] [PATCH v4 2/5] target/mips: Fix MSA instructions ST.<B|

qemu-devel
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v4 2/5] target/mips: Fix MSA instructions ST.<B|

From:	Aleksandar Markovic
Subject:	Re: [Qemu-devel] [PATCH v4 2/5] target/mips: Fix MSA instructions ST.<B\|H\|W\|D> on big endian host
Date:	Mon, 20 May 2019 14:59:45 +0200
On Apr 2, 2019 3:47 PM, "Mateja Marjanovic" <address@hidden>
wrote:
>
> From: Mateja Marjanovic <address@hidden>
>
> Fix the case when the host is a big endian machine, and change
> the approach toward ST.<B|H|W|D> instruction helpers.
>
> Signed-off-by: Mateja Marjanovic <address@hidden>
> ---

Hello, Mateja.

There is unfortunatelly still a slight problem with the new implementation:
it looks like the invocations to ensure_writable_pages() in new helpers are
missing. Or, perhaps, there is a reason you removed them. Please reanalyse
and reexplain. But thanks for previous efforts.

Yours,
Aleksandar

>  target/mips/op_helper.c | 188
++++++++++++++++++++++++++++++++++++++++++------
>  1 file changed, 168 insertions(+), 20 deletions(-)
>
> diff --git a/target/mips/op_helper.c b/target/mips/op_helper.c
> index 45be406..d94909a 100644
> --- a/target/mips/op_helper.c
> +++ b/target/mips/op_helper.c
> @@ -4565,31 +4565,179 @@ static inline void
ensure_writable_pages(CPUMIPSState *env,
>  #endif
>  }
>
> -#define MSA_ST_DF(DF, TYPE, ST_INSN, ...)                               \
> -void helper_msa_st_ ## TYPE(CPUMIPSState *env, uint32_t wd,             \
> -                            target_ulong addr)                          \
> -{                                                                       \
> -    wr_t *pwd = &(env->active_fpu.fpr[wd].wr);                          \
> -    int mmu_idx = cpu_mmu_index(env, false);                           \
> -    int i;                                                              \
> -    MEMOP_IDX(DF)                                                       \
> -    ensure_writable_pages(env, addr, mmu_idx, GETPC());                 \
> -    for (i = 0; i < DF_ELEMENTS(DF); i++) {                             \
> -        ST_INSN(env, addr + (i << DF), pwd->TYPE[i], ##__VA_ARGS__);    \
> -    }                                                                   \
> +void helper_msa_st_b(CPUMIPSState *env, uint32_t wd,
> +                     target_ulong addr)
> +{
> +    wr_t *pwd = &(env->active_fpu.fpr[wd].wr);
> +    MEMOP_IDX(DF_BYTE)
> +#if !defined(CONFIG_USER_ONLY)
> +#if !defined(HOST_WORDS_BIGENDIAN)
> +    helper_ret_stb_mmu(env, addr + (0  << DF_BYTE), pwd->b[0],  oi,
GETPC());
> +    helper_ret_stb_mmu(env, addr + (1  << DF_BYTE), pwd->b[1],  oi,
GETPC());
> +    helper_ret_stb_mmu(env, addr + (2  << DF_BYTE), pwd->b[2],  oi,
GETPC());
> +    helper_ret_stb_mmu(env, addr + (3  << DF_BYTE), pwd->b[3],  oi,
GETPC());
> +    helper_ret_stb_mmu(env, addr + (4  << DF_BYTE), pwd->b[4],  oi,
GETPC());
> +    helper_ret_stb_mmu(env, addr + (5  << DF_BYTE), pwd->b[5],  oi,
GETPC());
> +    helper_ret_stb_mmu(env, addr + (6  << DF_BYTE), pwd->b[6],  oi,
GETPC());
> +    helper_ret_stb_mmu(env, addr + (7  << DF_BYTE), pwd->b[7],  oi,
GETPC());
> +    helper_ret_stb_mmu(env, addr + (8  << DF_BYTE), pwd->b[8],  oi,
GETPC());
> +    helper_ret_stb_mmu(env, addr + (9  << DF_BYTE), pwd->b[9],  oi,
GETPC());
> +    helper_ret_stb_mmu(env, addr + (10 << DF_BYTE), pwd->b[10], oi,
GETPC());
> +    helper_ret_stb_mmu(env, addr + (11 << DF_BYTE), pwd->b[11], oi,
GETPC());
> +    helper_ret_stb_mmu(env, addr + (12 << DF_BYTE), pwd->b[12], oi,
GETPC());
> +    helper_ret_stb_mmu(env, addr + (13 << DF_BYTE), pwd->b[13], oi,
GETPC());
> +    helper_ret_stb_mmu(env, addr + (14 << DF_BYTE), pwd->b[14], oi,
GETPC());
> +    helper_ret_stb_mmu(env, addr + (15 << DF_BYTE), pwd->b[15], oi,
GETPC());
> +#else
> +    helper_ret_stb_mmu(env, addr + (7  << DF_BYTE), pwd->b[0],  oi,
GETPC());
> +    helper_ret_stb_mmu(env, addr + (6  << DF_BYTE), pwd->b[1],  oi,
GETPC());
> +    helper_ret_stb_mmu(env, addr + (5  << DF_BYTE), pwd->b[2],  oi,
GETPC());
> +    helper_ret_stb_mmu(env, addr + (4  << DF_BYTE), pwd->b[3],  oi,
GETPC());
> +    helper_ret_stb_mmu(env, addr + (3  << DF_BYTE), pwd->b[4],  oi,
GETPC());
> +    helper_ret_stb_mmu(env, addr + (2  << DF_BYTE), pwd->b[5],  oi,
GETPC());
> +    helper_ret_stb_mmu(env, addr + (1  << DF_BYTE), pwd->b[6],  oi,
GETPC());
> +    helper_ret_stb_mmu(env, addr + (0  << DF_BYTE), pwd->b[7],  oi,
GETPC());
> +    helper_ret_stb_mmu(env, addr + (15 << DF_BYTE), pwd->b[8],  oi,
GETPC());
> +    helper_ret_stb_mmu(env, addr + (14 << DF_BYTE), pwd->b[9],  oi,
GETPC());
> +    helper_ret_stb_mmu(env, addr + (13 << DF_BYTE), pwd->b[10], oi,
GETPC());
> +    helper_ret_stb_mmu(env, addr + (12 << DF_BYTE), pwd->b[11], oi,
GETPC());
> +    helper_ret_stb_mmu(env, addr + (11 << DF_BYTE), pwd->b[12], oi,
GETPC());
> +    helper_ret_stb_mmu(env, addr + (10 << DF_BYTE), pwd->b[13], oi,
GETPC());
> +    helper_ret_stb_mmu(env, addr + (9  << DF_BYTE), pwd->b[14], oi,
GETPC());
> +    helper_ret_stb_mmu(env, addr + (8  << DF_BYTE), pwd->b[15], oi,
GETPC());
> +#endif
> +#else
> +#if !defined(HOST_WORDS_BIGENDIAN)
> +    cpu_stb_data(env, addr + (0  << DF_BYTE), pwd->b[0]);
> +    cpu_stb_data(env, addr + (1  << DF_BYTE), pwd->b[1]);
> +    cpu_stb_data(env, addr + (2  << DF_BYTE), pwd->b[2]);
> +    cpu_stb_data(env, addr + (3  << DF_BYTE), pwd->b[3]);
> +    cpu_stb_data(env, addr + (4  << DF_BYTE), pwd->b[4]);
> +    cpu_stb_data(env, addr + (5  << DF_BYTE), pwd->b[5]);
> +    cpu_stb_data(env, addr + (6  << DF_BYTE), pwd->b[6]);
> +    cpu_stb_data(env, addr + (7  << DF_BYTE), pwd->b[7]);
> +    cpu_stb_data(env, addr + (8  << DF_BYTE), pwd->b[8]);
> +    cpu_stb_data(env, addr + (9  << DF_BYTE), pwd->b[9]);
> +    cpu_stb_data(env, addr + (10 << DF_BYTE), pwd->b[10]);
> +    cpu_stb_data(env, addr + (11 << DF_BYTE), pwd->b[11]);
> +    cpu_stb_data(env, addr + (12 << DF_BYTE), pwd->b[12]);
> +    cpu_stb_data(env, addr + (13 << DF_BYTE), pwd->b[13]);
> +    cpu_stb_data(env, addr + (14 << DF_BYTE), pwd->b[14]);
> +    cpu_stb_data(env, addr + (15 << DF_BYTE), pwd->b[15]);
> +#else
> +    cpu_stb_data(env, addr + (7  << DF_BYTE), pwd->b[0]);
> +    cpu_stb_data(env, addr + (6  << DF_BYTE), pwd->b[1]);
> +    cpu_stb_data(env, addr + (5  << DF_BYTE), pwd->b[2]);
> +    cpu_stb_data(env, addr + (4  << DF_BYTE), pwd->b[3]);
> +    cpu_stb_data(env, addr + (3  << DF_BYTE), pwd->b[4]);
> +    cpu_stb_data(env, addr + (2  << DF_BYTE), pwd->b[5]);
> +    cpu_stb_data(env, addr + (1  << DF_BYTE), pwd->b[6]);
> +    cpu_stb_data(env, addr + (0  << DF_BYTE), pwd->b[7]);
> +    cpu_stb_data(env, addr + (15 << DF_BYTE), pwd->b[8]);
> +    cpu_stb_data(env, addr + (14 << DF_BYTE), pwd->b[9]);
> +    cpu_stb_data(env, addr + (13 << DF_BYTE), pwd->b[10]);
> +    cpu_stb_data(env, addr + (12 << DF_BYTE), pwd->b[11]);
> +    cpu_stb_data(env, addr + (11 << DF_BYTE), pwd->b[12]);
> +    cpu_stb_data(env, addr + (10 << DF_BYTE), pwd->b[13]);
> +    cpu_stb_data(env, addr + (9  << DF_BYTE), pwd->b[14]);
> +    cpu_stb_data(env, addr + (8  << DF_BYTE), pwd->b[15]);
> +#endif
> +#endif
> +}
> +
> +void helper_msa_st_h(CPUMIPSState *env, uint32_t wd,
> +                     target_ulong addr)
> +{
> +    wr_t *pwd = &(env->active_fpu.fpr[wd].wr);
> +    MEMOP_IDX(DF_HALF)
> +#if !defined(CONFIG_USER_ONLY)
> +#if !defined(HOST_WORDS_BIGENDIAN)
> +    helper_ret_stw_mmu(env, addr + (0 << DF_HALF), pwd->h[0], oi,
GETPC());
> +    helper_ret_stw_mmu(env, addr + (1 << DF_HALF), pwd->h[1], oi,
GETPC());
> +    helper_ret_stw_mmu(env, addr + (2 << DF_HALF), pwd->h[2], oi,
GETPC());
> +    helper_ret_stw_mmu(env, addr + (3 << DF_HALF), pwd->h[3], oi,
GETPC());
> +    helper_ret_stw_mmu(env, addr + (4 << DF_HALF), pwd->h[4], oi,
GETPC());
> +    helper_ret_stw_mmu(env, addr + (5 << DF_HALF), pwd->h[5], oi,
GETPC());
> +    helper_ret_stw_mmu(env, addr + (6 << DF_HALF), pwd->h[6], oi,
GETPC());
> +    helper_ret_stw_mmu(env, addr + (7 << DF_HALF), pwd->h[7], oi,
GETPC());
> +#else
> +    helper_ret_stw_mmu(env, addr + (3 << DF_HALF), pwd->h[0], oi,
GETPC());
> +    helper_ret_stw_mmu(env, addr + (2 << DF_HALF), pwd->h[1], oi,
GETPC());
> +    helper_ret_stw_mmu(env, addr + (1 << DF_HALF), pwd->h[2], oi,
GETPC());
> +    helper_ret_stw_mmu(env, addr + (0 << DF_HALF), pwd->h[3], oi,
GETPC());
> +    helper_ret_stw_mmu(env, addr + (7 << DF_HALF), pwd->h[4], oi,
GETPC());
> +    helper_ret_stw_mmu(env, addr + (6 << DF_HALF), pwd->h[5], oi,
GETPC());
> +    helper_ret_stw_mmu(env, addr + (5 << DF_HALF), pwd->h[6], oi,
GETPC());
> +    helper_ret_stw_mmu(env, addr + (4 << DF_HALF), pwd->h[7], oi,
GETPC());
> +#endif
> +#else
> +#if !defined(HOST_WORDS_BIGENDIAN)
> +    cpu_stw_data(env, addr + (0 << DF_HALF), pwd->h[0]);
> +    cpu_stw_data(env, addr + (1 << DF_HALF), pwd->h[1]);
> +    cpu_stw_data(env, addr + (2 << DF_HALF), pwd->h[2]);
> +    cpu_stw_data(env, addr + (3 << DF_HALF), pwd->h[3]);
> +    cpu_stw_data(env, addr + (4 << DF_HALF), pwd->h[4]);
> +    cpu_stw_data(env, addr + (5 << DF_HALF), pwd->h[5]);
> +    cpu_stw_data(env, addr + (6 << DF_HALF), pwd->h[6]);
> +    cpu_stw_data(env, addr + (7 << DF_HALF), pwd->h[7]);
> +#else
> +    cpu_stw_data(env, addr + (3 << DF_HALF), pwd->h[0]);
> +    cpu_stw_data(env, addr + (2 << DF_HALF), pwd->h[1]);
> +    cpu_stw_data(env, addr + (1 << DF_HALF), pwd->h[2]);
> +    cpu_stw_data(env, addr + (0 << DF_HALF), pwd->h[3]);
> +    cpu_stw_data(env, addr + (7 << DF_HALF), pwd->h[4]);
> +    cpu_stw_data(env, addr + (6 << DF_HALF), pwd->h[5]);
> +    cpu_stw_data(env, addr + (5 << DF_HALF), pwd->h[6]);
> +    cpu_stw_data(env, addr + (4 << DF_HALF), pwd->h[7]);
> +#endif
> +#endif
>  }
>
> +void helper_msa_st_w(CPUMIPSState *env, uint32_t wd,
> +                     target_ulong addr)
> +{
> +    wr_t *pwd = &(env->active_fpu.fpr[wd].wr);
> +    MEMOP_IDX(DF_WORD)
>  #if !defined(CONFIG_USER_ONLY)
> -MSA_ST_DF(DF_BYTE,   b, helper_ret_stb_mmu, oi, GETPC())
> -MSA_ST_DF(DF_HALF,   h, helper_ret_stw_mmu, oi, GETPC())
> -MSA_ST_DF(DF_WORD,   w, helper_ret_stl_mmu, oi, GETPC())
> -MSA_ST_DF(DF_DOUBLE, d, helper_ret_stq_mmu, oi, GETPC())
> +#if !defined(HOST_WORDS_BIGENDIAN)
> +    helper_ret_stl_mmu(env, addr + (0 << DF_WORD), oi, GETPC(),
pwd->w[0]);
> +    helper_ret_stl_mmu(env, addr + (1 << DF_WORD), oi, GETPC(),
pwd->w[1]);
> +    helper_ret_stl_mmu(env, addr + (2 << DF_WORD), oi, GETPC(),
pwd->w[2]);
> +    helper_ret_stl_mmu(env, addr + (3 << DF_WORD), oi, GETPC(),
pwd->w[3]);
>  #else
> -MSA_ST_DF(DF_BYTE,   b, cpu_stb_data)
> -MSA_ST_DF(DF_HALF,   h, cpu_stw_data)
> -MSA_ST_DF(DF_WORD,   w, cpu_stl_data)
> -MSA_ST_DF(DF_DOUBLE, d, cpu_stq_data)
> +    helper_ret_stl_mmu(env, addr + (1 << DF_WORD), oi, GETPC(),
pwd->w[0]);
> +    helper_ret_stl_mmu(env, addr + (0 << DF_WORD), oi, GETPC(),
pwd->w[1]);
> +    helper_ret_stl_mmu(env, addr + (3 << DF_WORD), oi, GETPC(),
pwd->w[2]);
> +    helper_ret_stl_mmu(env, addr + (2 << DF_WORD), oi, GETPC(),
pwd->w[3]);
>  #endif
> +#else
> +#if !defined(HOST_WORDS_BIGENDIAN)
> +    cpu_stl_data(env, addr + (0 << DF_WORD), pwd->w[0]);
> +    cpu_stl_data(env, addr + (1 << DF_WORD), pwd->w[1]);
> +    cpu_stl_data(env, addr + (2 << DF_WORD), pwd->w[2]);
> +    cpu_stl_data(env, addr + (3 << DF_WORD), pwd->w[3]);
> +#else
> +    cpu_stl_data(env, addr + (1 << DF_WORD), pwd->w[0]);
> +    cpu_stl_data(env, addr + (0 << DF_WORD), pwd->w[1]);
> +    cpu_stl_data(env, addr + (3 << DF_WORD), pwd->w[2]);
> +    cpu_stl_data(env, addr + (2 << DF_WORD), pwd->w[3]);
> +#endif
> +#endif
> +}
> +
> +void helper_msa_st_d(CPUMIPSState *env, uint32_t wd,
> +                     target_ulong addr)
> +{
> +    wr_t *pwd = &(env->active_fpu.fpr[wd].wr);
> +    MEMOP_IDX(DF_DOUBLE)
> +#if !defined(CONFIG_USER_ONLY)
> +    helper_ret_stq_mmu(env, addr + (0 << DF_DOUBLE), pwd->d[0], oi,
GETPC());
> +    helper_ret_stq_mmu(env, addr + (1 << DF_DOUBLE), pwd->d[1], oi,
GETPC());
> +#else
> +    cpu_stq_data(env, addr + (0 << DF_DOUBLE), pwd->d[0]);
> +    cpu_stq_data(env, addr + (1 << DF_DOUBLE), pwd->d[1]);
> +#endif
> +}
>
>  void helper_cache(CPUMIPSState *env, target_ulong addr, uint32_t op)
>  {
> --
> 2.7.4
>
>
[Prev in Thread]
Current Thread
[Next in Thread]
Re: [Qemu-devel] [PATCH v4 2/5] target/mips: Fix MSA instructions ST.<B|H|W|D> on big endian host, Aleksandar Markovic <=
- Re: [Qemu-devel] [PATCH v4 2/5] target/mips: Fix MSA instructions ST.<B|H|W|D> on big endian host, Mateja Marjanovic, 2019/05/20
Prev by Date: Re: [Qemu-devel] [PATCH v9 20/27] gdbstub: Implement target halted (? pkt) with new infra
Next by Date: Re: [Qemu-devel] [PATCH] i386: Enable IA32_MISC_ENABLE MWAIT bit when exposing mwait/monitor
Previous by thread: [Qemu-devel] [PATCH v3 00/14] tests/vm: serial console autoinstall, misc fixes.
Next by thread: Re: [Qemu-devel] [PATCH v4 2/5] target/mips: Fix MSA instructions ST.<B|H|W|D> on big endian host
Index(es):
- Date
- Thread