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

[Joelle van Dyne]

Much of the code that uses the macro is like the following (from
aarch64/tcg-include.inc.c)

        *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.

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.

-j

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)
> {
> #ifdef CONFIG_IOS_JIT
>     return rx + s->code_rw_mirror_diff;
> #else
>     return rx;
> #endif
> }
>
>
> r~