All of lore.kernel.org
 help / color / mirror / Atom feed
From: Jan Beulich <jbeulich@suse.com>
To: Alejandro Vallejo <alejandro.garciavallejo@amd.com>
Cc: "Andrew Cooper" <andrew.cooper3@citrix.com>,
	"Roger Pau Monné" <roger@xenproject.org>,
	"Teddy Astie" <teddy.astie@vates.tech>,
	xen-devel@lists.xenproject.org
Subject: Re: [PATCH] x86emul: Cache the amd_like() output on x86_emulate() entry
Date: Tue, 22 Sep 2026 15:34:15 +0200	[thread overview]
Message-ID: <baabb75a-dc95-4fad-a682-e4eb40071a73@suse.com> (raw)
In-Reply-To: <20260921092545.37148-1-alejandro.garciavallejo@amd.com>

On 21.09.2026 11:25, Alejandro Vallejo wrote:
> The current code instantiates amd_like() way too many times, leading to
> codegen explosion. Unconditionally call it early on, and use that
> variable everywhere. This shrinks the emulator by ~3KiB.
> 
> No a functional change.
> 
> Signed-off-by: Alejandro Vallejo <alejandro.garciavallejo@amd.com>
> ---
> pipeline: https://gitlab.com/xen-project/people/agvallejo/xen/-/pipelines/2860748404
>           (it's red because of an caching issue and personal branch name
>            conventions it's just arm. x86 passes in full)
> 
> bloat-o-meter before-after patch.
> 
> add/remove: 0/0 grow/shrink: 0/1 up/down: 0/-3366 (-3366)
> Function                                     old     new   delta
> x86_emulate                               209351  205985   -3366
> Total: Before=3617694, After=3614328, chg -0.09%

That's surprisingly big a difference, considering how simple amd_like() is.
I'm counting 24 instances, so the savings would be well over 100 bytes per
instance.

That said - there are more uses, and I wonder why you ...

> @@ -1310,6 +1310,7 @@ x86_emulate(
>      /* Shadow copy of register state. Committed on successful emulation. */
>      struct cpu_user_regs _regs = *ctxt->regs;
>      const struct cpu_policy *__maybe_unused cp = ctxt->cpu_policy;
> +    bool is_amd_like = amd_like(ctxt);
>      struct x86_emulate_state state;

... make a standalone local variable here when you could introduce a new
field in struct x86_emulate_state, which would then filled early in
x86emul_decode(), and which could then be used elsewhere as well. With
that amd_like() could then likely also move from private.h to decode.c;
better yet - perhaps the function then wouldn't be needed at all anymore.

Another way to reduce the overhead at least some would be to make more use
of _amd_like(), now that x86_emulate() has a "cp" local variable.

Jan


  reply	other threads:[~2026-09-22 13:34 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-09-21  9:25 [PATCH] x86emul: Cache the amd_like() output on x86_emulate() entry Alejandro Vallejo
2026-09-22 13:34 ` Jan Beulich [this message]
2026-09-22 18:04   ` Alejandro Vallejo

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=baabb75a-dc95-4fad-a682-e4eb40071a73@suse.com \
    --to=jbeulich@suse.com \
    --cc=alejandro.garciavallejo@amd.com \
    --cc=andrew.cooper3@citrix.com \
    --cc=roger@xenproject.org \
    --cc=teddy.astie@vates.tech \
    --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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.