| Message ID | 1499586614-20507-20-git-send-email-cota@braap.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 07/08/2017 09:50 PM, Emilio G. Cota wrote: > @@ -409,6 +411,18 @@ void tcg_context_init(TCGContext *s) > } > > /* > + * Clone the initial TCGContext. Used by TCG threads to copy the TCGContext > + * set up by their parent thread via tcg_context_init(). > + */ > +void tcg_context_clone(TCGContext *s) > +{ > + if (unlikely(tcg_init_ctx == NULL || tcg_init_ctx == s)) { > + tcg_abort(); > + } > + memcpy(s, tcg_init_ctx, sizeof(*s)); > +} Under what conditions will this be called? How much of this might you need to zero out again after the fact? r~
On Sun, Jul 09, 2017 at 10:48:27 -1000, Richard Henderson wrote: > On 07/08/2017 09:50 PM, Emilio G. Cota wrote: > >@@ -409,6 +411,18 @@ void tcg_context_init(TCGContext *s) > > } > > /* > >+ * Clone the initial TCGContext. Used by TCG threads to copy the TCGContext > >+ * set up by their parent thread via tcg_context_init(). > >+ */ > >+void tcg_context_clone(TCGContext *s) > >+{ > >+ if (unlikely(tcg_init_ctx == NULL || tcg_init_ctx == s)) { > >+ tcg_abort(); > >+ } > >+ memcpy(s, tcg_init_ctx, sizeof(*s)); > >+} > > Under what conditions will this be called? How much of this might you need > to zero out again after the fact? I checked the profile/tb counts and all of them are zero when this is called, which is right after the thread has been created. But it is conceivable that those counts might be !0 for some targets, so yes it'd be better to actively zero out those. I don't think there are any other fields that would have to be zeroed out. E.
Emilio G. Cota <cota@braap.org> writes: > Before we make TCGContext thread-local. > > Signed-off-by: Emilio G. Cota <cota@braap.org> > --- > tcg/tcg.h | 1 + > tcg/tcg.c | 14 ++++++++++++++ > 2 files changed, 15 insertions(+) > > diff --git a/tcg/tcg.h b/tcg/tcg.h > index 2a64ee2..be5f3fd 100644 > --- a/tcg/tcg.h > +++ b/tcg/tcg.h > @@ -778,6 +778,7 @@ static inline void *tcg_malloc(int size) > } > > void tcg_context_init(TCGContext *s); > +void tcg_context_clone(TCGContext *s); > void tcg_prologue_init(TCGContext *s); > void tcg_register_thread(void); > void tcg_func_start(TCGContext *s); > diff --git a/tcg/tcg.c b/tcg/tcg.c > index 2f003a0..8febf53 100644 > --- a/tcg/tcg.c > +++ b/tcg/tcg.c > @@ -117,6 +117,7 @@ static bool tcg_out_tb_finalize(TCGContext *s); > > #define TCG_HIGHWATER 1024 > > +static const TCGContext *tcg_init_ctx; > static QemuMutex tcg_lock; > > /* > @@ -353,6 +354,7 @@ void tcg_context_init(TCGContext *s) > TCGArgConstraint *args_ct; > int *sorted_args; > > + tcg_init_ctx = s; > memset(s, 0, sizeof(*s)); > s->nb_globals = 0; > > @@ -409,6 +411,18 @@ void tcg_context_init(TCGContext *s) > } > > /* > + * Clone the initial TCGContext. Used by TCG threads to copy the TCGContext > + * set up by their parent thread via tcg_context_init(). > + */ > +void tcg_context_clone(TCGContext *s) > +{ > + if (unlikely(tcg_init_ctx == NULL || tcg_init_ctx == s)) { > + tcg_abort(); > + } > + memcpy(s, tcg_init_ctx, sizeof(*s)); > +} > + Why a copy approach as opposed to a plain tcg_context_new() with the appropriate init cleanup. Is is it just because of the extra shared stuff in: #if defined(CONFIG_SOFTMMU) /* There's no guest base to take into account, so go ahead and initialize the prologue now. */ tcg_prologue_init(&tcg_ctx); code_gen_set_region_size(&tcg_ctx); #endif ? > +/* > * Allocate TBs right before their corresponding translated code, making > * sure that TBs and code are on different cache lines. > */ -- Alex Bennée
On 07/12/2017 06:02 AM, Alex Bennée wrote: > Why a copy approach as opposed to a plain tcg_context_new() with the > appropriate init cleanup. We need the globals that were set up by the target/foo/translate.c code to be the same for each thread. That's what makes it easier to copy the struct. r~
Richard Henderson <rth@twiddle.net> writes: > On 07/12/2017 06:02 AM, Alex Bennée wrote: >> Why a copy approach as opposed to a plain tcg_context_new() with the >> appropriate init cleanup. > > We need the globals that were set up by the target/foo/translate.c > code to be the same for each thread. That's what makes it easier to > copy the struct. Ahh I see. Maybe expand the comment for the function a little then? -- Alex Bennée
diff --git a/tcg/tcg.h b/tcg/tcg.h index 2a64ee2..be5f3fd 100644 --- a/tcg/tcg.h +++ b/tcg/tcg.h @@ -778,6 +778,7 @@ static inline void *tcg_malloc(int size) } void tcg_context_init(TCGContext *s); +void tcg_context_clone(TCGContext *s); void tcg_prologue_init(TCGContext *s); void tcg_register_thread(void); void tcg_func_start(TCGContext *s); diff --git a/tcg/tcg.c b/tcg/tcg.c index 2f003a0..8febf53 100644 --- a/tcg/tcg.c +++ b/tcg/tcg.c @@ -117,6 +117,7 @@ static bool tcg_out_tb_finalize(TCGContext *s); #define TCG_HIGHWATER 1024 +static const TCGContext *tcg_init_ctx; static QemuMutex tcg_lock; /* @@ -353,6 +354,7 @@ void tcg_context_init(TCGContext *s) TCGArgConstraint *args_ct; int *sorted_args; + tcg_init_ctx = s; memset(s, 0, sizeof(*s)); s->nb_globals = 0; @@ -409,6 +411,18 @@ void tcg_context_init(TCGContext *s) } /* + * Clone the initial TCGContext. Used by TCG threads to copy the TCGContext + * set up by their parent thread via tcg_context_init(). + */ +void tcg_context_clone(TCGContext *s) +{ + if (unlikely(tcg_init_ctx == NULL || tcg_init_ctx == s)) { + tcg_abort(); + } + memcpy(s, tcg_init_ctx, sizeof(*s)); +} + +/* * Allocate TBs right before their corresponding translated code, making * sure that TBs and code are on different cache lines. */
Before we make TCGContext thread-local. Signed-off-by: Emilio G. Cota <cota@braap.org> --- tcg/tcg.h | 1 + tcg/tcg.c | 14 ++++++++++++++ 2 files changed, 15 insertions(+)