[PATCH] D129727: [ARM64EC 11/?] Add support for lowering variadic indirect calls.

[Eli Friedman via Phabricator via llvm-commits]

efriedma added inline comments.


================
Comment at: llvm/lib/Target/AArch64/AArch64ISelLowering.cpp:1467
+    setLibcallName(RTLIB::MEMSET, "#memset");
+    setLibcallName(RTLIB::MEMMOVE, "#memmove");
+  }
----------------
bcl5980 wrote:
> efriedma wrote:
> > bcl5980 wrote:
> > > efriedma wrote:
> > > > Is this actually necessary?  The linker should resolve plain "memcpy" to something reasonable, I think.
> > > Based on my local test, if I don't add `#` the memcpy will be link into x86 version memcpy and crash at runtime.
> > Does the linking process for arm64ec actually guarantee that we have an arm64ec msvcrt that includes "#memcpy" etc.?  Even if it does, I don't think we can make the same assumption for all the other functions SelectionDAG needs to call.
> > 
> > Given that, we're going to need some mechanism to allow calls generated by SelectionDAG to participate in thunking.
> > 
> > In any case, if this change unblocks testing for you, we can leave it in with a FIXME to address the above.
> I try to use dumpbin to dump the symbols for vcruntime.lib. And I find those symbols may related:
> 
> ```
>        40 #memchr
>        41 #memcmp
>        42 #memcpy
>        43 #memmove
>        44 #memset
> ```
> I think we only need to consider the memory intrinsic functions that MSVC also can export.
> We can also add these intrinsic's exit thunk in AArch64TargetLowering::LowerCall.
The full set of stuff SelectionDAG can generate includes is basically stuff from RuntimeLibcalls.def, plus a few target-specific bits.  (Off the top of my head, not sure if there are any target-specific calls on arm64 windows besides "`__chkstk`" and "`__security_check_cookie`".)

If we expect that arm64ec code normally links against an arm64ec C runtime, I guess most of the routines we'd want should be available in "#"-prefixed versions, but I'm not sure about all of them...


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

https://reviews.llvm.org/D129727