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

[Heejin Ahn via Phabricator via llvm-commits]

aheejin added a comment.

Nice!

- Please run clang-format before you submit patches
- When you did what the review suggested, please click 'Done' checkbox, so that the reviewer will know which one was taken care of and which one was not.



================
Comment at: llvm/lib/Target/WebAssembly/WebAssemblyISelLowering.cpp:994
+  if (verifyReturnAddressArgumentIsConstant(Op, DAG))
+    return SDValue();
+
----------------
Does this mean we ignore the whole node if the argument is not a constant? How about signaling a failure?


================
Comment at: llvm/lib/Target/WebAssembly/WebAssemblyRuntimeLibcallSignatures.cpp:468
+    // Return address handling
+    Table[RTLIB::RETURN_ADDRESS] = i32_func_i32;
   }
----------------
How about moving this up, before all undefined ones?


================
Comment at: llvm/lib/Target/WebAssembly/WebAssemblyRuntimeLibcallSignatures.cpp:496
     Map["__truncsfhf2"] = RTLIB::FPROUND_F32_F16;
+    Map["emscripten_return_address"] = RTLIB::RETURN_ADDRESS;
   }
----------------
This looks irrelevant to the comment about floats above. Maybe add a newline and comment for this?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D62210