Win64: Don't use REX prefix for direct tail calls

[hans]

Summary

The REX prefix should be used on indirect jmps, but not direct ones.
For direct jumps, the unwinder looks at the offset to determine if
it's inside the current function.

[majnemer]

It seems that not all indirect jumps should be REX prefixed: https://github.com/dotnet/coreclr/blob/master/src/unwinder/amd64/unwinder_amd64.cpp#L1378

Does TAILJMPm64_REX emit something that would match https://github.com/dotnet/coreclr/blob/master/src/unwinder/amd64/unwinder_amd64.cpp#L1389 ?

[majnemer]

Hmm, looks like we are over REXing there too:

void __declspec(dllimport) h();
void f() {
  h();
}

turns into:

f:                                      # @f
.Ltmp0:
.seh_proc f
# BB#0:                                 # %entry
.Ltmp1:
	.seh_endprologue
	rex64 jmpq	*__imp_h(%rip)  # TAILCALL
                                        # encoding: [0x48,0xff,0x25,A,A,A,A]
                                        #   fixup A - offset: 3, value: __imp_h-4, kind: reloc_riprel_4byte
	.seh_handlerdata
	.text
.Ltmp2:
	.seh_endproc

My reading indicates that the unwinder will consider this as a tail call. If anything, our REX prefix is probably confusing it.

I imagine that MSVC omits it here?

[hans]

In D24359#537575, @majnemer wrote:

Hmm, looks like we are over REXing there too:

void __declspec(dllimport) h();
void f() {
  h();
}

turns into:

f:                                      # @f
.Ltmp0:
.seh_proc f
# BB#0:                                 # %entry
.Ltmp1:
	.seh_endprologue
	rex64 jmpq	*__imp_h(%rip)  # TAILCALL
                                        # encoding: [0x48,0xff,0x25,A,A,A,A]
                                        #   fixup A - offset: 3, value: __imp_h-4, kind: reloc_riprel_4byte
	.seh_handlerdata
	.text
.Ltmp2:
	.seh_endproc

My reading indicates that the unwinder will consider this as a tail call. If anything, our REX prefix is probably confusing it.

I imagine that MSVC omits it here?

No, they emit it:

d:\src\tmp>type a.cc
void __declspec(dllimport) h();
void f() {
  h();
}

d:\src\tmp>cl /c /Ox a.cc && dumpbin /disasm a.obj
Microsoft (R) C/C++ Optimizing Compiler Version 18.00.31101 for x64
Copyright (C) Microsoft Corporation.  All rights reserved.

a.cc
Microsoft (R) COFF/PE Dumper Version 12.00.31101.0
Copyright (C) Microsoft Corporation.  All rights reserved.


Dump of file a.obj

File Type: COFF OBJECT

?f@@YAXXZ (void __cdecl f(void)):
  0000000000000000: 48 FF 25 00 00 00  jmp         qword ptr [__imp_?h@@YAXXZ]
                    00

  Summary

          64 .debug$S
          2F .drectve
           7 .text$mn

In D24359#537543, @majnemer wrote:

It seems that not all indirect jumps should be REX prefixed: https://github.com/dotnet/coreclr/blob/master/src/unwinder/amd64/unwinder_amd64.cpp#L1378

Hmm, it seems for this specific one they can handle both with and without REX prefix.

Does TAILJMPm64_REX emit something that would match https://github.com/dotnet/coreclr/blob/master/src/unwinder/amd64/unwinder_amd64.cpp#L1389 ?

Yes, I believe TAILJMPm64_REX is what we use in your dllimport example, and it seems that matches both if-statements here.

Perhaps the unwinder used to be stricter but then they had to change it to accept more non-rexed versions?

[majnemer]

LGTM

llvm/trunk/lib/Target/X86/X86ExpandPseudo.cpp

if (Opcode == X86::TCRETURNdi || Opcode == X86::TCRETURNdicc ||
       switch (Opcode) {
       case X86::TCRETURNdi:
         Op = X86::TAILJMPd;
         break;
       case X86::TCRETURNdicc:
         Op = X86::TAILJMPd_CC;
         break;
       default:
-        Op = IsWin64 ? X86::TAILJMPd64_REX : X86::TAILJMPd64;
+        // Note: Win64 uses REX prefixes indirect jumps out of functions, but
+        // not direct ones.
+        Op = X86::TAILJMPd64;
         break;
       }
       MachineInstrBuilder MIB = BuildMI(MBB, MBBI, DL, TII->get(Op));
       if (JumpTarget.isGlobal()) {
         MIB.addGlobalAddress(JumpTarget.getGlobal(), JumpTarget.getOffset(),
                              JumpTarget.getTargetFlags());
       } else {
         assert(JumpTarget.isSymbol());

llvm/trunk/lib/Target/X86/X86ISelLowering.cpp

 MachineBasicBlock *
 X86TargetLowering::EmitInstrWithCustomInserter(MachineInstr &MI,
                                                MachineBasicBlock *BB) const {
   switch (MI.getOpcode()) {
   default: llvm_unreachable("Unexpected instr type to insert");
   case X86::TAILJMPd64:
   case X86::TAILJMPr64:
   case X86::TAILJMPm64:
-  case X86::TAILJMPd64_REX:
   case X86::TAILJMPr64_REX:
   case X86::TAILJMPm64_REX:
     llvm_unreachable("TAILJMP64 would not be touched here.");
   case X86::TCRETURNdi64:
   case X86::TCRETURNri64:
   case X86::TCRETURNmi64:
     return BB;
   case X86::TLS_addr32:

llvm/trunk/lib/Target/X86/X86InstrControl.td

def TAILJMPd64 : Ii32PCRel<0xE9, RawFrm, (outs), (ins i64i32imm_pcrel:$dst),
                    "jmp\t$dst", [], IIC_JMP_REL>;
   def TAILJMPr64 : I<0xFF, MRM4r, (outs), (ins ptr_rc_tailcall:$dst),
                      "jmp{q}\t{*}$dst", [], IIC_JMP_MEM>;
 
   let mayLoad = 1 in
   def TAILJMPm64 : I<0xFF, MRM4m, (outs), (ins i64mem_TC:$dst),
                      "jmp{q}\t{*}$dst", [], IIC_JMP_MEM>;
 
-  // Win64 wants jumps leaving the function to have a REX_W prefix.
+  // Win64 wants indirect jumps leaving the function to have a REX_W prefix.
   let hasREX_WPrefix = 1 in {
-    def TAILJMPd64_REX : Ii32PCRel<0xE9, RawFrm, (outs),
-                                   (ins i64i32imm_pcrel:$dst),
-                                   "rex64 jmp\t$dst", [], IIC_JMP_REL>;
     def TAILJMPr64_REX : I<0xFF, MRM4r, (outs), (ins ptr_rc_tailcall:$dst),
                            "rex64 jmp{q}\t{*}$dst", [], IIC_JMP_MEM>;
 
     let mayLoad = 1 in
     def TAILJMPm64_REX : I<0xFF, MRM4m, (outs), (ins i64mem_TC:$dst),
                            "rex64 jmp{q}\t{*}$dst", [], IIC_JMP_MEM>;
   }
 }

llvm/trunk/lib/Target/X86/X86InstrInfo.cpp

switch (Inst.getOpcode()) {
     case X86::TCRETURNmi64:
     case X86::TCRETURNri64:
     case X86::TAILJMPd:
     case X86::TAILJMPm:
     case X86::TAILJMPr:
     case X86::TAILJMPd64:
     case X86::TAILJMPm64:
     case X86::TAILJMPr64:
-    case X86::TAILJMPd64_REX:
     case X86::TAILJMPm64_REX:
     case X86::TAILJMPr64_REX:
       return true;
     default:
       return false;
   }
 }
 

llvm/trunk/lib/Target/X86/X86MCInstLower.cpp

void X86AsmPrinter::EmitInstruction(const MachineInstr *MI) {
   case X86::TAILJMPm:
   case X86::TAILJMPd:
   case X86::TAILJMPd_CC:
   case X86::TAILJMPr64:
   case X86::TAILJMPm64:
   case X86::TAILJMPd64:
   case X86::TAILJMPr64_REX:
   case X86::TAILJMPm64_REX:
-  case X86::TAILJMPd64_REX:
     // Lower these as normal, but add some comments.
     OutStreamer->AddComment("TAILCALL");
     break;
 
   case X86::TLS_addr32:
   case X86::TLS_addr64:
   case X86::TLS_base_addr32:
   case X86::TLS_base_addr64:

llvm/trunk/test/CodeGen/X86/seh-catchpad.ll

 entry:
   %call = tail call i32 @filt()
   ret i32 %call
 }
 
 ; CHECK: "?filt$0@0@main@@":                     # @"\01?filt$0@0@main@@"
 ; CHECK: .seh_proc "?filt$0@0@main@@"
 ; CHECK:         .seh_endprologue
-; CHECK:         rex64 jmp       filt  # TAILCALL
+; CHECK:         jmp       filt  # TAILCALL
 ; CHECK:         .seh_handlerdata
 
 declare i32 @filt() #1
 
 declare i32 @__C_specific_handler(...)
 
 ; Function Attrs: noinline nounwind
 define internal fastcc void @"\01?fin$0@0@main@@"() #2 {

llvm/trunk/test/CodeGen/X86/tail-call-win64.ll

 declare void @tail_tgt()
 
 define void @tail_jmp_imm() {
   tail call void @tail_tgt()
   ret void
 }
 
 ; CHECK-LABEL: tail_jmp_imm:
-; CHECK: rex64 jmp tail_tgt
+; CHECK: jmp tail_tgt
 
 @g_fptr = global void ()* @tail_tgt
 
 define void @tail_jmp_mem() {
   %fptr = load void ()*, void ()** @g_fptr
   tail call void () %fptr()
   ret void
 }
 
 ; CHECK-LABEL: tail_jmp_mem:
 ; CHECK: rex64 jmpq *g_fptr(%rip)

llvm/trunk/test/CodeGen/X86/win64_sibcall.ll

; LINUX:  movq  $0, -8(%rsp)
   store %Object addrspace(1)* %param0, %Object addrspace(1)** %this
   br label %0
 
 ; <label>:0                                       ; preds = %entry
   %1 = load %Object addrspace(1)*, %Object addrspace(1)** %this, align 8
 
 ; WIN_X64:  xorl  %r8d, %r8d
 ; WIN_X64:  popq  %rax
-; WIN_X64:  rex64 jmp C2              # TAILCALL
+; WIN_X64:  jmp   C2                  # TAILCALL
 ; LINUX:  xorl  %edx, %edx
 ; LINUX:  jmp C2                      # TAILCALL
 
   tail call void @C2(%Object addrspace(1)* %1, i32 0, %Object addrspace(1)* null)
   ret void
 }
 
 declare void @C2(%Object addrspace(1)*, i32, %Object addrspace(1)*)
 
 ; Function Attrs: nounwind
 declare void @llvm.localescape(...) #0
 
 attributes #0 = { nounwind }
 

llvm/trunk/test/DebugInfo/COFF/register-variables.ll

 ; ASM:         incl    x(%rip)
 ; ASM: [[after_if:\.Ltmp.*]]:
 ; ASM: .LBB0_2:                                # %if.else
 ; ASM:         #DEBUG_VALUE: f:p <- %ESI
 ; ASM:         movl    %eax, %ecx
 ; ASM:         addq    $32, %rsp
 ; ASM:         popq    %rsi
 ; ASM: [[func_end:\.Ltmp.*]]:
-; ASM:         rex64 jmp       putint          # TAILCALL
+; ASM:         jmp     putint                  # TAILCALL
 
 ; ASM:         .short  4414                    # Record kind: S_LOCAL
 ; ASM:         .asciz  "p"
 ; ASM:         .cv_def_range    .Lfunc_begin0 [[p_ecx_esi]], "A\021\022\000\000\000"
 ; ASM:         .cv_def_range    [[p_ecx_esi]] [[func_end]], "A\021\027\000\000\000"
 ; ASM:         .short  4414                    # Record kind: S_LOCAL
 ; ASM:         .asciz  "a"
 ; ASM:         .cv_def_range    [[after_getint]] [[after_inc_eax]], "A\021\021\000\000\000"