Richard Henderson writes:
On 03/09/2016 09:38 AM, Lluís Vilanova wrote:
Hi,
NOTE: I won't be throwing patches anytime soon, I just want to know if there's
interest in this for the future.
While adding events for tracing guest instructions, I've found that the
per-target "gen_intermediate_code()" function is very similar but not exactly
the same for each of the targets. This makes architecture-agnostic features
harder to maintain across targets, specially when it comes to their relative
order.
So, would it be worth it if I generalized part of that code into an
architecture-agnostic function that calls into target-specific hooks wherever it
needs extending? There are many ways to do it that we can discuss later.
It's worth talking about, since I do believe it would make long-term maintenance
across the targets easier.
These "target-specific hooks" probably ought not be "hooks" in the
traditional sense of attaching them to CPUState. I'd be more comfortable with a
refactoring that used include files -- maybe .h or maybe .inc.c. If we do the
normal sort of hook, then we've got to either expose DisasContext in places we
shouldn't, or dynamically allocate it. Neither seems particularly appealing.
Great. I pondered about using QOM vs "template code", and I'm leaning towards
the latter:
* translate.c:
struct DisasContextArch {
DisasContext common;
};
// implement "hooks"
void gen_intermediate_code(CPUArchState *env, TranslationBlock *tb)
{
DisasContextArch dc;
gen_intermediate_code_template(get_cpu(env), &dc.common, tb);
}
* translate-template.h:
struct DisasContext { /* ... */ };
void gen_intermediate_code_template(CPUState *cpu, DisasContext *dc,
TranslationBlock *tb)
{
// init dc
// arch-specific init dc hook
// gen_icount, etc.
while (true) {
// generic processing code with calls to hooks
}
}
While initially simpler, the "template code" still feels a little dirty to
me. With QOM, you could implement a DisasContextClass that provides the
per-target hook pointers, which can be globally allocated once per target and
pointed to by the DisasContext allocated in stack.
I'm not sure about what you mean by exposing DisasContext in places it shouldn't
be, though.