Uses target_supports_mttcg() and target_long_bits() to turn ifdefs into
runtime branches.

Signed-off-by: Anton Johansson <anjo@rev.ng>
---
 accel/tcg/tcg-all.c | 25 +++++++++----------------
 1 file changed, 9 insertions(+), 16 deletions(-)

diff --git a/accel/tcg/tcg-all.c b/accel/tcg/tcg-all.c
index a40e0aee37..b8e920e3a8 100644
--- a/accel/tcg/tcg-all.c
+++ b/accel/tcg/tcg-all.c
@@ -28,7 +28,6 @@
 #include "exec/replay-core.h"
 #include "sysemu/cpu-timers.h"
 #include "tcg/tcg.h"
-#include "tcg/oversized-guest.h"
 #include "qapi/error.h"
 #include "qemu/error-report.h"
 #include "qemu/accel.h"
@@ -67,20 +66,13 @@ DECLARE_INSTANCE_CHECKER(TCGState, TCG_STATE,
  * there is one remaining limitation to check:
  *   - The guest can't be oversized (e.g. 64 bit guest on 32 bit host)
  */
-
 static bool default_mttcg_enabled(void)
 {
-    if (icount_enabled() || TCG_OVERSIZED_GUEST) {
+    const bool oversized_guest = target_long_bits() > TCG_TARGET_REG_BITS;
+    if (icount_enabled() || oversized_guest) {
         return false;
     }
-#ifdef TARGET_SUPPORTS_MTTCG
-# ifndef TCG_GUEST_DEFAULT_MO
-#  error "TARGET_SUPPORTS_MTTCG without TCG_GUEST_DEFAULT_MO"
-# endif
-    return true;
-#else
-    return false;
-#endif
+    return target_supports_mttcg();
 }
 
 static void tcg_accel_instance_init(Object *obj)
@@ -137,17 +129,18 @@ static char *tcg_get_thread(Object *obj, Error **errp)
 static void tcg_set_thread(Object *obj, const char *value, Error **errp)
 {
     TCGState *s = TCG_STATE(obj);
+    const bool oversized_guest = target_long_bits() > TCG_TARGET_REG_BITS;
 
     if (strcmp(value, "multi") == 0) {
-        if (TCG_OVERSIZED_GUEST) {
+        if (oversized_guest) {
             error_setg(errp, "No MTTCG when guest word size > hosts");
         } else if (icount_enabled()) {
             error_setg(errp, "No MTTCG when icount is enabled");
         } else {
-#ifndef TARGET_SUPPORTS_MTTCG
-            warn_report("Guest not yet converted to MTTCG - "
-                        "you may get unexpected results");
-#endif
+            if (target_supports_mttcg()) {
+                warn_report("Guest not yet converted to MTTCG - "
+                            "you may get unexpected results");
+            }
             s->mttcg_enabled = true;
         }
     } else if (strcmp(value, "single") == 0) {
-- 
2.43.0

On 1/20/24 00:40, Anton Johansson wrote:
> Uses target_supports_mttcg() and target_long_bits() to turn ifdefs into
> runtime branches.
> 
> Signed-off-by: Anton Johansson <anjo@rev.ng>
> ---
>   accel/tcg/tcg-all.c | 25 +++++++++----------------
>   1 file changed, 9 insertions(+), 16 deletions(-)
> 
> diff --git a/accel/tcg/tcg-all.c b/accel/tcg/tcg-all.c
> index a40e0aee37..b8e920e3a8 100644
> --- a/accel/tcg/tcg-all.c
> +++ b/accel/tcg/tcg-all.c
> @@ -28,7 +28,6 @@
>   #include "exec/replay-core.h"
>   #include "sysemu/cpu-timers.h"
>   #include "tcg/tcg.h"
> -#include "tcg/oversized-guest.h"
>   #include "qapi/error.h"
>   #include "qemu/error-report.h"
>   #include "qemu/accel.h"
> @@ -67,20 +66,13 @@ DECLARE_INSTANCE_CHECKER(TCGState, TCG_STATE,
>    * there is one remaining limitation to check:
>    *   - The guest can't be oversized (e.g. 64 bit guest on 32 bit host)
>    */
> -
>   static bool default_mttcg_enabled(void)
>   {
> -    if (icount_enabled() || TCG_OVERSIZED_GUEST) {
> +    const bool oversized_guest = target_long_bits() > TCG_TARGET_REG_BITS;
> +    if (icount_enabled() || oversized_guest) {
>           return false;
>       }
> -#ifdef TARGET_SUPPORTS_MTTCG
> -# ifndef TCG_GUEST_DEFAULT_MO
> -#  error "TARGET_SUPPORTS_MTTCG without TCG_GUEST_DEFAULT_MO"
> -# endif
> -    return true;
> -#else
> -    return false;
> -#endif
> +    return target_supports_mttcg();
>   }
>   
>   static void tcg_accel_instance_init(Object *obj)
> @@ -137,17 +129,18 @@ static char *tcg_get_thread(Object *obj, Error **errp)
>   static void tcg_set_thread(Object *obj, const char *value, Error **errp)
>   {
>       TCGState *s = TCG_STATE(obj);
> +    const bool oversized_guest = target_long_bits() > TCG_TARGET_REG_BITS;
>   
>       if (strcmp(value, "multi") == 0) {
> -        if (TCG_OVERSIZED_GUEST) {
> +        if (oversized_guest) {
>               error_setg(errp, "No MTTCG when guest word size > hosts");
>           } else if (icount_enabled()) {
>               error_setg(errp, "No MTTCG when icount is enabled");
>           } else {
> -#ifndef TARGET_SUPPORTS_MTTCG
> -            warn_report("Guest not yet converted to MTTCG - "
> -                        "you may get unexpected results");
> -#endif
> +            if (target_supports_mttcg()) {
> +                warn_report("Guest not yet converted to MTTCG - "
> +                            "you may get unexpected results");
> +            }
>               s->mttcg_enabled = true;
>           }
>       } else if (strcmp(value, "single") == 0) {


All of this happens at startup.  Are you sure you're going to have determined these values 
early enough for heterogeneous mode?  I guess for the moment you don't care, because 
they're all constants supplied by functions.

Acked-by: Richard Henderson <richard.henderson@linaro.org>


r~

Hi Anton,

On 19/1/24 15:40, Anton Johansson wrote:
> Uses target_supports_mttcg() and target_long_bits() to turn ifdefs into
> runtime branches.
> 
> Signed-off-by: Anton Johansson <anjo@rev.ng>
> ---
>   accel/tcg/tcg-all.c | 25 +++++++++----------------
>   1 file changed, 9 insertions(+), 16 deletions(-)


>   static void tcg_accel_instance_init(Object *obj)
> @@ -137,17 +129,18 @@ static char *tcg_get_thread(Object *obj, Error **errp)
>   static void tcg_set_thread(Object *obj, const char *value, Error **errp)
>   {
>       TCGState *s = TCG_STATE(obj);
> +    const bool oversized_guest = target_long_bits() > TCG_TARGET_REG_BITS;
>   
>       if (strcmp(value, "multi") == 0) {
> -        if (TCG_OVERSIZED_GUEST) {
> +        if (oversized_guest) {
>               error_setg(errp, "No MTTCG when guest word size > hosts");
>           } else if (icount_enabled()) {
>               error_setg(errp, "No MTTCG when icount is enabled");
>           } else {
> -#ifndef TARGET_SUPPORTS_MTTCG
> -            warn_report("Guest not yet converted to MTTCG - "
> -                        "you may get unexpected results");
> -#endif
> +            if (target_supports_mttcg()) {

I started smth similar but then realized this call has to be per target,
so put my work on hold. My plan is to have a single common tcg
accelerator framework, having target-specific code handled by vcpu
dispatchers. Is your plan to have each target link its own tcg?

> +                warn_report("Guest not yet converted to MTTCG - "
> +                            "you may get unexpected results");
> +            }
>               s->mttcg_enabled = true;
>           }
>       } else if (strcmp(value, "single") == 0) {

On 23/01/24, Philippe Mathieu-Daudé wrote:
> Hi Anton,
> 
> On 19/1/24 15:40, Anton Johansson wrote:
> > Uses target_supports_mttcg() and target_long_bits() to turn ifdefs into
> > runtime branches.
> > 
> > Signed-off-by: Anton Johansson <anjo@rev.ng>
> > ---
> >   accel/tcg/tcg-all.c | 25 +++++++++----------------
> >   1 file changed, 9 insertions(+), 16 deletions(-)
> 
> 
> >   static void tcg_accel_instance_init(Object *obj)
> > @@ -137,17 +129,18 @@ static char *tcg_get_thread(Object *obj, Error **errp)
> >   static void tcg_set_thread(Object *obj, const char *value, Error **errp)
> >   {
> >       TCGState *s = TCG_STATE(obj);
> > +    const bool oversized_guest = target_long_bits() > TCG_TARGET_REG_BITS;
> >       if (strcmp(value, "multi") == 0) {
> > -        if (TCG_OVERSIZED_GUEST) {
> > +        if (oversized_guest) {
> >               error_setg(errp, "No MTTCG when guest word size > hosts");
> >           } else if (icount_enabled()) {
> >               error_setg(errp, "No MTTCG when icount is enabled");
> >           } else {
> > -#ifndef TARGET_SUPPORTS_MTTCG
> > -            warn_report("Guest not yet converted to MTTCG - "
> > -                        "you may get unexpected results");
> > -#endif
> > +            if (target_supports_mttcg()) {
> 
> I started smth similar but then realized this call has to be per target,
> so put my work on hold. My plan is to have a single common tcg
> accelerator framework, having target-specific code handled by vcpu
> dispatchers. Is your plan to have each target link its own tcg?

Yes I was leaning towards one tcg per target, but hadn't put much 
thought into it. I think your approach is better.  This patchset was
primarily focused on getting accl/tcg/ to compile once, with 
heterogeneous stuff coming down the line. IMO it becomes a bit easier to 
see what target-specific information we really need.

What do you think of a simple TargetConfig struct for information such 
as target_supports_mttcg() and the other functions introduced in 
cpu-target.c? We probably need dispatcher for other stuff but I think we 
can get quite far with struct.