All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/4] Fix building tools/tests/x86_emulator with clang-21
@ 2026-02-23 10:04 Edwin Török
  2026-02-23 10:04 ` [PATCH 1/4] tools/tests/x86_emulator: fix 'shifting a negative signed value is undefined' Edwin Török
                   ` (3 more replies)
  0 siblings, 4 replies; 19+ messages in thread
From: Edwin Török @ 2026-02-23 10:04 UTC (permalink / raw)
  To: xen-devel
  Cc: Edwin Török, Jan Beulich, Andrew Cooper,
	Roger Pau Monné, Anthony PERARD

While working on another patch series I noticed that `clang-21` cannot
build `tools/tests/x86_emulator`. Once I fixed that `xmm0` related tests
failed, because `clang` doesn't support `-ffixed-xmm0`.

This series fixes building and running the test with `clang`.

There might still be some latent bugs in the tests though, i.e. `xmm0` may not
be the only reason it fails on clang: the `fxsave` tests have
2 overlapping areas of memory at `res + 0x80`, and `res + 0x100` of length `0x200`.
Instead the 2nd area should start at `0x280`, and the memset should
clear an area of `0x480`.
I didn't attempt to fix all those, because attempting to do so caused
the tests to start failing on GCC.

For convenience this patch series is also available at:
https://gitlab.com/xen-project/people/edwintorok/xen/-/compare/staging...private%2Fedvint%2Femulator?from_project_id=2336572
https://gitlab.com/xen-project/people/edwintorok/xen/-/pipelines/2343355874

Edwin Török (4):
  tools/tests/x86_emulator: fix 'shifting a negative signed value is
    undefined'
  tools/tests/x86_emulator: avoid duplicating loop body
  tools/tests/x86_emulator: fix build on clang-21
  tools/tests/x86_emulator: disable xmm* tests on clang

 tools/tests/x86_emulator/Makefile            |  1 +
 tools/tests/x86_emulator/test_x86_emulator.c | 48 +++++++++++++++++++-
 2 files changed, 47 insertions(+), 2 deletions(-)

-- 
2.47.3



^ permalink raw reply	[flat|nested] 19+ messages in thread

* [PATCH 1/4] tools/tests/x86_emulator: fix 'shifting a negative signed value is undefined'
  2026-02-23 10:04 [PATCH 0/4] Fix building tools/tests/x86_emulator with clang-21 Edwin Török
@ 2026-02-23 10:04 ` Edwin Török
  2026-02-23 15:42   ` Jan Beulich
  2026-02-23 10:04 ` [PATCH 2/4] tools/tests/x86_emulator: avoid duplicating loop body Edwin Török
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 19+ messages in thread
From: Edwin Török @ 2026-02-23 10:04 UTC (permalink / raw)
  To: xen-devel
  Cc: Edwin Török, Jan Beulich, Andrew Cooper,
	Roger Pau Monné, Anthony PERARD

clang-21 refuses to compile this:
```
test_x86_emulator.c:1164:24: error: shifting a negative signed value is undefined [-Werror,-Wshift-negative-value]
 1164 |     regs.r8     = (-1L << 40) + 1;
      |                    ~~~ ^
1 error generated.
```

The shift could also exceed the width of the type on some systems.
I confirmed that both the old and the new code would produce the same value
with GCC on x86-64: `0xffffff0000000001`.

Signed-off-by: Edwin Török <edwin.torok@citrix.com>
---
 tools/tests/x86_emulator/test_x86_emulator.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/tests/x86_emulator/test_x86_emulator.c b/tools/tests/x86_emulator/test_x86_emulator.c
index 39e8056d77..a25eca1634 100644
--- a/tools/tests/x86_emulator/test_x86_emulator.c
+++ b/tools/tests/x86_emulator/test_x86_emulator.c
@@ -1161,7 +1161,7 @@ int main(int argc, char **argv)
     instr[0] = 0x4d; instr[1] = 0x0f; instr[2] = 0xbb; instr[3] = 0x03;
     regs.eflags = EFLAGS_ALWAYS_SET;
     regs.rip    = (unsigned long)&instr[0];
-    regs.r8     = (-1L << 40) + 1;
+    regs.r8     = (~0ULL << 40) + 1;
     regs.r11    = (unsigned long)(res + (1L << 35));
     rc = x86_emulate(&ctxt, &emulops);
     if ( (rc != X86EMUL_OKAY) ||
-- 
2.47.3



^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH 2/4] tools/tests/x86_emulator: avoid duplicating loop body
  2026-02-23 10:04 [PATCH 0/4] Fix building tools/tests/x86_emulator with clang-21 Edwin Török
  2026-02-23 10:04 ` [PATCH 1/4] tools/tests/x86_emulator: fix 'shifting a negative signed value is undefined' Edwin Török
@ 2026-02-23 10:04 ` Edwin Török
  2026-02-23 15:57   ` Jan Beulich
  2026-02-23 10:04 ` [PATCH 3/4] tools/tests/x86_emulator: fix build on clang-21 Edwin Török
  2026-02-23 10:04 ` [PATCH 4/4] tools/tests/x86_emulator: disable xmm* tests on clang Edwin Török
  3 siblings, 1 reply; 19+ messages in thread
From: Edwin Török @ 2026-02-23 10:04 UTC (permalink / raw)
  To: xen-devel
  Cc: Edwin Török, Jan Beulich, Andrew Cooper,
	Roger Pau Monné, Anthony PERARD

clang would duplicate the loop body and end up with a double definition
of the symbol:
```
/tmp/test_x86_emulator-0f3576.s:27823: Error: symbol `vmovsh_to_mem' is already defined
/tmp/test_x86_emulator-0f3576.s:27825: Error: symbol `.Lvmovsh_to_mem_end' is already defined
```

This is a valid transformation, that even GCC might do, see
https://gcc.gnu.org/onlinedocs/gcc/Extended-Asm.html#Volatile-1 which
says that `%=` should be used instead. However the C code here also
needs to know the name of the label, so I don't immediately see how to
solve that.

For now mark the loop variable `volatile` to prevent the optimization as a
workaround.
(another way to fix this could be to move the loop body into a function,
but a compiler might inline it, or specialize it, leading to the same problem)

Signed-off-by: Edwin Török <edwin.torok@citrix.com>
---
 tools/tests/x86_emulator/test_x86_emulator.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/tests/x86_emulator/test_x86_emulator.c b/tools/tests/x86_emulator/test_x86_emulator.c
index a25eca1634..cf4e5cc593 100644
--- a/tools/tests/x86_emulator/test_x86_emulator.c
+++ b/tools/tests/x86_emulator/test_x86_emulator.c
@@ -5248,7 +5248,7 @@ int main(int argc, char **argv)
         memset(res + 3, ~0, 8);
         regs.eax = (unsigned long)res;
         regs.ecx = ~0;
-        for ( i = 0; i < 2; ++i )
+        for (volatile int i = 0; i < 2; ++i )
         {
             decl_insn(vmovsh_to_mem);
 
-- 
2.47.3



^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH 3/4] tools/tests/x86_emulator: fix build on clang-21
  2026-02-23 10:04 [PATCH 0/4] Fix building tools/tests/x86_emulator with clang-21 Edwin Török
  2026-02-23 10:04 ` [PATCH 1/4] tools/tests/x86_emulator: fix 'shifting a negative signed value is undefined' Edwin Török
  2026-02-23 10:04 ` [PATCH 2/4] tools/tests/x86_emulator: avoid duplicating loop body Edwin Török
@ 2026-02-23 10:04 ` Edwin Török
  2026-02-23 16:02   ` Jan Beulich
  2026-02-23 10:04 ` [PATCH 4/4] tools/tests/x86_emulator: disable xmm* tests on clang Edwin Török
  3 siblings, 1 reply; 19+ messages in thread
From: Edwin Török @ 2026-02-23 10:04 UTC (permalink / raw)
  To: xen-devel
  Cc: Edwin Török, Jan Beulich, Andrew Cooper,
	Roger Pau Monné, Anthony PERARD

clang-21's built-in assembler is enabled by default, but it doesn't
support some mnemonics:
```
test_x86_emulator.c:2636:36: error: invalid instruction mnemonic 'fsaves'
 2636 |                        "fidivs %1\n\t"
test_x86_emulator.c:2640:24: error: invalid instruction mnemonic 'frstors'
 2640 |         asm volatile ( "frstors %0" :: "m" (res[25]) : "memory" );
      |                        ^
test_x86_emulator.c:4251:24: error: invalid instruction mnemonic 'vpcmpestriq'
 4251 |                        put_insn(vpcmpestri,
      |                        ^
```

Use the external assembler with Clang for consistency with GCC.

Signed-off-by: Edwin Török <edwin.torok@citrix.com>
---
 tools/tests/x86_emulator/Makefile | 1 +
 1 file changed, 1 insertion(+)

diff --git a/tools/tests/x86_emulator/Makefile b/tools/tests/x86_emulator/Makefile
index 376cfe244d..2d8874a1d1 100644
--- a/tools/tests/x86_emulator/Makefile
+++ b/tools/tests/x86_emulator/Makefile
@@ -297,6 +297,7 @@ x86_emulate:
 
 HOSTCFLAGS-x86_64 := -fno-PIE
 $(call cc-option-add,HOSTCFLAGS-x86_64,HOSTCC,-no-pie)
+$(call cc-option-add,HOSTCFLAGS-x86_64,HOSTCC,-no-integrated-as)
 HOSTCFLAGS += $(CFLAGS_xeninclude) -I. $(HOSTCFLAGS-$(XEN_COMPILE_ARCH))
 
 x86.h := $(addprefix $(XEN_ROOT)/tools/include/xen/asm/,\
-- 
2.47.3



^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH 4/4] tools/tests/x86_emulator: disable xmm* tests on clang
  2026-02-23 10:04 [PATCH 0/4] Fix building tools/tests/x86_emulator with clang-21 Edwin Török
                   ` (2 preceding siblings ...)
  2026-02-23 10:04 ` [PATCH 3/4] tools/tests/x86_emulator: fix build on clang-21 Edwin Török
@ 2026-02-23 10:04 ` Edwin Török
  2026-02-23 16:10   ` Jan Beulich
  3 siblings, 1 reply; 19+ messages in thread
From: Edwin Török @ 2026-02-23 10:04 UTC (permalink / raw)
  To: xen-devel
  Cc: Edwin Török, Jan Beulich, Andrew Cooper,
	Roger Pau Monné, Anthony PERARD

clang-21 doesn't support `-ffixed-xmm0`, so `%xmm0` won't have the
expected value.
Disable these tests on clang.

Signed-off-by: Edwin Török <edwin.torok@citrix.com>
---
 tools/tests/x86_emulator/test_x86_emulator.c | 44 ++++++++++++++++++++
 1 file changed, 44 insertions(+)

diff --git a/tools/tests/x86_emulator/test_x86_emulator.c b/tools/tests/x86_emulator/test_x86_emulator.c
index cf4e5cc593..3d0ad07c6b 100644
--- a/tools/tests/x86_emulator/test_x86_emulator.c
+++ b/tools/tests/x86_emulator/test_x86_emulator.c
@@ -2683,6 +2683,7 @@ int main(int argc, char **argv)
         printf("skipped\n");
 
     printf("%-40s", "Testing fxsave 4(%ecx)...");
+#ifndef __clang__
     if ( stack_exec && cpu_has_fxsr )
     {
         const uint16_t nine = 9;
@@ -2712,6 +2713,7 @@ int main(int argc, char **argv)
         printf("okay\n");
     }
     else
+#endif
         printf("skipped\n");
 
     printf("%-40s", "Testing fxrstor -4(%ecx)...");
@@ -2748,6 +2750,7 @@ int main(int argc, char **argv)
 
 #ifdef __x86_64__
     printf("%-40s", "Testing fxsaveq 8(%edx)...");
+#ifndef __clang__
     if ( stack_exec && cpu_has_fxsr )
     {
         memset(res + 0x80, 0xcc, 0x400);
@@ -2765,6 +2768,7 @@ int main(int argc, char **argv)
         printf("okay\n");
     }
     else
+#endif
         printf("skipped\n");
 #endif
 
@@ -2816,6 +2820,7 @@ int main(int argc, char **argv)
         printf("skipped\n");
 
     printf("%-40s", "Testing movq %xmm0,32(%ecx)...");
+#ifndef __clang__
     if ( stack_exec && cpu_has_sse2 )
     {
         decl_insn(movq_to_mem2);
@@ -2837,9 +2842,11 @@ int main(int argc, char **argv)
         printf("okay\n");
     }
     else
+#endif
         printf("skipped\n");
 
     printf("%-40s", "Testing movq 32(%ecx),%xmm1...");
+#ifndef __clang__
     if ( stack_exec && cpu_has_sse2 )
     {
         decl_insn(movq_from_mem2);
@@ -2860,9 +2867,11 @@ int main(int argc, char **argv)
         printf("okay\n");
     }
     else
+#endif
         printf("skipped\n");
 
     printf("%-40s", "Testing vmovq %xmm1,32(%edx)...");
+#ifndef __clang__
     if ( stack_exec && cpu_has_avx )
     {
         decl_insn(vmovq_to_mem);
@@ -2884,9 +2893,11 @@ int main(int argc, char **argv)
         printf("okay\n");
     }
     else
+#endif
         printf("skipped\n");
 
     printf("%-40s", "Testing vmovq 32(%edx),%xmm0...");
+#ifndef __clang__
     if ( stack_exec && cpu_has_avx )
     {
         decl_insn(vmovq_from_mem);
@@ -2907,9 +2918,11 @@ int main(int argc, char **argv)
         printf("okay\n");
     }
     else
+#endif
         printf("skipped\n");
 
     printf("%-40s", "Testing {evex} vmovq %xmm1,32(%edx)...");
+#ifndef __clang__
     if ( stack_exec && cpu_has_avx512f )
     {
         decl_insn(evex_vmovq_to_mem);
@@ -2931,9 +2944,11 @@ int main(int argc, char **argv)
         printf("okay\n");
     }
     else
+#endif
         printf("skipped\n");
 
     printf("%-40s", "Testing {evex} vmovq 32(%edx),%xmm0...");
+#ifndef __clang__
     if ( stack_exec && cpu_has_avx512f )
     {
         decl_insn(evex_vmovq_from_mem);
@@ -2954,9 +2969,11 @@ int main(int argc, char **argv)
         printf("okay\n");
     }
     else
+#endif
         printf("skipped\n");
 
     printf("%-40s", "Testing movdqu %xmm2,(%ecx)...");
+#ifndef __clang__
     if ( stack_exec && cpu_has_sse2 )
     {
         decl_insn(movdqu_to_mem);
@@ -2976,9 +2993,11 @@ int main(int argc, char **argv)
         printf("okay\n");
     }
     else
+#endif
         printf("skipped\n");
 
     printf("%-40s", "Testing movdqu (%edx),%xmm4...");
+#ifndef __clang__
     if ( stack_exec && cpu_has_sse2 )
     {
         decl_insn(movdqu_from_mem);
@@ -3001,6 +3020,7 @@ int main(int argc, char **argv)
         printf("okay\n");
     }
     else
+#endif
         printf("skipped\n");
 
     printf("%-40s", "Testing vmovdqu %ymm2,(%ecx)...");
@@ -3386,6 +3406,7 @@ int main(int argc, char **argv)
         printf("skipped\n");
 
     printf("%-40s", "Testing movd %xmm2,32(%edx)...");
+#ifndef __clang__
     if ( stack_exec && cpu_has_sse2 )
     {
         decl_insn(movd_to_mem2);
@@ -3407,9 +3428,11 @@ int main(int argc, char **argv)
         printf("okay\n");
     }
     else
+#endif
         printf("skipped\n");
 
     printf("%-40s", "Testing movd 32(%edx),%xmm3...");
+#ifndef __clang__
     if ( stack_exec && cpu_has_sse2 )
     {
         decl_insn(movd_from_mem2);
@@ -3435,9 +3458,11 @@ int main(int argc, char **argv)
         printf("okay\n");
     }
     else
+#endif
         printf("skipped\n");
 
     printf("%-40s", "Testing vmovd %xmm1,32(%ecx)...");
+#ifndef __clang__
     if ( stack_exec && cpu_has_avx )
     {
         decl_insn(vmovd_to_mem);
@@ -3459,9 +3484,11 @@ int main(int argc, char **argv)
         printf("okay\n");
     }
     else
+#endif
         printf("skipped\n");
 
     printf("%-40s", "Testing vmovd 32(%ecx),%xmm2...");
+#ifndef __clang__
     if ( stack_exec && cpu_has_avx )
     {
         decl_insn(vmovd_from_mem);
@@ -3487,6 +3514,7 @@ int main(int argc, char **argv)
         printf("okay\n");
     }
     else
+#endif
         printf("skipped\n");
 
     printf("%-40s", "Testing {evex} vmovd %xmm3,32(%ecx)...");
@@ -3597,6 +3625,7 @@ int main(int argc, char **argv)
         printf("skipped\n");
 
     printf("%-40s", "Testing movd %xmm2,%ebx...");
+#ifndef __clang__
     if ( stack_exec && cpu_has_sse2 )
     {
         decl_insn(movd_to_reg2);
@@ -3619,6 +3648,7 @@ int main(int argc, char **argv)
         printf("okay\n");
     }
     else
+#endif
         printf("skipped\n");
 
     printf("%-40s", "Testing movd %ebx,%xmm3...");
@@ -3651,6 +3681,7 @@ int main(int argc, char **argv)
         printf("skipped\n");
 
     printf("%-40s", "Testing vmovd %xmm1,%ebx...");
+#ifndef __clang__
     if ( stack_exec && cpu_has_avx )
     {
         decl_insn(vmovd_to_reg);
@@ -3673,6 +3704,7 @@ int main(int argc, char **argv)
         printf("okay\n");
     }
     else
+#endif
         printf("skipped\n");
 
     printf("%-40s", "Testing vmovd %ebx,%xmm2...");
@@ -3705,6 +3737,7 @@ int main(int argc, char **argv)
         printf("skipped\n");
 
     printf("%-40s", "Testing {evex} vmovd %xmm2,%ebx...");
+#ifndef __clang__
     if ( stack_exec && cpu_has_avx512f )
     {
         decl_insn(evex_vmovd_to_reg);
@@ -3728,6 +3761,7 @@ int main(int argc, char **argv)
         printf("okay\n");
     }
     else
+#endif
         printf("skipped\n");
 
     printf("%-40s", "Testing {evex} vmovd %ebx,%xmm1...");
@@ -3781,6 +3815,7 @@ int main(int argc, char **argv)
         printf("skipped\n");
 
     printf("%-40s", "Testing movq %xmm2,32(%edx)...");
+#ifndef __clang__
     if ( stack_exec )
     {
         decl_insn(movq_to_mem4);
@@ -3802,9 +3837,11 @@ int main(int argc, char **argv)
         printf("okay\n");
     }
     else
+#endif
         printf("skipped\n");
 
     printf("%-40s", "Testing vmovq %xmm1,32(%ecx)...");
+#ifndef __clang__
     if ( stack_exec && cpu_has_avx )
     {
         decl_insn(vmovq_to_mem2);
@@ -3830,9 +3867,11 @@ int main(int argc, char **argv)
         printf("okay\n");
     }
     else
+#endif
         printf("skipped\n");
 
     printf("%-40s", "Testing {evex} vmovq %xmm11,32(%ecx)...");
+#ifndef __clang__
     if ( stack_exec && cpu_has_avx512f )
     {
         decl_insn(evex_vmovq_to_mem2);
@@ -3860,6 +3899,7 @@ int main(int argc, char **argv)
         printf("okay\n");
     }
     else
+#endif
         printf("skipped\n");
 
     printf("%-40s", "Testing movq %mm3,%rbx...");
@@ -3883,6 +3923,7 @@ int main(int argc, char **argv)
         printf("skipped\n");
 
     printf("%-40s", "Testing movq %xmm2,%rbx...");
+#ifndef __clang__
     if ( stack_exec )
     {
         decl_insn(movq_to_reg2);
@@ -3900,9 +3941,11 @@ int main(int argc, char **argv)
         printf("okay\n");
     }
     else
+#endif
         printf("skipped\n");
 
     printf("%-40s", "Testing vmovq %xmm1,%rbx...");
+#ifndef __clang__
     if ( stack_exec && cpu_has_avx )
     {
         decl_insn(vmovq_to_reg);
@@ -3920,6 +3963,7 @@ int main(int argc, char **argv)
         printf("okay\n");
     }
     else
+#endif
         printf("skipped\n");
 
     printf("%-40s", "Testing vmovq %xmm22,%rbx...");
-- 
2.47.3



^ permalink raw reply related	[flat|nested] 19+ messages in thread

* Re: [PATCH 1/4] tools/tests/x86_emulator: fix 'shifting a negative signed value is undefined'
  2026-02-23 10:04 ` [PATCH 1/4] tools/tests/x86_emulator: fix 'shifting a negative signed value is undefined' Edwin Török
@ 2026-02-23 15:42   ` Jan Beulich
  2026-02-24 10:12     ` Edwin Torok
  0 siblings, 1 reply; 19+ messages in thread
From: Jan Beulich @ 2026-02-23 15:42 UTC (permalink / raw)
  To: Edwin Török
  Cc: Andrew Cooper, Roger Pau Monné, Anthony PERARD, xen-devel

On 23.02.2026 11:04, Edwin Török wrote:
> --- a/tools/tests/x86_emulator/test_x86_emulator.c
> +++ b/tools/tests/x86_emulator/test_x86_emulator.c
> @@ -1161,7 +1161,7 @@ int main(int argc, char **argv)
>      instr[0] = 0x4d; instr[1] = 0x0f; instr[2] = 0xbb; instr[3] = 0x03;
>      regs.eflags = EFLAGS_ALWAYS_SET;
>      regs.rip    = (unsigned long)&instr[0];
> -    regs.r8     = (-1L << 40) + 1;
> +    regs.r8     = (~0ULL << 40) + 1;

While -1 vs ~0 doesn't make a big difference, I think we want to stick to
"register size" here, and hence have only UL as the suffix. Then (happy to
adjust while committing):
Acked-by: Jan Beulich <jbeulich@suse.com>

Jan


^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 2/4] tools/tests/x86_emulator: avoid duplicating loop body
  2026-02-23 10:04 ` [PATCH 2/4] tools/tests/x86_emulator: avoid duplicating loop body Edwin Török
@ 2026-02-23 15:57   ` Jan Beulich
  2026-02-24  9:21     ` Edwin Torok
  0 siblings, 1 reply; 19+ messages in thread
From: Jan Beulich @ 2026-02-23 15:57 UTC (permalink / raw)
  To: Edwin Török
  Cc: Andrew Cooper, Roger Pau Monné, Anthony PERARD, xen-devel

On 23.02.2026 11:04, Edwin Török wrote:
> clang would duplicate the loop body and end up with a double definition
> of the symbol:
> ```
> /tmp/test_x86_emulator-0f3576.s:27823: Error: symbol `vmovsh_to_mem' is already defined
> /tmp/test_x86_emulator-0f3576.s:27825: Error: symbol `.Lvmovsh_to_mem_end' is already defined
> ```
> 
> This is a valid transformation, that even GCC might do, see
> https://gcc.gnu.org/onlinedocs/gcc/Extended-Asm.html#Volatile-1 which
> says that `%=` should be used instead. However the C code here also
> needs to know the name of the label, so I don't immediately see how to
> solve that.
> 
> For now mark the loop variable `volatile` to prevent the optimization as a
> workaround.
> (another way to fix this could be to move the loop body into a function,
> but a compiler might inline it, or specialize it, leading to the same problem)

Hmm, moving decl and asm() out of the loop wouldn't really work. One option
would be to extend the long-pending [1] to also cover test_x86_emulator.c.
Another might be to make the upper loop bound not a literal 2, but one
loaded from memory, suitably arranged for the compiler to not be able to
const-propagate the value.

But I wonder how many other transformations the compiler could do, causing
the same issue elsewhere. Maybe we need to overhaul how this instruction
instantiation works. (I further wonder how many such issues are lurking in
the many patches I have out for review, plus the yet further ones I didn't
post yet to not pointlessly increase the pile.)

> --- a/tools/tests/x86_emulator/test_x86_emulator.c
> +++ b/tools/tests/x86_emulator/test_x86_emulator.c
> @@ -5248,7 +5248,7 @@ int main(int argc, char **argv)
>          memset(res + 3, ~0, 8);
>          regs.eax = (unsigned long)res;
>          regs.ecx = ~0;
> -        for ( i = 0; i < 2; ++i )
> +        for (volatile int i = 0; i < 2; ++i )

Even if we went with this as a workaround, style should please not be screwed
up.

Also, whichever the workaround, I think a comment wants leaving.

And then I don't think it is a good idea to introduce a shadowing induction
variable.

Jan

[1] https://lists.xen.org/archives/html/xen-devel/2023-04/msg00283.html


^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 3/4] tools/tests/x86_emulator: fix build on clang-21
  2026-02-23 10:04 ` [PATCH 3/4] tools/tests/x86_emulator: fix build on clang-21 Edwin Török
@ 2026-02-23 16:02   ` Jan Beulich
  2026-02-23 16:06     ` Andrew Cooper
  0 siblings, 1 reply; 19+ messages in thread
From: Jan Beulich @ 2026-02-23 16:02 UTC (permalink / raw)
  To: Edwin Török
  Cc: Andrew Cooper, Roger Pau Monné, Anthony PERARD, xen-devel

On 23.02.2026 11:04, Edwin Török wrote:
> clang-21's built-in assembler is enabled by default, but it doesn't
> support some mnemonics:
> ```
> test_x86_emulator.c:2636:36: error: invalid instruction mnemonic 'fsaves'
>  2636 |                        "fidivs %1\n\t"
> test_x86_emulator.c:2640:24: error: invalid instruction mnemonic 'frstors'
>  2640 |         asm volatile ( "frstors %0" :: "m" (res[25]) : "memory" );
>       |                        ^
> test_x86_emulator.c:4251:24: error: invalid instruction mnemonic 'vpcmpestriq'
>  4251 |                        put_insn(vpcmpestri,
>       |                        ^
> ```

Was this reported to them as a bug (or perhaps even two)? That would be nice
to add ...

> --- a/tools/tests/x86_emulator/Makefile
> +++ b/tools/tests/x86_emulator/Makefile
> @@ -297,6 +297,7 @@ x86_emulate:
>  
>  HOSTCFLAGS-x86_64 := -fno-PIE
>  $(call cc-option-add,HOSTCFLAGS-x86_64,HOSTCC,-no-pie)
> +$(call cc-option-add,HOSTCFLAGS-x86_64,HOSTCC,-no-integrated-as)
... as reference in a comment here.

Jan


^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 3/4] tools/tests/x86_emulator: fix build on clang-21
  2026-02-23 16:02   ` Jan Beulich
@ 2026-02-23 16:06     ` Andrew Cooper
  2026-02-23 16:21       ` Jan Beulich
  0 siblings, 1 reply; 19+ messages in thread
From: Andrew Cooper @ 2026-02-23 16:06 UTC (permalink / raw)
  To: Jan Beulich, Edwin Török
  Cc: Andrew Cooper, Roger Pau Monné, Anthony PERARD, xen-devel

On 23/02/2026 4:02 pm, Jan Beulich wrote:
> On 23.02.2026 11:04, Edwin Török wrote:
>> clang-21's built-in assembler is enabled by default, but it doesn't
>> support some mnemonics:
>> ```
>> test_x86_emulator.c:2636:36: error: invalid instruction mnemonic 'fsaves'
>>  2636 |                        "fidivs %1\n\t"
>> test_x86_emulator.c:2640:24: error: invalid instruction mnemonic 'frstors'
>>  2640 |         asm volatile ( "frstors %0" :: "m" (res[25]) : "memory" );
>>       |                        ^
>> test_x86_emulator.c:4251:24: error: invalid instruction mnemonic 'vpcmpestriq'
>>  4251 |                        put_insn(vpcmpestri,
>>       |                        ^
>> ```
> Was this reported to them as a bug (or perhaps even two)?

Looking at just one of these, the mnemonic is FRSTOR without an S, and
Clang 21 is happy with that.

What is the trailing S supposed to signify to GAS?

~Andrew


^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 4/4] tools/tests/x86_emulator: disable xmm* tests on clang
  2026-02-23 10:04 ` [PATCH 4/4] tools/tests/x86_emulator: disable xmm* tests on clang Edwin Török
@ 2026-02-23 16:10   ` Jan Beulich
  2026-02-24  9:34     ` Edwin Torok
  0 siblings, 1 reply; 19+ messages in thread
From: Jan Beulich @ 2026-02-23 16:10 UTC (permalink / raw)
  To: Edwin Török
  Cc: Andrew Cooper, Roger Pau Monné, Anthony PERARD, xen-devel

On 23.02.2026 11:04, Edwin Török wrote:
> clang-21 doesn't support `-ffixed-xmm0`, so `%xmm0` won't have the
> expected value.
> Disable these tests on clang.

I don't think that's what we want, and not only because of the clutter the
various #ifdef cause. We want to be able to run as many of the tests as
possible, so the first goal should be to find some alternative mechanism to
achieve the same effect. A global register variable comes to mind as a
possible option.

Further, how did you arrive at which tests need suppressing? I don't think
we rely on an "expected value" anywhere. I don't even recall us passing
-ffixed-xmm0 when compiling test_x86_emulate.c. We use that option when
building various of the test blobs, iirc. And the comment ahead of the
first use explains why we use the option there. (Later we also use
-ffixed-ymm<N> and -ffixed-zmm<N>, btw.)

Jan


^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 3/4] tools/tests/x86_emulator: fix build on clang-21
  2026-02-23 16:06     ` Andrew Cooper
@ 2026-02-23 16:21       ` Jan Beulich
  2026-02-23 16:36         ` Andrew Cooper
  0 siblings, 1 reply; 19+ messages in thread
From: Jan Beulich @ 2026-02-23 16:21 UTC (permalink / raw)
  To: Andrew Cooper
  Cc: Roger Pau Monné, Anthony PERARD, xen-devel,
	Edwin Török

On 23.02.2026 17:06, Andrew Cooper wrote:
> On 23/02/2026 4:02 pm, Jan Beulich wrote:
>> On 23.02.2026 11:04, Edwin Török wrote:
>>> clang-21's built-in assembler is enabled by default, but it doesn't
>>> support some mnemonics:
>>> ```
>>> test_x86_emulator.c:2636:36: error: invalid instruction mnemonic 'fsaves'
>>>  2636 |                        "fidivs %1\n\t"
>>> test_x86_emulator.c:2640:24: error: invalid instruction mnemonic 'frstors'
>>>  2640 |         asm volatile ( "frstors %0" :: "m" (res[25]) : "memory" );
>>>       |                        ^
>>> test_x86_emulator.c:4251:24: error: invalid instruction mnemonic 'vpcmpestriq'
>>>  4251 |                        put_insn(vpcmpestri,
>>>       |                        ^
>>> ```
>> Was this reported to them as a bug (or perhaps even two)?
> 
> Looking at just one of these, the mnemonic is FRSTOR without an S, and
> Clang 21 is happy with that.
> 
> What is the trailing S supposed to signify to GAS?

"short", i.e. the want for a 16-bit operand size prefix. Just like in
vpcmpestriq the request is for a REX64 prefix. Suffixes are the way to
go in AT&T syntax when operands alone can't disambiguate operand size.
The less nice alternative are data16 prefixes; not sure if Clang would
support those.

Jan


^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 3/4] tools/tests/x86_emulator: fix build on clang-21
  2026-02-23 16:21       ` Jan Beulich
@ 2026-02-23 16:36         ` Andrew Cooper
  2026-02-23 16:40           ` Jan Beulich
  0 siblings, 1 reply; 19+ messages in thread
From: Andrew Cooper @ 2026-02-23 16:36 UTC (permalink / raw)
  To: Jan Beulich, Andrew Cooper
  Cc: Roger Pau Monné, Anthony PERARD, xen-devel,
	Edwin Török

On 23/02/2026 4:21 pm, Jan Beulich wrote:
> On 23.02.2026 17:06, Andrew Cooper wrote:
>> On 23/02/2026 4:02 pm, Jan Beulich wrote:
>>> On 23.02.2026 11:04, Edwin Török wrote:
>>>> clang-21's built-in assembler is enabled by default, but it doesn't
>>>> support some mnemonics:
>>>> ```
>>>> test_x86_emulator.c:2636:36: error: invalid instruction mnemonic 'fsaves'
>>>>  2636 |                        "fidivs %1\n\t"
>>>> test_x86_emulator.c:2640:24: error: invalid instruction mnemonic 'frstors'
>>>>  2640 |         asm volatile ( "frstors %0" :: "m" (res[25]) : "memory" );
>>>>       |                        ^
>>>> test_x86_emulator.c:4251:24: error: invalid instruction mnemonic 'vpcmpestriq'
>>>>  4251 |                        put_insn(vpcmpestri,
>>>>       |                        ^
>>>> ```
>>> Was this reported to them as a bug (or perhaps even two)?
>> Looking at just one of these, the mnemonic is FRSTOR without an S, and
>> Clang 21 is happy with that.
>>
>> What is the trailing S supposed to signify to GAS?
> "short", i.e. the want for a 16-bit operand size prefix.

But that is normally spelled 'w', not 's' in AT&T syntax.

Not that it matters; Clang doesn't like 'w' either.

>  Just like in
> vpcmpestriq the request is for a REX64 prefix. Suffixes are the way to
> go in AT&T syntax when operands alone can't disambiguate operand size.
> The less nice alternative are data16 prefixes; not sure if Clang would
> support those.

data16 seems to be tolerated.

https://godbolt.org/z/MWGjnfWs7

Interestingly Clang automatically inserts a WAIT, although that's a side
effect of the FSAVE it turns into FNSAVE.  

Compiling to a full binary removes the WAIT and reinstates FSAVE, which
I suspect means it's switching back to no-IAS/binutils behind the scenes.

~Andrew


^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 3/4] tools/tests/x86_emulator: fix build on clang-21
  2026-02-23 16:36         ` Andrew Cooper
@ 2026-02-23 16:40           ` Jan Beulich
  0 siblings, 0 replies; 19+ messages in thread
From: Jan Beulich @ 2026-02-23 16:40 UTC (permalink / raw)
  To: Andrew Cooper
  Cc: Roger Pau Monné, Anthony PERARD, xen-devel,
	Edwin Török, Andrew Cooper

On 23.02.2026 17:36, Andrew Cooper wrote:
> On 23/02/2026 4:21 pm, Jan Beulich wrote:
>> On 23.02.2026 17:06, Andrew Cooper wrote:
>>> On 23/02/2026 4:02 pm, Jan Beulich wrote:
>>>> On 23.02.2026 11:04, Edwin Török wrote:
>>>>> clang-21's built-in assembler is enabled by default, but it doesn't
>>>>> support some mnemonics:
>>>>> ```
>>>>> test_x86_emulator.c:2636:36: error: invalid instruction mnemonic 'fsaves'
>>>>>  2636 |                        "fidivs %1\n\t"
>>>>> test_x86_emulator.c:2640:24: error: invalid instruction mnemonic 'frstors'
>>>>>  2640 |         asm volatile ( "frstors %0" :: "m" (res[25]) : "memory" );
>>>>>       |                        ^
>>>>> test_x86_emulator.c:4251:24: error: invalid instruction mnemonic 'vpcmpestriq'
>>>>>  4251 |                        put_insn(vpcmpestri,
>>>>>       |                        ^
>>>>> ```
>>>> Was this reported to them as a bug (or perhaps even two)?
>>> Looking at just one of these, the mnemonic is FRSTOR without an S, and
>>> Clang 21 is happy with that.
>>>
>>> What is the trailing S supposed to signify to GAS?
>> "short", i.e. the want for a 16-bit operand size prefix.
> 
> But that is normally spelled 'w', not 's' in AT&T syntax.

Floating point mnemonics look to be special (and AT&T syntax is absurd in some
situations anyway).

Jan


^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 2/4] tools/tests/x86_emulator: avoid duplicating loop body
  2026-02-23 15:57   ` Jan Beulich
@ 2026-02-24  9:21     ` Edwin Torok
  0 siblings, 0 replies; 19+ messages in thread
From: Edwin Torok @ 2026-02-24  9:21 UTC (permalink / raw)
  To: Jan Beulich
  Cc: Andrew Cooper, Roger Pau Monne, Anthony PERARD,
	xen-devel@lists.xenproject.org

[-- Attachment #1: Type: text/plain, Size: 3239 bytes --]



On 23 Feb 2026, at 15:57, Jan Beulich <jbeulich@suse.com> wrote:

On 23.02.2026 11:04, Edwin Török wrote:
clang would duplicate the loop body and end up with a double definition
of the symbol:
```
/tmp/test_x86_emulator-0f3576.s:27823: Error: symbol `vmovsh_to_mem' is already defined
/tmp/test_x86_emulator-0f3576.s:27825: Error: symbol `.Lvmovsh_to_mem_end' is already defined
```

This is a valid transformation, that even GCC might do, see
https://gcc.gnu.org/onlinedocs/gcc/Extended-Asm.html#Volatile-1 which
says that `%=` should be used instead. However the C code here also
needs to know the name of the label, so I don't immediately see how to
solve that.

For now mark the loop variable `volatile` to prevent the optimization as a
workaround.
(another way to fix this could be to move the loop body into a function,
but a compiler might inline it, or specialize it, leading to the same problem)

Hmm, moving decl and asm() out of the loop wouldn't really work. One option
would be to extend the long-pending [1] to also cover test_x86_emulator.c.
Another might be to make the upper loop bound not a literal 2, but one
loaded from memory, suitably arranged for the compiler to not be able to
const-propagate the value.

But I wonder how many other transformations the compiler could do, causing
the same issue elsewhere. Maybe we need to overhaul how this instruction
instantiation works. (I further wonder how many such issues are lurking in
the many patches I have out for review, plus the yet further ones I didn't
post yet to not pointlessly increase the pile.)

I thought more about the %= I mentioned above. Maybe instead of using a C extern to refer to the label inside the inline assembly,
 the inline assembly should store the address of that label into a C pointer that is an output parameter of the assembly.

Something like:
decl would do: uint8_t *my_instruction, *my_instruction_end;
put insn would do: asm volatile(“my_instruction%=:” …. “my_instruction_end%=:” ... store into my_instruction the value of my_instruction%= and my_instruction_end the value of my_instruction_end%=)

(The label prefix is not needed, but I assume would be very useful for debugging, so you can find these in gdb/etc.)

Although I’d have to see whether that last part would be possible and what the syntax for it would be, considering that put_insn typically has its own output parameters already.


--- a/tools/tests/x86_emulator/test_x86_emulator.c
+++ b/tools/tests/x86_emulator/test_x86_emulator.c
@@ -5248,7 +5248,7 @@ int main(int argc, char **argv)
        memset(res + 3, ~0, 8);
        regs.eax = (unsigned long)res;
        regs.ecx = ~0;
-        for ( i = 0; i < 2; ++i )
+        for (volatile int i = 0; i < 2; ++i )

Even if we went with this as a workaround, style should please not be screwed
up.

Also, whichever the workaround, I think a comment wants leaving.

And then I don't think it is a good idea to introduce a shadowing induction
variable.


Thanks, I’ll update as suggested if I can’t find another solution.

Best regards,
—Edwin


Jan

[1] https://lists.xen.org/archives/html/xen-devel/2023-04/msg00283.html


[-- Attachment #2: Type: text/html, Size: 23256 bytes --]

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 4/4] tools/tests/x86_emulator: disable xmm* tests on clang
  2026-02-23 16:10   ` Jan Beulich
@ 2026-02-24  9:34     ` Edwin Torok
  2026-02-24  9:48       ` Jan Beulich
  0 siblings, 1 reply; 19+ messages in thread
From: Edwin Torok @ 2026-02-24  9:34 UTC (permalink / raw)
  To: Jan Beulich
  Cc: Andrew Cooper, Roger Pau Monne, Anthony PERARD,
	xen-devel@lists.xenproject.org



> On 23 Feb 2026, at 16:10, Jan Beulich <jbeulich@suse.com> wrote:
> 
> On 23.02.2026 11:04, Edwin Török wrote:
>> clang-21 doesn't support `-ffixed-xmm0`, so `%xmm0` won't have the
>> expected value.
>> Disable these tests on clang.
> 
> I don't think that's what we want, and not only because of the clutter the
> various #ifdef cause. We want to be able to run as many of the tests as
> possible, so the first goal should be to find some alternative mechanism to
> achieve the same effect. A global register variable comes to mind as a
> possible option.
> 
> Further, how did you arrive at which tests need suppressing?

I used gdb to look at the 2 memory areas, and noticed that the XMM region was different between emulated and actual when built with clang.
Then I noticed the build failures due to the lack of fixed-xmm0.
Then I added the ifdefs one by one as I ran the test until the whole test program passed without failure.
I tried adding some ‘pxor xmm0, xmm0’ into the cpu_has_sse2 sections, but that didn’t really work either.

Although I may have been misled by the overlapping region, see below.

> I don't think
> we rely on an "expected value" anywhere. I don't even recall us passing
> -ffixed-xmm0 when compiling test_x86_emulate.c.

Yes, I’m surprised it works with GCC. But maybe only because the emulator overwrites the actual FXSAVE area corresponding to XMM.
XMM0 begins at offset 160, and 0x100 - 0x80 = 128.
AFAICT the actual execution stores its result at [0x80, 0x80+0x200), and the emulator stores its result into [0x100, 0x100+0x200).
So the emulator will overwrite some of the values from the actual run. 

This only works if the end of the FXSAVE area looks like its beginning (i.e. if FCW/FSW/etc. happens to match MM6/etc.)

If I move the regions, such that they are distinct, then this begins to fail with GCC too (perhaps due to the lack of fixed-xmm0, I haven’t tried).

Perhaps a better way to fix this would be to make the 2 regions distinct first, get it to work with GCC and then see what bugs remain on Clang.
I’ll try that approach, and see how far I get.

Best regards,
—Edwin

> We use that option when
> building various of the test blobs, iirc. And the comment ahead of the
> first use explains why we use the option there. (Later we also use
> -ffixed-ymm<N> and -ffixed-zmm<N>, btw.)
> 
> Jan


^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 4/4] tools/tests/x86_emulator: disable xmm* tests on clang
  2026-02-24  9:34     ` Edwin Torok
@ 2026-02-24  9:48       ` Jan Beulich
  2026-02-27 11:02         ` Edwin Torok
  0 siblings, 1 reply; 19+ messages in thread
From: Jan Beulich @ 2026-02-24  9:48 UTC (permalink / raw)
  To: Edwin Torok
  Cc: Andrew Cooper, Roger Pau Monne, Anthony PERARD,
	xen-devel@lists.xenproject.org

On 24.02.2026 10:34, Edwin Torok wrote:
>> On 23 Feb 2026, at 16:10, Jan Beulich <jbeulich@suse.com> wrote:
>> On 23.02.2026 11:04, Edwin Török wrote:
>>> clang-21 doesn't support `-ffixed-xmm0`, so `%xmm0` won't have the
>>> expected value.
>>> Disable these tests on clang.
>>
>> I don't think that's what we want, and not only because of the clutter the
>> various #ifdef cause. We want to be able to run as many of the tests as
>> possible, so the first goal should be to find some alternative mechanism to
>> achieve the same effect. A global register variable comes to mind as a
>> possible option.
>>
>> Further, how did you arrive at which tests need suppressing?
> 
> I used gdb to look at the 2 memory areas, and noticed that the XMM region was different between emulated and actual when built with clang.
> Then I noticed the build failures due to the lack of fixed-xmm0.
> Then I added the ifdefs one by one as I ran the test until the whole test program passed without failure.
> I tried adding some ‘pxor xmm0, xmm0’ into the cpu_has_sse2 sections, but that didn’t really work either.
> 
> Although I may have been misled by the overlapping region, see below.
> 
>> I don't think
>> we rely on an "expected value" anywhere. I don't even recall us passing
>> -ffixed-xmm0 when compiling test_x86_emulate.c.
> 
> Yes, I’m surprised it works with GCC. But maybe only because the emulator overwrites the actual FXSAVE area corresponding to XMM.
> XMM0 begins at offset 160, and 0x100 - 0x80 = 128.
> AFAICT the actual execution stores its result at [0x80, 0x80+0x200), and the emulator stores its result into [0x100, 0x100+0x200).
> So the emulator will overwrite some of the values from the actual run. 
> 
> This only works if the end of the FXSAVE area looks like its beginning (i.e. if FCW/FSW/etc. happens to match MM6/etc.)

Are you possibly overlooking the fact that res[] is an array of unsigned int elements,
i.e. the offsets used in source code all need to be multiplied by 4 to give offsets in
memory?

Jan


^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 1/4] tools/tests/x86_emulator: fix 'shifting a negative signed value is undefined'
  2026-02-23 15:42   ` Jan Beulich
@ 2026-02-24 10:12     ` Edwin Torok
  2026-02-24 10:25       ` Jan Beulich
  0 siblings, 1 reply; 19+ messages in thread
From: Edwin Torok @ 2026-02-24 10:12 UTC (permalink / raw)
  To: Jan Beulich
  Cc: Andrew Cooper, Roger Pau Monne, Anthony PERARD,
	xen-devel@lists.xenproject.org



> On 23 Feb 2026, at 15:42, Jan Beulich <jbeulich@suse.com> wrote:
> 
> On 23.02.2026 11:04, Edwin Török wrote:
>> --- a/tools/tests/x86_emulator/test_x86_emulator.c
>> +++ b/tools/tests/x86_emulator/test_x86_emulator.c
>> @@ -1161,7 +1161,7 @@ int main(int argc, char **argv)
>>     instr[0] = 0x4d; instr[1] = 0x0f; instr[2] = 0xbb; instr[3] = 0x03;
>>     regs.eflags = EFLAGS_ALWAYS_SET;
>>     regs.rip    = (unsigned long)&instr[0];
>> -    regs.r8     = (-1L << 40) + 1;
>> +    regs.r8     = (~0ULL << 40) + 1;
> 
> While -1 vs ~0 doesn't make a big difference, I think we want to stick to
> "register size" here, and hence have only UL as the suffix. Then (happy to
> adjust while committing):
> Acked-by: Jan Beulich <jbeulich@suse.com>
> 

That is what I tried initially, but I got an error that the shift exceeds the width of the type in (0UL << 40).
(I think the failure was with -m32 in the CI, but can’t find it now)

Best regards,
—Edwin

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 1/4] tools/tests/x86_emulator: fix 'shifting a negative signed value is undefined'
  2026-02-24 10:12     ` Edwin Torok
@ 2026-02-24 10:25       ` Jan Beulich
  0 siblings, 0 replies; 19+ messages in thread
From: Jan Beulich @ 2026-02-24 10:25 UTC (permalink / raw)
  To: Edwin Torok
  Cc: Andrew Cooper, Roger Pau Monne, Anthony PERARD,
	xen-devel@lists.xenproject.org

On 24.02.2026 11:12, Edwin Torok wrote:
>> On 23 Feb 2026, at 15:42, Jan Beulich <jbeulich@suse.com> wrote:
>> On 23.02.2026 11:04, Edwin Török wrote:
>>> --- a/tools/tests/x86_emulator/test_x86_emulator.c
>>> +++ b/tools/tests/x86_emulator/test_x86_emulator.c
>>> @@ -1161,7 +1161,7 @@ int main(int argc, char **argv)
>>>     instr[0] = 0x4d; instr[1] = 0x0f; instr[2] = 0xbb; instr[3] = 0x03;
>>>     regs.eflags = EFLAGS_ALWAYS_SET;
>>>     regs.rip    = (unsigned long)&instr[0];
>>> -    regs.r8     = (-1L << 40) + 1;
>>> +    regs.r8     = (~0ULL << 40) + 1;
>>
>> While -1 vs ~0 doesn't make a big difference, I think we want to stick to
>> "register size" here, and hence have only UL as the suffix. Then (happy to
>> adjust while committing):
>> Acked-by: Jan Beulich <jbeulich@suse.com>
> 
> That is what I tried initially, but I got an error that the shift exceeds the width of the type in (0UL << 40).
> (I think the failure was with -m32 in the CI, but can’t find it now)

But this code has "#ifdef __x86_64__" around it.

Jan


^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 4/4] tools/tests/x86_emulator: disable xmm* tests on clang
  2026-02-24  9:48       ` Jan Beulich
@ 2026-02-27 11:02         ` Edwin Torok
  0 siblings, 0 replies; 19+ messages in thread
From: Edwin Torok @ 2026-02-27 11:02 UTC (permalink / raw)
  To: Jan Beulich
  Cc: Andrew Cooper, Roger Pau Monne, Anthony PERARD,
	xen-devel@lists.xenproject.org


> On 24 Feb 2026, at 09:48, Jan Beulich <jbeulich@suse.com> wrote:
> 
> On 24.02.2026 10:34, Edwin Torok wrote:
>>> On 23 Feb 2026, at 16:10, Jan Beulich <jbeulich@suse.com> wrote:
>>> On 23.02.2026 11:04, Edwin Török wrote:
>>>> clang-21 doesn't support `-ffixed-xmm0`, so `%xmm0` won't have the
>>>> expected value.
>>>> Disable these tests on clang.
>>> 
>>> I don't think that's what we want, and not only because of the clutter the
>>> various #ifdef cause. We want to be able to run as many of the tests as
>>> possible, so the first goal should be to find some alternative mechanism to
>>> achieve the same effect. A global register variable comes to mind as a
>>> possible option.
>>> 
>>> Further, how did you arrive at which tests need suppressing?
>> 
>> I used gdb to look at the 2 memory areas, and noticed that the XMM region was different between emulated and actual when built with clang.
>> Then I noticed the build failures due to the lack of fixed-xmm0.
>> Then I added the ifdefs one by one as I ran the test until the whole test program passed without failure.
>> I tried adding some ‘pxor xmm0, xmm0’ into the cpu_has_sse2 sections, but that didn’t really work either.
>> 
>> Although I may have been misled by the overlapping region, see below.
>> 
>>> I don't think
>>> we rely on an "expected value" anywhere. I don't even recall us passing
>>> -ffixed-xmm0 when compiling test_x86_emulate.c.
>> 
>> Yes, I’m surprised it works with GCC. But maybe only because the emulator overwrites the actual FXSAVE area corresponding to XMM.
>> XMM0 begins at offset 160, and 0x100 - 0x80 = 128.
>> AFAICT the actual execution stores its result at [0x80, 0x80+0x200), and the emulator stores its result into [0x100, 0x100+0x200).
>> So the emulator will overwrite some of the values from the actual run. 
>> 
>> This only works if the end of the FXSAVE area looks like its beginning (i.e. if FCW/FSW/etc. happens to match MM6/etc.)
> 
> Are you possibly overlooking the fact that res[] is an array of unsigned int elements,
> i.e. the offsets used in source code all need to be multiplied by 4 to give offsets in
> memory?


Thanks, now the tests start making more sense! 

Although there is still a lot that I don’t understand about how some of the other failing tests work.
For now I’ve dropped this patch from my series though, i.e. lets first get the clang build fixes in,
and see how to fix them to actually run separately.

Best regards,
—Edwin

^ permalink raw reply	[flat|nested] 19+ messages in thread

end of thread, other threads:[~2026-02-27 11:02 UTC | newest]

Thread overview: 19+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-02-23 10:04 [PATCH 0/4] Fix building tools/tests/x86_emulator with clang-21 Edwin Török
2026-02-23 10:04 ` [PATCH 1/4] tools/tests/x86_emulator: fix 'shifting a negative signed value is undefined' Edwin Török
2026-02-23 15:42   ` Jan Beulich
2026-02-24 10:12     ` Edwin Torok
2026-02-24 10:25       ` Jan Beulich
2026-02-23 10:04 ` [PATCH 2/4] tools/tests/x86_emulator: avoid duplicating loop body Edwin Török
2026-02-23 15:57   ` Jan Beulich
2026-02-24  9:21     ` Edwin Torok
2026-02-23 10:04 ` [PATCH 3/4] tools/tests/x86_emulator: fix build on clang-21 Edwin Török
2026-02-23 16:02   ` Jan Beulich
2026-02-23 16:06     ` Andrew Cooper
2026-02-23 16:21       ` Jan Beulich
2026-02-23 16:36         ` Andrew Cooper
2026-02-23 16:40           ` Jan Beulich
2026-02-23 10:04 ` [PATCH 4/4] tools/tests/x86_emulator: disable xmm* tests on clang Edwin Török
2026-02-23 16:10   ` Jan Beulich
2026-02-24  9:34     ` Edwin Torok
2026-02-24  9:48       ` Jan Beulich
2026-02-27 11:02         ` Edwin Torok

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.