[PATCH] D24359: Win64: Don't use REX prefix for direct tail calls

[Hans Wennborg via llvm-commits]

hans added a comment.

In https://reviews.llvm.org/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 https://reviews.llvm.org/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?


https://reviews.llvm.org/D24359