Skip to content

Commit

Permalink
[Codegen][X86] Fix /HOTPATCH with clang-cl and inline asm (#87639)
Browse files Browse the repository at this point in the history
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 a no-op (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

(cherry picked from commit ec1af63)
  • Loading branch information
aganea authored and tstellar committed Apr 15, 2024
1 parent d0ddcce commit c837970
Show file tree
Hide file tree
Showing 2 changed files with 20 additions and 1 deletion.
4 changes: 3 additions & 1 deletion llvm/lib/Target/X86/X86MCInstLower.cpp
Expand Up @@ -959,8 +959,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);

Expand Down
17 changes: 17 additions & 0 deletions llvm/test/CodeGen/X86/patchable-prologue.ll
Expand Up @@ -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: int3 # encoding: [0xcc]
; 64-NEXT: #NO_APP

define dso_local void @inline_asm() "patchable-function"="prologue-short-redirect" {
entry:
call void asm sideeffect "int3", "~{dirflag},~{fpsr},~{flags}"()
ret void
}

0 comments on commit c837970

Please sign in to comment.