[PATCH] D126809: [ARM64EC 8/?] Implement ARM64EC "thunk" calling conventions

[Eli Friedman via Phabricator via llvm-commits]

efriedma added inline comments.


================
Comment at: llvm/lib/Target/AArch64/AArch64CallingConvention.td:247-252
+  CCIfType<[f16],
+           CCAssignToRegWithShadow<[H0, H1, H2, H3],
+                                   [X0, X1, X2, X2]>>,
+  CCIfType<[f32],
+           CCAssignToRegWithShadow<[S0, S1, S2, S3],
+                                   [X0, X1, X2, X2]>>,
----------------
dpaoliello wrote:
> I'm not sure that this is correct: x64 doesn't have a way to read half- or single-precision floating point numbers, and (as I understand it) AArch64 half/single-precision floating point register writes don't modify the upper-bits. What happens on x64 when you write an f16/f32 to an XMM register? I assume that it gets sign-extended...
AArch64 half/single float register writes do zero the high bits.  x86 is the one that's a bit inconsistent (the behavior of movss depends on whether the source is a register or memory).

In any case, I think the high bits of a floating-point register are undefined in both conventions.


================
Comment at: llvm/lib/Target/AArch64/AArch64CallingConvention.td:263
+  // convention requires "this" to be passed in RCX.
+  // FIXME: Do we need a ARM64EC_Thunk_thiscall convention?
+  // CCIfCC<"CallingConv::X86_ThisCall",
----------------
dpaoliello wrote:
> I'll get some clarification on this...
I copy-pasted the X86_ThisCall thing from the x86 code.  It might be a red herring, though; I just looked at this in clang, and it looks like clang doesn't actually use the X86_ThisCall on x64 Windows, so this doesn't actually do anything, even in the original?  I'm sort of confused how it ended up that way.  Maybe can just delete it, though.

I probably do need some handling for C++ methods, though, which I haven't implemented yet.  (In particular, I need to ensure we emit the arguments for exit thunks for C++ methods in the correct order.)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126809