On 06.02.2026 17:15, Alejandro Vallejo wrote:
> Open code the vendor check through the policy as a one-off. The emulator
> embeds amd_like() in macros and is called in MANY places. Using a
> local variable (cp->x86_vendor) makes it a lot smaller (300-400 bytes
> smaller). So treat this as the exception it is and let it use the policy
> rather than boot_cpu_data.

As elsewhere you mainly discuss benefits for the single-vendor case, is the
above about the opposite situation? Else why would codegen suffer this much
here?

Using cp also is preferable for test and fuzzing harnesses, which don't
even know boot_cpu_data.

> @@ -30,8 +31,15 @@ void BUG(void);
>  #  define X86EMUL_NO_SIMD
>  # endif
>  
> +/* intentionally avoid cpu_vendor(), as it produces much worse codegen */

Nit (style): Capital letter wanted at the start.

> +# define x86emul_cpu(cp) ((X86_ENABLED_VENDORS ==            \
> +                           ISOLATE_LSB(X86_ENABLED_VENDORS)) \
> +                               ? X86_ENABLED_VENDORS         \
> +                               : ((cp)->x86_vendor & X86_ENABLED_VENDORS))

Nit: Indentation. The ? and : want to align with the controlling expression.

Further, is this a good name, without "vendor" in it?

And then I'm of two minds here as to the use of the macro parameter: On one
hand we can be pretty certain what is passed in won't have side effects.
Otoh in a hypothetical odd case (seeing that this lives in a header file,
not local to an isolated piece of code) where there would be one, the
argument being evaluated unreliably could cause an unpleasant surprise.
The more ...

>  #else /* !__XEN__ */
>  # include "x86-emulate.h"
> +# define x86emul_cpu(cp) ((cp)->x86_vendor)

... that the same wouldn't be observable in the fuzzing or test harnesses.

Jan

Reply via email to