[v7,21/52] tcg: Use offsets not indices for TCGv_*

Message ID 20171020232023.15010-22-richard.henderson@linaro.org
State New
Headers show
Series
  • tcg queued patches
Related show

Commit Message

Richard Henderson Oct. 20, 2017, 11:19 p.m.
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

Comments

Emilio G. Cota Oct. 23, 2017, 5:37 p.m. | #1
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.
Philippe Mathieu-Daudé Oct. 24, 2017, 3:22 a.m. | #2
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

>
Philippe Mathieu-Daudé Oct. 24, 2017, 3:23 a.m. | #3
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.
Philippe Mathieu-Daudé Oct. 24, 2017, 3:30 a.m. | #4
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>

Patch

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