[X86] Use 64-bit version of source register in LowerPATCHABLE_EVENT_CALL and LowerPATCHABLE_TYPED_EVENT_CALL

[craig.topper]

Summary

The PATCHABLE_EVENT_CALL uses i32 in the intrinsic. This
results in the register allocator picking a 32-bit register. We
need to use the 64-bit register when forming the MOV64rr
instructions. Otherwise we print illegal assembly in the text
output.

I think prior to this it was impossible for SrcReg to be equal
to DstReg so the NOP code was not reachable.

While there use Register instead of unsigned.

Also add a FIXME for what looks like a bug.

[dberris]

LGTM -- thanks!

Do you have any suggestions on how to achieve what the FIXME'ed areas are attempting to do?

[craig.topper]

In D69365#1722730, @dberris wrote:

LGTM -- thanks!

Do you have any suggestions on how to achieve what the FIXME'ed areas are attempting to do?

Could we have an X86 specific pseudo that implicitly uses the registers we want here then emit the necessary copies from virtual regs to those physical registers during instruction selection. Then the register allocator will do all the work? But would make the sled size different.

llvm/lib/Target/X86/X86MCInstLower.cpp

void X86AsmPrinter::LowerPATCHABLE_EVENT_CALL(const MachineInstr &MI,
 
   // Use a two-byte `jmp`. This version of JMP takes an 8-bit relative offset as
   // an operand (computed as an offset from the jmp instruction).
   // FIXME: Find another less hacky way do force the relative jump.
   OutStreamer->EmitBinaryData("\xeb\x0f");
 
   // The default C calling convention will place two arguments into %rcx and
   // %rdx -- so we only work with those.
-  unsigned DestRegs[] = {X86::RDI, X86::RSI};
+  const Register DestRegs[] = {X86::RDI, X86::RSI};
   bool UsedMask[] = {false, false};
   // Filled out in loop.
-  unsigned SrcRegs[] = {0, 0};
+  Register SrcRegs[] = {0, 0};
 
   // Then we put the operands in the %rdi and %rsi registers. We spill the
   // values in the register before we clobber them, and mark them as used in
   // UsedMask. In case the arguments are already in the correct register, we use
   // emit nops appropriately sized to keep the sled the same size in every
   // situation.
   for (unsigned I = 0; I < MI.getNumOperands(); ++I)
     if (auto Op = MCIL.LowerMachineOperand(&MI, MI.getOperand(I))) {
       assert(Op->isReg() && "Only support arguments in registers");
-      SrcRegs[I] = Op->getReg();
+      SrcRegs[I] = getX86SubSuperRegister(Op->getReg(), 64);
       if (SrcRegs[I] != DestRegs[I]) {
         UsedMask[I] = true;
         EmitAndCountInstruction(
             MCInstBuilder(X86::PUSH64r).addReg(DestRegs[I]));
       } else {
         EmitNops(*OutStreamer, 4, Subtarget->is64Bit(), getSubtargetInfo());
       }
     }
 
   // Now that the register values are stashed, mov arguments into place.
+  // FIXME: This doesn't work if one of the later SrcRegs is equal to an
+  // earlier DestReg. We will have already overwritten over the register before
+  // we can copy from it.
   for (unsigned I = 0; I < MI.getNumOperands(); ++I)
     if (SrcRegs[I] != DestRegs[I])
       EmitAndCountInstruction(
           MCInstBuilder(X86::MOV64rr).addReg(DestRegs[I]).addReg(SrcRegs[I]));
 
   // We emit a hard dependency on the __xray_CustomEvent symbol, which is the
   // name of the trampoline to be implemented by the XRay runtime.
   auto TSym = OutContext.getOrCreateSymbol("__xray_CustomEvent");
void X86AsmPrinter::LowerPATCHABLE_TYPED_EVENT_CALL(const MachineInstr &MI,
   // Use a two-byte `jmp`. This version of JMP takes an 8-bit relative offset as
   // an operand (computed as an offset from the jmp instruction).
   // FIXME: Find another less hacky way do force the relative jump.
   OutStreamer->EmitBinaryData("\xeb\x14");
 
   // An x86-64 convention may place three arguments into %rcx, %rdx, and R8,
   // so we'll work with those. Or we may be called via SystemV, in which case
   // we don't have to do any translation.
-  unsigned DestRegs[] = {X86::RDI, X86::RSI, X86::RDX};
+  const Register DestRegs[] = {X86::RDI, X86::RSI, X86::RDX};
   bool UsedMask[] = {false, false, false};
 
   // Will fill out src regs in the loop.
-  unsigned SrcRegs[] = {0, 0, 0};
+  Register SrcRegs[] = {0, 0, 0};
 
   // Then we put the operands in the SystemV registers. We spill the values in
   // the registers before we clobber them, and mark them as used in UsedMask.
   // In case the arguments are already in the correct register, we emit nops
   // appropriately sized to keep the sled the same size in every situation.
   for (unsigned I = 0; I < MI.getNumOperands(); ++I)
     if (auto Op = MCIL.LowerMachineOperand(&MI, MI.getOperand(I))) {
       // TODO: Is register only support adequate?
       assert(Op->isReg() && "Only supports arguments in registers");
-      SrcRegs[I] = Op->getReg();
+      SrcRegs[I] = getX86SubSuperRegister(Op->getReg(), 64);
       if (SrcRegs[I] != DestRegs[I]) {
         UsedMask[I] = true;
         EmitAndCountInstruction(
             MCInstBuilder(X86::PUSH64r).addReg(DestRegs[I]));
       } else {
         EmitNops(*OutStreamer, 4, Subtarget->is64Bit(), getSubtargetInfo());
       }
     }
 
   // In the above loop we only stash all of the destination registers or emit
   // nops if the arguments are already in the right place. Doing the actually
   // moving is postponed until after all the registers are stashed so nothing
   // is clobbers. We've already added nops to account for the size of mov and
   // push if the register is in the right place, so we only have to worry about
   // emitting movs.
+  // FIXME: This doesn't work if one of the later SrcRegs is equal to an
+  // earlier DestReg. We will have already overwritten over the register before
+  // we can copy from it.
   for (unsigned I = 0; I < MI.getNumOperands(); ++I)
     if (UsedMask[I])
       EmitAndCountInstruction(
           MCInstBuilder(X86::MOV64rr).addReg(DestRegs[I]).addReg(SrcRegs[I]));
 
   // We emit a hard dependency on the __xray_TypedEvent symbol, which is the
   // name of the trampoline to be implemented by the XRay runtime.
   auto TSym = OutContext.getOrCreateSymbol("__xray_TypedEvent");

llvm/test/CodeGen/X86/xray-custom-log.ll

 ; RUN: llc -verify-machineinstrs -filetype=asm -o - -mtriple=x86_64-unknown-linux-gnu < %s | FileCheck %s
 ; RUN: llc -verify-machineinstrs -filetype=asm -o - \
 ; RUN:     -mtriple=x86_64-unknown-linux-gnu -relocation-model=pic < %s | FileCheck %s -check-prefix=PIC
 
 define i32 @fn() nounwind noinline uwtable "function-instrument"="xray-always" {
     %eventptr = alloca i8
     %eventsize = alloca i32
     store i32 3, i32* %eventsize
     %val = load i32, i32* %eventsize
     call void @llvm.xray.customevent(i8* %eventptr, i32 %val)
     ; CHECK-LABEL: Lxray_event_sled_0:
     ; CHECK:       .byte 0xeb, 0x0f
     ; CHECK-NEXT:  pushq %rdi
     ; CHECK-NEXT:  pushq %rsi
     ; CHECK-NEXT:  movq %rcx, %rdi
-    ; CHECK-NEXT:  movq %eax, %rsi
+    ; CHECK-NEXT:  movq %rax, %rsi
     ; CHECK-NEXT:  callq __xray_CustomEvent
     ; CHECK-NEXT:  popq %rsi
     ; CHECK-NEXT:  popq %rdi
 
     ; PIC-LABEL: Lxray_event_sled_0:
     ; PIC:       .byte 0xeb, 0x0f
     ; PIC-NEXT:  pushq %rdi
     ; PIC-NEXT:  pushq %rsi
     ; PIC-NEXT:  movq %rcx, %rdi
-    ; PIC-NEXT:  movq %eax, %rsi
+    ; PIC-NEXT:  movq %rax, %rsi
     ; PIC-NEXT:  callq __xray_CustomEvent@PLT
     ; PIC-NEXT:  popq %rsi
     ; PIC-NEXT:  popq %rdi
     ret i32 0
 }
 ; CHECK-LABEL: xray_instr_map
 ; CHECK-LABEL: Lxray_sleds_start0:
 ; CHECK:       .quad {{.*}}xray_event_sled_0
 
 declare void @llvm.xray.customevent(i8*, i32)

llvm/test/CodeGen/X86/xray-typed-event-log.ll

define i32 @fn() nounwind noinline uwtable "function-instrument"="xray-always" {
     store i32 3, i32* %eventsize
     %val = load i32, i32* %eventsize
     call void @llvm.xray.typedevent(i16 %type, i8* %eventptr, i32 %val)
     ; CHECK-LABEL: Lxray_typed_event_sled_0:
     ; CHECK:       .byte 0xeb, 0x14
     ; CHECK-NEXT:  pushq %rdi
     ; CHECK-NEXT:  pushq %rsi
     ; CHECK-NEXT:  pushq %rdx
-    ; CHECK-NEXT:  movq %dx, %rdi
+    ; CHECK-NEXT:  movq %rdx, %rdi
     ; CHECK-NEXT:  movq %rcx, %rsi
-    ; CHECK-NEXT:  movq %eax, %rdx
+    ; CHECK-NEXT:  movq %rax, %rdx
     ; CHECK-NEXT:  callq __xray_TypedEvent
     ; CHECK-NEXT:  popq %rdx
     ; CHECK-NEXT:  popq %rsi
     ; CHECK-NEXT:  popq %rdi
 
     ; PIC-LABEL: Lxray_typed_event_sled_0:
     ; PIC:       .byte 0xeb, 0x14
     ; PIC-NEXT:  pushq %rdi
     ; PIC-NEXT:  pushq %rsi
     ; PIC-NEXT:  pushq %rdx
-    ; PIC-NEXT:  movq %dx, %rdi
+    ; PIC-NEXT:  movq %rdx, %rdi
     ; PIC-NEXT:  movq %rcx, %rsi
-    ; PIC-NEXT:  movq %eax, %rdx
+    ; PIC-NEXT:  movq %rax, %rdx
     ; PIC-NEXT:  callq __xray_TypedEvent@PLT
     ; PIC-NEXT:  popq %rdx
     ; PIC-NEXT:  popq %rsi
     ; PIC-NEXT:  popq %rdi
     ret i32 0
 }
 ; CHECK-LABEL: xray_instr_map
 ; CHECK-LABEL: Lxray_sleds_start0:
 ; CHECK:       .quad {{.*}}xray_typed_event_sled_0
 
 declare void @llvm.xray.typedevent(i16, i8*, i32)