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

[BALATON Zoltan]

On Wed, 14 Oct 2020, Richard Henderson 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;

This looks better but can you add to void *? I think some compilers may 
complain about that so may need to cast here to uint8_t * then back to 
void * but that's at least within this func or maybe declare rx as uint_t 
* and return void *? Or is rx promoted to the type of 
s->code_rw_mirror_diff and that avoids the warning? If the gcc and clang 
versions we care about don't mind then it's simpler without a cast as you 
suggest.

Regards,
BALATON Zoltan

> #else
>    return rx;
> #endif
> }
>
>
> r~