[PATCH] D78785: Fix x86/x86_64 calling convention for _ExtInt

Fri Apr 24 06:59:40 PDT 2020

erichkeane marked an inline comment as done.
erichkeane added inline comments.

================
Comment at: clang/lib/CodeGen/TargetInfo.cpp:2804
+    return;
+  }
+
----------------
Quuxplusone wrote:
> Asking silly questions because I don't stand a chance of understanding either the code or the tests...
> 
> In the x86-64 ABI, an ordinary 16-byte `struct` would be passed in `rdi/rsi` and returned in `rdx/rax`. An ordinary 17-byte `struct` would be passed by hidden reference and returned by hidden reference.
> 
> Is your intent here that `_ExtInt` will follow the same convention, i.e., `_ExtInt(128)` will be passed in `rdi/rsi` and returned in `rdx/rax`? and `_ExtInt(129)` will be passed and returned by hidden reference?
> 
> Clang trunk currently passes `_ExtInt(256)` in `rdi/rsi/rdx/rcx/r8/r9/stack` as if it were a series of `uint64_t`s. (I hadn't noticed until now.) That's kind of cute. Is the current PR motivated by performance concerns with the multi-`uint64_t` approach, or just trying to be consistent with the `struct` rules?
Yes, that is my intent.  The bug you filed apparently showed why putting it in registers is perhaps a bad idea (for something that large), AND gets us more consistent with structs.

As there is no official ABI for any of the 3 platforms, this seems more consistent and gives performance benefit.  @craig.topper can explain this part better than me.

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D78785/new/

https://reviews.llvm.org/D78785