From 455c4e5594c064cd7b519834f5acc84687026e03 Mon Sep 17 00:00:00 2001 From: Alexandre Ganea Date: Thu, 4 Apr 2024 10:08:07 -0400 Subject: [PATCH 1/2] [Codegen][X86] Fix /hotpatch with clang-cl and inline asm This fixes an edge case where functions starting with inline assembly would assert while trying to lower that inline asm instruction. After this commit, for now we always add 2 NOPs without considering the size of the next inline asm instruction. We might want to revisit this in the future. This fixes Unreal Engine 5.3.2 compilation with clang-cl and /HOTPATCH. Should close https://github.com/llvm/llvm-project/issues/56234 --- llvm/lib/Target/X86/X86MCInstLower.cpp | 4 +++- llvm/test/CodeGen/X86/patchable-prologue.ll | 17 +++++++++++++++++ 2 files changed, 20 insertions(+), 1 deletion(-) diff --git a/llvm/lib/Target/X86/X86MCInstLower.cpp b/llvm/lib/Target/X86/X86MCInstLower.cpp index e2330ff34c175..e6510be6b9afd 100644 --- a/llvm/lib/Target/X86/X86MCInstLower.cpp +++ b/llvm/lib/Target/X86/X86MCInstLower.cpp @@ -980,8 +980,10 @@ void X86AsmPrinter::LowerPATCHABLE_OP(const MachineInstr &MI, SmallString<256> Code; unsigned MinSize = MI.getOperand(0).getImm(); - if (NextMI != MI.getParent()->end()) { + if (NextMI != MI.getParent()->end() && !NextMI->isInlineAsm()) { // Lower the next MachineInstr to find its byte size. + // If the next instruction is inline assembly, we skip lowering it for now, + // and assume we should always generate NOPs. MCInst MCI; MCIL.Lower(&*NextMI, MCI); diff --git a/llvm/test/CodeGen/X86/patchable-prologue.ll b/llvm/test/CodeGen/X86/patchable-prologue.ll index 71a392845fdea..901da41d5b6ca 100644 --- a/llvm/test/CodeGen/X86/patchable-prologue.ll +++ b/llvm/test/CodeGen/X86/patchable-prologue.ll @@ -193,3 +193,20 @@ do.body: ; preds = %do.body, %entry do.end: ; preds = %do.body ret void } + + +; Test that inline asm is properly hotpatched. We currently don't examine the +; asm instruction when printing it, thus we always emit patching NOPs. + +; 64: inline_asm: +; 64-NEXT: # %bb.0: +; 64-NEXT: xchgw %ax, %ax # encoding: [0x66,0x90] +; 64-NEXT: #APP +; 64-NEXT: nop # encoding: [0x90] +; 64-NEXT: #NO_APP + +define dso_local void @inline_asm() "patchable-function"="prologue-short-redirect" { +entry: + call void asm sideeffect "nop", "~{dirflag},~{fpsr},~{flags}"() + ret void +} \ No newline at end of file From 29f40cb294bde38aa85c6777ad63ec6b9e8e3871 Mon Sep 17 00:00:00 2001 From: Alexandre Ganea Date: Mon, 8 Apr 2024 09:42:11 -0400 Subject: [PATCH 2/2] Change nop for int3 in test --- llvm/test/CodeGen/X86/patchable-prologue.ll | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/llvm/test/CodeGen/X86/patchable-prologue.ll b/llvm/test/CodeGen/X86/patchable-prologue.ll index 901da41d5b6ca..43761e3d1e1eb 100644 --- a/llvm/test/CodeGen/X86/patchable-prologue.ll +++ b/llvm/test/CodeGen/X86/patchable-prologue.ll @@ -202,11 +202,11 @@ do.end: ; preds = %do.body ; 64-NEXT: # %bb.0: ; 64-NEXT: xchgw %ax, %ax # encoding: [0x66,0x90] ; 64-NEXT: #APP -; 64-NEXT: nop # encoding: [0x90] +; 64-NEXT: int3 # encoding: [0xcc] ; 64-NEXT: #NO_APP define dso_local void @inline_asm() "patchable-function"="prologue-short-redirect" { entry: - call void asm sideeffect "nop", "~{dirflag},~{fpsr},~{flags}"() + call void asm sideeffect "int3", "~{dirflag},~{fpsr},~{flags}"() ret void -} \ No newline at end of file +}