[PATCH] D62210: [WebAssembly] Implement __builtin_return_address for emscripten

[Guanzhong Chen via Phabricator via llvm-commits]

quantum marked 4 inline comments as done.
quantum added inline comments.


================
Comment at: llvm/lib/Target/WebAssembly/WebAssemblyISelLowering.cpp:277
+  // Define the emscripten name for return address helper.
+  setLibcallName(RTLIB::RETURN_ADDRESS, "emscripten_return_address");
+
----------------
sbc100 wrote:
> I kind of agree with Dan re: the naming of this.
> 
> If anything we should we at least put "__" at the start so that its clear this is not a normal user function.
> 
> What is this called on other plaforms?  Why not just call it "__builtin_return_address"?
The function already exists in emscripten under that name. In emscripten, these functions don't have the `__` prefix, for example, `emscripten_get_callstack`.

On other platforms, `__builtin_return_address` is lowered to code that reads LR or pulls it out of the stack, instead of a helper function.


================
Comment at: llvm/lib/Target/WebAssembly/WebAssemblyISelLowering.cpp:992
+    return SDValue();
+  }
+
----------------
sbc100 wrote:
> Should we just lower this anyway?  And thereby turn this from compile error into a link error if you happen to have not implemented it?
I don't think so. We aren't sure what this can be lowered to on other WASM targets. Emscripten is the first target to use a library function to compute return addresses, and I would hesitate to extend this to other targets.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D62210