| Message ID | 20240119144024.14289-31-anjo@rev.ng (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Compile accel/tcg once (partially) | expand |
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.
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~
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) {
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(-)