[PATCH] D149848: [AArch64, compiler-rt] Implement trampoline intrinsics

[Lou via Phabricator via llvm-commits]

iamlouk added inline comments.


================
Comment at: compiler-rt/lib/builtins/trampoline_setup.c:45
+
+#if defined(__aarch64__) && !defined(__ANDROID__) && !defined(__APPLE__)
+static __inline void aarch64_gen_constant(uint32_t *buf, uint64_t constant,
----------------
compnerd wrote:
> Does this apply to Windows?  I think that the condition here is incorrect.
The x86 trampoline intrinsic (which inlines this logic) does work on Windows, but sadly I was not able to find out if Windows on ARM still allowes an executable stack via the online documentation I looked through, nor do I have the possibility to test it myself. I will search some more, but if I do not find a clear answer, I will change this condition here and the one where the call is emitted to exclude Windows in the next differential.


================
Comment at: llvm/lib/Target/AArch64/AArch64ISelLowering.cpp:5772
+    report_fatal_error("trampoline intrinsics are "
+                       "not supported on this platform.");
+
----------------
compnerd wrote:
> This will emit this on Windows as well, is this supposed to work there?
Same as above where the trampoline setup function is implemented: I will change this to exclude windows in the next differential I upload.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D149848