From: Thomas Huth <thuth@redhat.com>
To: "Igor Mammedov" <imammedo@redhat.com>,
"Philippe Mathieu-Daudé" <philmd@linaro.org>
Cc: qemu-devel@nongnu.org, qemu-riscv@nongnu.org,
qemu-s390x@nongnu.org, Paolo Bonzini <pbonzini@redhat.com>,
kvm@vger.kernel.org, qemu-ppc@nongnu.org, qemu-arm@nongnu.org,
Richard Henderson <richard.henderson@linaro.org>,
"Michael S. Tsirkin" <mst@redhat.com>,
Marcel Apfelbaum <marcel.apfelbaum@gmail.com>,
Eduardo Habkost <eduardo@habkost.net>,
Stefano Stabellini <sstabellini@kernel.org>,
Anthony Perard <anthony.perard@citrix.com>,
Paul Durrant <paul@xen.org>, Cameron Esfahani <dirty@apple.com>,
Roman Bolshakov <rbolshakov@ddn.com>,
Marcelo Tosatti <mtosatti@redhat.com>,
David Woodhouse <dwmw2@infradead.org>,
xen-devel@lists.xenproject.org
Subject: Re: [PATCH v3 14/29] target/i386: Prefer fast cpu_env() over slower CPU QOM cast macro
Date: Tue, 12 Mar 2024 12:24:49 +0100 [thread overview]
Message-ID: <96422cf0-30a8-4e2a-99c5-8388ec342e36@redhat.com> (raw)
In-Reply-To: <20240130140115.135f533d@imammedo.users.ipa.redhat.com>
On 30/01/2024 14.01, Igor Mammedov wrote:
> On Mon, 29 Jan 2024 17:44:56 +0100
> Philippe Mathieu-Daudé <philmd@linaro.org> 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@linaro.org>
>> ---
> ...
>> 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
next prev parent reply other threads:[~2024-03-12 11:25 UTC|newest]
Thread overview: 50+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-01-29 16:44 [PATCH v3 00/29] hw, target: Prefer fast cpu_env() over slower CPU QOM cast macro Philippe Mathieu-Daudé
2024-01-29 16:44 ` [PATCH v3 01/29] bulk: Access existing variables initialized to &S->F when available Philippe Mathieu-Daudé
2024-01-29 17:20 ` Fabiano Rosas
2024-01-29 19:46 ` BALATON Zoltan
2024-01-30 16:48 ` Zhao Liu
2024-03-08 17:35 ` Anthony PERARD
2024-01-29 16:44 ` [PATCH v3 02/29] hw/core: Declare CPUArchId::cpu as CPUState instead of Object Philippe Mathieu-Daudé
2024-01-31 6:18 ` Thomas Huth
2024-01-29 16:44 ` [PATCH v3 03/29] hw/acpi/cpu: Use CPUState typedef Philippe Mathieu-Daudé
2024-01-29 16:44 ` [PATCH v3 04/29] bulk: Call in place single use cpu_env() Philippe Mathieu-Daudé
2024-01-29 16:44 ` [PATCH v3 05/29] scripts/coccinelle: Add cpu_env.cocci script Philippe Mathieu-Daudé
2024-01-29 16:53 ` Philippe Mathieu-Daudé
2024-01-29 16:44 ` [PATCH v3 06/29] target: Replace CPU_GET_CLASS(cpu -> obj) in cpu_reset_hold() handler Philippe Mathieu-Daudé
2024-01-29 16:44 ` [PATCH v3 07/29] target/alpha: Prefer fast cpu_env() over slower CPU QOM cast macro Philippe Mathieu-Daudé
2024-01-29 16:44 ` [PATCH v3 08/29] target/arm: " Philippe Mathieu-Daudé
2024-01-29 16:44 ` [PATCH v3 09/29] target/avr: " Philippe Mathieu-Daudé
2024-01-29 16:44 ` [PATCH v3 10/29] target/cris: " Philippe Mathieu-Daudé
2024-01-29 16:44 ` [PATCH v3 11/29] target/hexagon: " Philippe Mathieu-Daudé
2024-01-30 2:44 ` Brian Cain
2024-01-30 10:00 ` Richard Henderson
2024-01-29 16:44 ` [PATCH v3 12/29] target/hppa: " Philippe Mathieu-Daudé
2024-01-29 20:25 ` Helge Deller
2024-01-29 16:44 ` [PATCH v3 13/29] target/i386/hvf: Use CPUState typedef Philippe Mathieu-Daudé
2024-01-29 16:44 ` [PATCH v3 14/29] target/i386: Prefer fast cpu_env() over slower CPU QOM cast macro Philippe Mathieu-Daudé
2024-01-29 16:50 ` Philippe Mathieu-Daudé
2024-01-30 13:01 ` Igor Mammedov
2024-03-12 11:24 ` Thomas Huth [this message]
2024-01-30 16:20 ` Zhao Liu
2024-01-29 16:44 ` [PATCH v3 15/29] target/loongarch: " Philippe Mathieu-Daudé
2024-01-30 10:14 ` Richard Henderson
2024-01-29 16:44 ` [PATCH v3 16/29] target/m68k: " Philippe Mathieu-Daudé
2024-01-29 16:44 ` [PATCH v3 17/29] target/microblaze: " Philippe Mathieu-Daudé
2024-01-29 16:45 ` [PATCH v3 18/29] target/mips: " Philippe Mathieu-Daudé
2024-01-29 16:45 ` [PATCH v3 19/29] target/nios2: " Philippe Mathieu-Daudé
2024-01-29 16:45 ` [PATCH v3 20/29] target/openrisc: " Philippe Mathieu-Daudé
2024-01-29 16:45 ` [PATCH v3 21/29] target/ppc: " Philippe Mathieu-Daudé
2024-01-29 16:45 ` [PATCH v3 22/29] target/riscv: " Philippe Mathieu-Daudé
2024-01-29 21:53 ` Daniel Henrique Barboza
2024-01-30 8:35 ` Philippe Mathieu-Daudé
2024-01-30 0:39 ` Alistair Francis
2024-01-29 16:45 ` [PATCH v3 23/29] target/rx: " Philippe Mathieu-Daudé
2024-01-29 16:45 ` [PATCH v3 24/29] target/s390x: " Philippe Mathieu-Daudé
2024-01-29 16:45 ` [PATCH v3 25/29] target/sh4: " Philippe Mathieu-Daudé
2024-01-29 16:45 ` [PATCH v3 26/29] target/sparc: " Philippe Mathieu-Daudé
2024-01-30 7:51 ` Philippe Mathieu-Daudé
2024-01-29 16:45 ` [PATCH v3 27/29] target/tricore: " Philippe Mathieu-Daudé
2024-01-29 16:45 ` [PATCH v3 28/29] target/xtensa: " Philippe Mathieu-Daudé
2024-01-29 16:45 ` [PATCH v3 29/29] user: " Philippe Mathieu-Daudé
2024-02-07 21:06 ` Warner Losh
2024-03-12 10:58 ` [PATCH v3 00/29] hw, target: " Thomas Huth
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=96422cf0-30a8-4e2a-99c5-8388ec342e36@redhat.com \
--to=thuth@redhat.com \
--cc=anthony.perard@citrix.com \
--cc=dirty@apple.com \
--cc=dwmw2@infradead.org \
--cc=eduardo@habkost.net \
--cc=imammedo@redhat.com \
--cc=kvm@vger.kernel.org \
--cc=marcel.apfelbaum@gmail.com \
--cc=mst@redhat.com \
--cc=mtosatti@redhat.com \
--cc=paul@xen.org \
--cc=pbonzini@redhat.com \
--cc=philmd@linaro.org \
--cc=qemu-arm@nongnu.org \
--cc=qemu-devel@nongnu.org \
--cc=qemu-ppc@nongnu.org \
--cc=qemu-riscv@nongnu.org \
--cc=qemu-s390x@nongnu.org \
--cc=rbolshakov@ddn.com \
--cc=richard.henderson@linaro.org \
--cc=sstabellini@kernel.org \
--cc=xen-devel@lists.xenproject.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).