On 30/01/2024 14.01, Igor Mammedov wrote:
On Mon, 29 Jan 2024 17:44:56 +0100
Philippe Mathieu-Daudé <philmd@xxxxxxxxxx> wrote:
Mechanical patch produced running the command documented
in scripts/coccinelle/cpu_env.cocci_template header.
commenting here since, I'm not expert on coccinelle scripts.
On negative side we are permanently loosing type checking in this area.
Not really that much. Have a look at cpu_env(), it has a comment saying:
"We validate that CPUArchState follows CPUState in cpu-all.h"
So instead of run-time checking, the check should have already been done
during compile time, i.e. when you have a valid CPUState pointer, it should
be possible to derive a valid CPUArchState pointer from it without much
further checking during runtime.
Is it worth it, what gains do we get with this series?
It's a small optimization, but why not?
Side note,
QOM cast expenses you are replacing could be negated by disabling
CONFIG_QOM_CAST_DEBUG without killing type check code when it's enabled.
That way you will speed up not only cpuenv access but also all other casts
across the board.
Yes, but that checking is enabled by default and does not have such
compile-time checks that could be used instead, so I think Philippe's series
here is still a good idea.
Signed-off-by: Philippe Mathieu-Daudé <philmd@xxxxxxxxxx>
---
...
static inline void vmx_clear_nmi_blocking(CPUState *cpu)
{
- X86CPU *x86_cpu = X86_CPU(cpu);
- CPUX86State *env = &x86_cpu->env;
-
- env->hflags2 &= ~HF2_NMI_MASK;
+ cpu_env(cpu)->hflags2 &= ~HF2_NMI_MASK;
this style of de-referencing return value of macro/function
was discouraged in past and preferred way was 'Foo f = CAST(me); f->some_access
(it's just imprint speaking, I don't recall where it comes from)
I agree, though the new code is perfectly valid, it looks nicer if we'd use
a variable here instead.
Thomas
