* [PATCH] x86emul: Cache the amd_like() output on x86_emulate() entry
@ 2026-09-21 9:25 Alejandro Vallejo
2026-09-22 13:34 ` Jan Beulich
0 siblings, 1 reply; 3+ messages in thread
From: Alejandro Vallejo @ 2026-09-21 9:25 UTC (permalink / raw)
To: xen-devel
Cc: Alejandro Vallejo, Jan Beulich, Andrew Cooper,
Roger Pau Monné, Teddy Astie
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%
---
xen/arch/x86/x86_emulate/x86_emulate.c | 19 ++++++++++---------
1 file changed, 10 insertions(+), 9 deletions(-)
diff --git a/xen/arch/x86/x86_emulate/x86_emulate.c b/xen/arch/x86/x86_emulate/x86_emulate.c
index 89c37fea2c..c69ef0781e 100644
--- a/xen/arch/x86/x86_emulate/x86_emulate.c
+++ b/xen/arch/x86/x86_emulate/x86_emulate.c
@@ -363,7 +363,7 @@ do { \
#define jmp_rel(rel) \
do { \
unsigned long ip = _regs.r(ip) + (int)(rel); \
- if ( op_bytes == 2 && (amd_like(ctxt) || !mode_64bit()) ) \
+ if ( op_bytes == 2 && (is_amd_like || !mode_64bit()) ) \
ip = (uint16_t)ip; \
else if ( !mode_64bit() ) \
ip = (uint32_t)ip; \
@@ -576,7 +576,7 @@ static inline void put_loop_count(
* zero extend relevant registers first when using 32-bit \
* addressing in 64-bit mode. \
*/ \
- if ( !amd_like(ctxt) && mode_64bit() && ad_bytes == 4 ) \
+ if ( !is_amd_like && mode_64bit() && ad_bytes == 4 ) \
{ \
_regs.r(cx) = 0; \
if ( extend_si ) _regs.r(si) = (uint32_t)_regs.r(si); \
@@ -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;
int rc;
uint8_t b, d, *opc = NULL;
@@ -1810,7 +1811,7 @@ x86_emulate(
if ( ea.type == OP_REG )
src.val = *ea.reg;
else if ( (rc = read_ulong(ea.mem.seg, ea.mem.off, &src.val,
- (op_bytes == 2 && !amd_like(ctxt)
+ (op_bytes == 2 && !is_amd_like
? 2 : 4),
ctxt, ops)) )
goto done;
@@ -2356,7 +2357,7 @@ x86_emulate(
case 0xc2: /* ret imm16 (near) */
case 0xc3: /* ret (near) */
- op_bytes = (op_bytes == 4 || !amd_like(ctxt)) && mode_64bit()
+ op_bytes = (op_bytes == 4 || !is_amd_like) && mode_64bit()
? 8 : op_bytes;
if ( (rc = read_ulong(x86_seg_ss, sp_post_inc(op_bytes + src.val),
&dst.val, op_bytes, ctxt, ops)) != 0 ||
@@ -3089,7 +3090,7 @@ x86_emulate(
if ( (rc = ops->read_msr(MSR_EFER, &msr_val, ctxt)) != X86EMUL_OKAY )
goto done;
generate_exception_if((msr_val & EFER_SCE) == 0, X86_EXC_UD);
- generate_exception_if(!amd_like(ctxt) && !mode_64bit(), X86_EXC_UD);
+ generate_exception_if(!is_amd_like && !mode_64bit(), X86_EXC_UD);
if ( (rc = ops->read_msr(MSR_STAR, &msr_val, ctxt)) != X86EMUL_OKAY )
goto done;
@@ -3174,7 +3175,7 @@ x86_emulate(
if ( (rc = ops->read_msr(MSR_EFER, &msr_val, ctxt)) != X86EMUL_OKAY )
goto done;
generate_exception_if(!(msr_val & EFER_SCE), X86_EXC_UD);
- generate_exception_if(!amd_like(ctxt) && !mode_64bit(), X86_EXC_UD);
+ generate_exception_if(!is_amd_like && !mode_64bit(), X86_EXC_UD);
generate_exception_if(!mode_ring0(), X86_EXC_GP, 0);
generate_exception_if(!in_protmode(ctxt, ops), X86_EXC_GP, 0);
#ifdef __x86_64__
@@ -3200,7 +3201,7 @@ x86_emulate(
sreg.attr = 0xcf3; /* G+DB+P+DPL3+S+Data */
/* Only the selector part of SS gets updated by AMD and alike. */
- if ( amd_like(ctxt) )
+ if ( is_amd_like )
{
fail_if(!ops->read_segment);
if ( (rc = ops->read_segment(x86_seg_ss, &sreg,
@@ -3924,7 +3925,7 @@ x86_emulate(
case X86EMUL_OPC(0x0f, 0x34): /* sysenter */
vcpu_must_have(sep);
- generate_exception_if(amd_like(ctxt) && ctxt->lma, X86_EXC_UD);
+ generate_exception_if(is_amd_like && ctxt->lma, X86_EXC_UD);
generate_exception_if(!in_protmode(ctxt, ops), X86_EXC_GP, 0);
fail_if(ops->read_msr == NULL);
@@ -3973,7 +3974,7 @@ x86_emulate(
case X86EMUL_OPC(0x0f, 0x35): /* sysexit */
vcpu_must_have(sep);
- generate_exception_if(amd_like(ctxt) && ctxt->lma, X86_EXC_UD);
+ generate_exception_if(is_amd_like && ctxt->lma, X86_EXC_UD);
generate_exception_if(!mode_ring0(), X86_EXC_GP, 0);
generate_exception_if(!in_protmode(ctxt, ops), X86_EXC_GP, 0);
base-commit: adbbbd47a1fad8e3bc1ab65c555f11d831fd6681
--
2.43.0
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH] x86emul: Cache the amd_like() output on x86_emulate() entry
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
2026-09-22 18:04 ` Alejandro Vallejo
0 siblings, 1 reply; 3+ messages in thread
From: Jan Beulich @ 2026-09-22 13:34 UTC (permalink / raw)
To: Alejandro Vallejo
Cc: Andrew Cooper, Roger Pau Monné, Teddy Astie, xen-devel
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
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] x86emul: Cache the amd_like() output on x86_emulate() entry
2026-09-22 13:34 ` Jan Beulich
@ 2026-09-22 18:04 ` Alejandro Vallejo
0 siblings, 0 replies; 3+ messages in thread
From: Alejandro Vallejo @ 2026-09-22 18:04 UTC (permalink / raw)
To: Jan Beulich; +Cc: Andrew Cooper, Roger Pau Monné, Teddy Astie, xen-devel
On Tue Sep 22, 2026 at 3:34 PM CEST, Jan Beulich wrote:
> 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.
I have some more interesting results. I can very much repro at that
commit with GCC 13 with debug=y, but ALSO the variable must be set at
the tail of all other locals. I never noticed the gain was gone after
moving where it is in this patch.
Clearly I misattributed the origin of the shrinkage. In retrospect, it
turns out to be an unrelated matter. I _THINK_ register pressure is
somehow precluding the inlining of a helper. The problem is gone in
debug=n where CSE ensures all is good.
If I find a way of shrinking debug builds in a way that's more resilient
to compiler specifics I'll push that as a separate patch, but for now
I'll just drop this. Clearly it doesn't matter here for a realistic
build.
Apologies for the noise. Codegen is hard :)
Alejandro
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2026-09-22 18:05 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
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
2026-09-22 18:04 ` Alejandro Vallejo
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.