[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH 07/10] tcg: implement bulletproof JIT

From: BALATON Zoltan
Subject: Re: [PATCH 07/10] tcg: implement bulletproof JIT
Date: Wed, 14 Oct 2020 23:08:59 +0200 (CEST)

On Wed, 14 Oct 2020, Joelle van Dyne wrote:
Much of the code that uses the macro is like the following (from

       *TCG_CODE_PTR_RW(s, code_ptr) =
           deposit32(*TCG_CODE_PTR_RW(s, code_ptr), 0, 26, offset);

Before the change, it was just *code_ptr. I'm saying the alternative
was to have to write "tcg_insn_unit *rw_code_ptr = (tcg_insn_unit
*)TCG_CODE_PTR_RW(s, code_ptr)" everywhere or else inline cast it.
Whereas making it return tcg_insn_unit * means only three instances of
casting to uint8_t *. Using void * means casting at every instance.

It's not C++ so void * does not need to be cast when assigned to some other pointer.

Not opposed to using an inline function over a macro but "inline" is
not ANSI C. Not sure what this project thinks about that considering
the style checker prohibits C99 style comments. Personally I don't
care much.

QEMU has some compiler dependencies, I think only some recent versions of gcc and clang are supported and static inline is used elsewhere in the code. Richard is an expert on TCG so you can take his advice.



On Wed, Oct 14, 2020 at 1:35 PM Richard Henderson
<richard.henderson@linaro.org> wrote:

On 10/14/20 9:03 AM, Joelle van Dyne wrote:
static int encode_search(TranslationBlock *tb, uint8_t *block)
-    uint8_t *highwater = tcg_ctx->code_gen_highwater;
-    uint8_t *p = block;
+    uint8_t *highwater;
+    uint8_t *p;
    int i, j, n;

+    highwater = (uint8_t *)TCG_CODE_PTR_RW(tcg_ctx,
+                                           tcg_ctx->code_gen_highwater);
+    p = (uint8_t *)TCG_CODE_PTR_RW(tcg_ctx, block);

Why do you need explicit casts here? Can this be avoided by using
appropriate type or within the macro (I haven't checked this at all just
dislike casts as they can hide problems otherwise caught by the compiler).
There's the choice between tcg_insn_unit * and uint8_t *. Since it's
used much more widely in tcg-target.inc.c, it seemed like
tcg_insn_unit * was a better choice.

False choice.  This is why tcg_ctx->code_gen_highwater is void*.

+#if defined(CONFIG_IOS_JIT)
+# define TCG_CODE_PTR_RW(s, code_ptr) \
+    (tcg_insn_unit *)((uintptr_t)(code_ptr) + (s)->code_rw_mirror_diff)

Better as

static inline void *tcg_code_ptr_rw(TCGContext *s, void *rx)
    return rx + s->code_rw_mirror_diff;
    return rx;


reply via email to

[Prev in Thread] Current Thread [Next in Thread]