| Message ID | 20171020232023.15010-22-richard.henderson@linaro.org |
|---|---|
| State | New |
| Headers | show |
| Series | tcg queued patches | expand |
On Fri, Oct 20, 2017 at 16:19:52 -0700, Richard Henderson wrote: > Using the offset of a temporary, relative to TCGContext, rather than > its index means that we don't use 0. That leaves offset 0 free for > a NULL representation without having to leave index 0 unused. > > Signed-off-by: Richard Henderson <richard.henderson@linaro.org> > --- > tcg/tcg.h | 37 ++++++++++++++++++++----------------- > 1 file changed, 20 insertions(+), 17 deletions(-) > > diff --git a/tcg/tcg.h b/tcg/tcg.h > index 8f692bc6cf..7fe0fb9e07 100644 > --- a/tcg/tcg.h > +++ b/tcg/tcg.h > @@ -429,13 +429,13 @@ typedef TCGv_ptr TCGv_env; > #endif (snip) > /* used to align parameters */ > -#define TCG_CALL_DUMMY_ARG ((TCGArg)(-1)) > +#define TCG_CALL_DUMMY_ARG ((TCGArg)0) We're doing something clever here (on a first read I thought TCGContext was a typo), so I'd leave a comment somewhere. TCG_CALL_DUMMY_ARG might be a good place to do so; a copy of the commit's message should suffice. Reviewed-by: Emilio G. Cota <cota@braap.org> E.
On 10/20/2017 08:19 PM, Richard Henderson wrote: > Using the offset of a temporary, relative to TCGContext, rather than > its index means that we don't use 0. That leaves offset 0 free for > a NULL representation without having to leave index 0 unused. > > Signed-off-by: Richard Henderson <richard.henderson@linaro.org> > --- > tcg/tcg.h | 37 ++++++++++++++++++++----------------- > 1 file changed, 20 insertions(+), 17 deletions(-) > > diff --git a/tcg/tcg.h b/tcg/tcg.h > index 8f692bc6cf..7fe0fb9e07 100644 > --- a/tcg/tcg.h > +++ b/tcg/tcg.h > @@ -429,13 +429,13 @@ typedef TCGv_ptr TCGv_env; > #endif > > /* Dummy definition to avoid compiler warnings. */ > -#define TCGV_UNUSED_I32(x) (x = (TCGv_i32)-1) > -#define TCGV_UNUSED_I64(x) (x = (TCGv_i64)-1) > -#define TCGV_UNUSED_PTR(x) (x = (TCGv_ptr)-1) > +#define TCGV_UNUSED_I32(x) (x = (TCGv_i32)NULL) > +#define TCGV_UNUSED_I64(x) (x = (TCGv_i64)NULL) > +#define TCGV_UNUSED_PTR(x) (x = (TCGv_ptr)NULL) > > -#define TCGV_IS_UNUSED_I32(x) ((x) == (TCGv_i32)-1) > -#define TCGV_IS_UNUSED_I64(x) ((x) == (TCGv_i64)-1) > -#define TCGV_IS_UNUSED_PTR(x) ((x) == (TCGv_ptr)-1) > +#define TCGV_IS_UNUSED_I32(x) ((x) == (TCGv_i32)NULL) > +#define TCGV_IS_UNUSED_I64(x) ((x) == (TCGv_i64)NULL) > +#define TCGV_IS_UNUSED_PTR(x) ((x) == (TCGv_ptr)NULL) > > /* call flags */ > /* Helper does not read globals (either directly or through an exception). It > @@ -454,7 +454,7 @@ typedef TCGv_ptr TCGv_env; > #define TCG_CALL_NO_WG_SE (TCG_CALL_NO_WG | TCG_CALL_NO_SE) > > /* used to align parameters */ > -#define TCG_CALL_DUMMY_ARG ((TCGArg)(-1)) > +#define TCG_CALL_DUMMY_ARG ((TCGArg)0) > > /* Conditions. Note that these are laid out for easy manipulation by > the functions below: > @@ -701,17 +701,20 @@ static inline size_t temp_idx(TCGTemp *ts) > > static inline TCGArg temp_arg(TCGTemp *ts) > { > - return temp_idx(ts); > + ptrdiff_t a = (void *)ts - (void *)&tcg_ctx; > + tcg_debug_assert(a >= offsetof(TCGContext, temps) > + && a < offsetof(TCGContext, temps[tcg_ctx.nb_temps])); > + return a; > } > > static inline TCGTemp *arg_temp(TCGArg a) > { > - return a == TCG_CALL_DUMMY_ARG ? NULL : &tcg_ctx.temps[a]; > -} > - > -static inline size_t arg_index(TCGArg a) > -{ > - return a; > + if (a == TCG_CALL_DUMMY_ARG) { > + return NULL; > + } > + tcg_debug_assert(a >= offsetof(TCGContext, temps) > + && a < offsetof(TCGContext, temps[tcg_ctx.nb_temps])); > + return (void *)&tcg_ctx + a; Hmmm why not cast it as TCGTemp*? > } > > static inline TCGArg tcgv_i32_arg(TCGv_i32 t) > @@ -746,17 +749,17 @@ static inline TCGTemp *tcgv_ptr_temp(TCGv_ptr t) > > static inline TCGv_i32 temp_tcgv_i32(TCGTemp *t) > { > - return (TCGv_i32)temp_idx(t); > + return (TCGv_i32)temp_arg(t); > } > > static inline TCGv_i64 temp_tcgv_i64(TCGTemp *t) > { > - return (TCGv_i64)temp_idx(t); > + return (TCGv_i64)temp_arg(t); > } > > static inline TCGv_ptr temp_tcgv_ptr(TCGTemp *t) > { > - return (TCGv_ptr)temp_idx(t); > + return (TCGv_ptr)temp_arg(t); > } > > #if TCG_TARGET_REG_BITS == 32 >
On 10/23/2017 02:37 PM, Emilio G. Cota wrote: > On Fri, Oct 20, 2017 at 16:19:52 -0700, Richard Henderson wrote: >> Using the offset of a temporary, relative to TCGContext, rather than >> its index means that we don't use 0. That leaves offset 0 free for >> a NULL representation without having to leave index 0 unused. [...] >> /* used to align parameters */ >> -#define TCG_CALL_DUMMY_ARG ((TCGArg)(-1)) >> +#define TCG_CALL_DUMMY_ARG ((TCGArg)0) > > We're doing something clever here (on a first read I thought TCGContext > was a typo), so I'd leave a comment somewhere. TCG_CALL_DUMMY_ARG might > be a good place to do so; a copy of the commit's message should suffice. agreed.
On 10/24/2017 12:22 AM, Philippe Mathieu-Daudé wrote: > On 10/20/2017 08:19 PM, Richard Henderson wrote: >> Using the offset of a temporary, relative to TCGContext, rather than >> its index means that we don't use 0. That leaves offset 0 free for >> a NULL representation without having to leave index 0 unused. >> >> Signed-off-by: Richard Henderson <richard.henderson@linaro.org> >> --- >> tcg/tcg.h | 37 ++++++++++++++++++++----------------- >> 1 file changed, 20 insertions(+), 17 deletions(-) >> [...] >> + return (void *)&tcg_ctx + a; > > Hmmm why not cast it as TCGTemp*? just read next patch, so: Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
diff --git a/tcg/tcg.h b/tcg/tcg.h index 8f692bc6cf..7fe0fb9e07 100644 --- a/tcg/tcg.h +++ b/tcg/tcg.h @@ -429,13 +429,13 @@ typedef TCGv_ptr TCGv_env; #endif /* Dummy definition to avoid compiler warnings. */ -#define TCGV_UNUSED_I32(x) (x = (TCGv_i32)-1) -#define TCGV_UNUSED_I64(x) (x = (TCGv_i64)-1) -#define TCGV_UNUSED_PTR(x) (x = (TCGv_ptr)-1) +#define TCGV_UNUSED_I32(x) (x = (TCGv_i32)NULL) +#define TCGV_UNUSED_I64(x) (x = (TCGv_i64)NULL) +#define TCGV_UNUSED_PTR(x) (x = (TCGv_ptr)NULL) -#define TCGV_IS_UNUSED_I32(x) ((x) == (TCGv_i32)-1) -#define TCGV_IS_UNUSED_I64(x) ((x) == (TCGv_i64)-1) -#define TCGV_IS_UNUSED_PTR(x) ((x) == (TCGv_ptr)-1) +#define TCGV_IS_UNUSED_I32(x) ((x) == (TCGv_i32)NULL) +#define TCGV_IS_UNUSED_I64(x) ((x) == (TCGv_i64)NULL) +#define TCGV_IS_UNUSED_PTR(x) ((x) == (TCGv_ptr)NULL) /* call flags */ /* Helper does not read globals (either directly or through an exception). It @@ -454,7 +454,7 @@ typedef TCGv_ptr TCGv_env; #define TCG_CALL_NO_WG_SE (TCG_CALL_NO_WG | TCG_CALL_NO_SE) /* used to align parameters */ -#define TCG_CALL_DUMMY_ARG ((TCGArg)(-1)) +#define TCG_CALL_DUMMY_ARG ((TCGArg)0) /* Conditions. Note that these are laid out for easy manipulation by the functions below: @@ -701,17 +701,20 @@ static inline size_t temp_idx(TCGTemp *ts) static inline TCGArg temp_arg(TCGTemp *ts) { - return temp_idx(ts); + ptrdiff_t a = (void *)ts - (void *)&tcg_ctx; + tcg_debug_assert(a >= offsetof(TCGContext, temps) + && a < offsetof(TCGContext, temps[tcg_ctx.nb_temps])); + return a; } static inline TCGTemp *arg_temp(TCGArg a) { - return a == TCG_CALL_DUMMY_ARG ? NULL : &tcg_ctx.temps[a]; -} - -static inline size_t arg_index(TCGArg a) -{ - return a; + if (a == TCG_CALL_DUMMY_ARG) { + return NULL; + } + tcg_debug_assert(a >= offsetof(TCGContext, temps) + && a < offsetof(TCGContext, temps[tcg_ctx.nb_temps])); + return (void *)&tcg_ctx + a; } static inline TCGArg tcgv_i32_arg(TCGv_i32 t) @@ -746,17 +749,17 @@ static inline TCGTemp *tcgv_ptr_temp(TCGv_ptr t) static inline TCGv_i32 temp_tcgv_i32(TCGTemp *t) { - return (TCGv_i32)temp_idx(t); + return (TCGv_i32)temp_arg(t); } static inline TCGv_i64 temp_tcgv_i64(TCGTemp *t) { - return (TCGv_i64)temp_idx(t); + return (TCGv_i64)temp_arg(t); } static inline TCGv_ptr temp_tcgv_ptr(TCGTemp *t) { - return (TCGv_ptr)temp_idx(t); + return (TCGv_ptr)temp_arg(t); } #if TCG_TARGET_REG_BITS == 32
Using the offset of a temporary, relative to TCGContext, rather than its index means that we don't use 0. That leaves offset 0 free for a NULL representation without having to leave index 0 unused. Signed-off-by: Richard Henderson <richard.henderson@linaro.org> --- tcg/tcg.h | 37 ++++++++++++++++++++----------------- 1 file changed, 20 insertions(+), 17 deletions(-) -- 2.13.6