Re: [PATCH 1/2] x86: optimize XCHG to MOV for same-register forms
Jan Beulich via Valgrind-developers <valgrind-developers-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org>
| Newsgroups | gmane.comp.debugging.valgrind.devel,gmane.comp.gnu.binutils |
|---|---|
| Message-ID | <[email protected]> |
On 02.07.2026 21:07, Matthias Schwarzott wrote: > Am 02.07.26 um 14:00 schrieb Mark Wielaard: >> Hi, >> >> CC vaglgrind-developers >> >> Background: >> - new gas optimization >> x86: optimize XCHG to MOV for same-register forms >> https://sourceware.org/cgit/binutils-gdb/commit/?id=1c3c3e4b3c2ac2eed9abcbce0b9cba1be10ed3f0 >> - binutils list discussion: >> https://inbox.sourceware.org/binutils/1ef40cfc-55eb-4dfa-a2db-e8f2d6d3be03-IBi9RG/[email protected]/T >> - valgrind bug report: >> https://bugs.kde.org/show_bug.cgi?id=522533 >> >> On Thu, 2026-07-02 at 07:45 +0100, Sam James wrote: >>> Jan Beulich <jbeulich-IBi9RG/[email protected]> writes: >>> >>>> On 02.07.2026 08:06, Sam James wrote: >>>>> Jan Beulich <jbeulich-IBi9RG/[email protected]> writes: >>>>> >>>>>> MOV requires far less execution resources. >>>>> >>>>> In case anyone ends up stumbling upon this and wonders why Valgrind >>>>> stopped working, it needs adjusting for this change: >>>>> https://bugs.kde.org/522533. >>>> >>>> So the instruction bytes quoted there: >>>> >>>> IR: unhandled instruction bytes: 0x48 0xC1 0xC7 0x3 0x48 0xC1 0xC7 0xD 0x48 0xC1 >>>> >>>> are really unrelated to the issue? That's the earlier ROLs afaict ... >>>> >>> >>> AFAICT yes! I found this surprising too but it's the only difference >>> in the object. >>> >>>> What is that code doing anyway? >>> >>> Mark explained it to me yesterday as (any errors mine in paraphrasing).. >> >> This comment from VEX/priv/guest_amd64_toIR.c might explain it best: >> >> /* "Special" instructions. >> >> This instruction decoder can decode three special instructions >> which mean nothing natively (are no-ops as far as regs/mem are >> concerned) but have meaning for supporting Valgrind. A special >> instruction is flagged by the 16-byte preamble 48C1C703 48C1C70D >> 48C1C73D 48C1C733 (in the standard interpretation, that means: rolq >> $3, %rdi; rolq $13, %rdi; rolq $61, %rdi; rolq $51, %rdi). >> Following that, one of the following 3 are allowed (standard >> interpretation in parentheses): >> >> 4887DB (xchgq %rbx,%rbx) %RDX = client_request ( %RAX ) >> 4887C9 (xchgq %rcx,%rcx) %RAX = guest_NRADDR >> 4887D2 (xchgq %rdx,%rdx) call-noredir *%RAX >> 4887F6 (xchgq %rdi,%rdi) IR injection >> >> Any other bytes following the 16-byte preamble are illegal and >> constitute a failure in instruction decoding. This all assumes >> that the preamble will never occur except in specific code >> fragments designed for Valgrind to catch. >> >> No prefixes may precede a "Special" instruction. >> */ >> >> Which is "implemented" in valgrind.h (which applications include to >> insert these special instructions into their executable to support >> various valgrind "client requests") as James explained: >> >>> We die in vg_preloaded.c:129 which is: >>> >>> void * VG_NOTIFY_ON_LOAD(ifunc_wrapper) (void) >>> ... >>> /* Call the original indirect function and get it's result */ >>> VALGRIND_GET_ORIG_FN(fn); /* <-- */ >>> CALL_FN_W_v(result, fn); >>> >>> which is VALGRIND_GET_NR_CONTEXT: >>> >>> #define VALGRIND_GET_NR_CONTEXT(_zzq_rlval) \ >>> { volatile OrigFn* _zzq_orig = &(_zzq_rlval); \ >>> volatile unsigned int __addr; \ >>> __asm__ volatile(__SPECIAL_INSTRUCTION_PREAMBLE \ >>> /* %EAX = guest_NRADDR */ \ >>> "xchgl %%ecx,%%ecx" \ >>> : "=a" (__addr) \ >>> : \ >>> : "cc", "memory" \ >>> ); \ >>> _zzq_orig->nraddr = __addr; \ >>> } >>> >>> and the rols are the _S_I_P macro: >>> >>> #define __SPECIAL_INSTRUCTION_PREAMBLE \ >>> "roll $3, %%edi ; roll $13, %%edi\n\t" \ >>> "roll $29, %%edi ; roll $19, %%edi\n\t" >>> >>> but what I don't understand is why it ends up seeing that. I think when >>> it sees _S_I_P, it is supposed to rewrite it (?), but I am not an expert on >>> valgrind's VEX interpreter at all. >> >> It is supposed to sees _S_I_P followed by one of the special xchg >> instructions, but now sees _S_I_P followed by a mov which confuses the >> instruction decoder. >> >>>> And why is -O passed to gas there, when specific >>>> insn selection matters? >>> >>> That's just because it's one of the things I test in some runs. I'll >>> filter it out for Valgrind as I agree it makes no sense there, but >>> another problem happens when Valgrind itself is built without it, but >>> e.g. systemd has -Wa,-O2: >> >> Yeah, we don't control how the object files that include the inlined >> assembly in valgrind.h is compiled. >> >> Also note that valgrind.h is often vendored into other code bases >> because it is meant to be useful standalone. Which means we cannot >> change the special instruction sequence or the inline assembly used to >> generate it. >> >> So we need a solution that prevents this particular xchg to mov >> translation (at least for same register ones) even if the sequence is >> compiled with gas optimizations. > > the file valgrind.h is widespread and cannot be easily modified. > How about accepting both instructions, xchg and mov at this point? May I point out that switching to MOV would set things up for failure again at a later point? I'm certainly having the plan to transform such MOVs (same register, not 32-bit in 64-bit mode) to NOP at some point. In fact, while it may be the goal here to use insns which are NOP-like, and such insn is liable to be subject to optimizing to NOP at some point. The four ROLs together (which aren't truly NOP-like, as they zero- extend %edi from 32 to 64 bits) aren't very likely to become subject to such optimization (if, in the first place, they weren't be 32-bit or would be seen outside of 64-bit mode), as imo taking multiple insns into account when trying to optimize is somewhat error prone. Jan