Re: [Qemu-devel] [PATCH RFC v1 06/29] target-arc: EX, LD, ST, SYNC, PREFETCH

[Richard Henderson]

On 09/08/2016 03:31 PM, Michael Rolnik wrote:
> Signed-off-by: Michael Rolnik <address@hidden>
> ---
>  target-arc/translate-inst.c | 230 ++++++++++++++++++++++++++++++++++++++++++++
>  target-arc/translate-inst.h |  10 ++
>  2 files changed, 240 insertions(+)
>
> diff --git a/target-arc/translate-inst.c b/target-arc/translate-inst.c
> index 2032823..ac13c86 100644
> --- a/target-arc/translate-inst.c
> +++ b/target-arc/translate-inst.c
> @@ -664,3 +664,233 @@ int arc_gen_RORm(DisasCtxt *ctx, TCGv dest, TCGv src1, 
> TCGv src2)
>      return  BS_NONE;
>  }
>
> +/*
> +    EX
> +*/
> +int arc_gen_EX(DisasCtxt *ctx, TCGv dest, TCGv src1)
> +{
> +    TCGv temp = tcg_temp_new_i32();
> +
> +    tcg_gen_mov_tl(temp, dest);
> +
> +    tcg_gen_qemu_ld_tl(dest, src1, ctx->memidx, MO_UL);
> +    tcg_gen_qemu_st_tl(temp, src1, ctx->memidx, MO_UL);
> +
> +    tcg_temp_free_i32(temp);

You need to load into temp and store directly from dest.  Otherwise you 
incorrectly clobber dest when the page is read-only.


> +int arc_gen_LD(DisasCtxt *ctx, TCGv dest, TCGv src1, TCGv src2)
> +{
> +    TCGv addr = tcg_temp_new_i32();
> +
> +    /*  address             */
> +    switch (ctx->opt.aa) {

You should have a common subroutine for computing addresses.  And probably for 
implementing all loads and all stores.  Use TCGMemOp to distinguish the cases.

> +int arc_gen_PREFETCH(DisasCtxt *ctx, TCGv src1, TCGv src2)
> +{
> +    TCGv temp = tcg_temp_new_i32();
> +
> +    arc_gen_LD(ctx, temp, src1, src2);

A prefetch should be implemented as a NOP.  Otherwise you'll generate 
exceptions when you shouldn't.


r~