On 05.12.2023 14:08, Roger Pau Monné wrote: > On Tue, Dec 05, 2023 at 12:49:38PM +0100, Jan Beulich wrote: >> On 28.11.2023 11:03, Roger Pau Monne wrote: >>> --- /dev/null >>> +++ b/xen/arch/x86/test-smc-lp-alt.c >>> @@ -0,0 +1,23 @@ >>> +/* SPDX-License-Identifier: GPL-2.0 */ >>> + >>> +#include <asm/test-smc.h> >>> + >>> +/* >>> + * Interesting case because `return false` can be encoded as an xor >>> + * instruction, which is shorter than `return true` which is a mov >>> instruction, >>> + * and also shorter than a jmp instruction. >>> + */ >> >> I'm a little wary of this comment: "mov $1, %al" is two bytes only, just like > > Don't we need to zero the high part of the register also? Or since > the return type is a bool the compiler could assume it's truncated by > the caller?
I think so, yes. I.e. ... >> "xor %eax, %eax" is. > > GCC 13.2 (from godbolt) generates at -O2: > > mov $0x1,%eax > ret > > Which is 5 bytes long mov insn. ... at -Os I'd kind of expect the compiler to use the shorter (albeit slower to execute) "mov $1,%al" (unless of course I'm overlooking a specific rule in psABI). > The return false case is: > > xor %eax,%eax > ret > > I can adjust to mention this specific behavior. > >>> +bool cf_check test_lp_insn_replacement(void) >> >> What's the purpose of the cf_check here? > > Because it's added to the array of test functions to call in > test_smc(). Doesn't it need cf_check in that case? Oh, of course it does. I managed to overlook that use (misguided by one of the two files being built without being actually used). Jan
