[Codegen][X86] Fix /hotpatch with clang-cl and inline asm

[aganea]

This fixes an edge case where functions starting with inline assembly would assert while trying to lower that inline asm instruction.

After this PR, for now we always add 2 NOPs (xchgw in this case) 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 #56234

[llvmbot]

@llvm/pr-subscribers-backend-x86

Author: Alexandre Ganea (aganea)

Changes

This fixes an edge case where functions starting with inline assembly would assert while trying to lower that inline asm instruction.

After this PR, 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 #56234

---

Full diff: https://github.com/llvm/llvm-project/pull/87639.diff

2 Files Affected:

• (modified) llvm/lib/Target/X86/X86MCInstLower.cpp (+3-1)
• (modified) llvm/test/CodeGen/X86/patchable-prologue.ll (+17)

diff --git a/llvm/lib/Target/X86/X86MCInstLower.cpp b/llvm/lib/Target/X86/X86MCInstLower.cpp
index e2330ff34c1753..e6510be6b9afd0 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 71a392845fdea3..901da41d5b6cac 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

[KanRobert]

LGTM in general

[sylvain-audi]

LGTM too