WebAssembly: Move runtime libcall setting out of TargetLowering

[arsenm]

RuntimeLibcallInfo needs to be correct outside of codegen contexts.

[arsenm]

• WebAssembly: Stop directly using RuntimeLibcalls.def #143054
• WebAssembly: Move runtime libcall setting out of TargetLowering #142624 👈 (View in Graphite)
• main

This stack of pull requests is managed by Graphite. Learn more about stacking.

[llvmbot]

@llvm/pr-subscribers-lld-wasm
@llvm/pr-subscribers-llvm-ir

@llvm/pr-subscribers-backend-webassembly

Author: Matt Arsenault (arsenm)

Changes

RuntimeLibcallInfo needs to be correct outside of codegen contexts.

---

Full diff: https://github.com/llvm/llvm-project/pull/142624.diff

2 Files Affected:

• (modified) llvm/lib/IR/RuntimeLibcalls.cpp (+5)
• (modified) llvm/lib/Target/WebAssembly/WebAssemblyISelLowering.cpp (-5)

diff --git a/llvm/lib/IR/RuntimeLibcalls.cpp b/llvm/lib/IR/RuntimeLibcalls.cpp
index db0373055160c..ec4dc1858e4a5 100644
--- a/llvm/lib/IR/RuntimeLibcalls.cpp
+++ b/llvm/lib/IR/RuntimeLibcalls.cpp
@@ -277,5 +277,10 @@ void RuntimeLibcallsInfo::initLibcalls(const Triple &TT) {
       setLibcallName(RTLIB::MULO_I64, nullptr);
     }
     setLibcallName(RTLIB::MULO_I128, nullptr);
+  } else {
+    // Define the emscripten name for return address helper.
+    // TODO: when implementing other Wasm backends, make this generic or only do
+    // this on emscripten depending on what they end up doing.
+    setLibcallName(RTLIB::RETURN_ADDRESS, "emscripten_return_address");
   }
 }
diff --git a/llvm/lib/Target/WebAssembly/WebAssemblyISelLowering.cpp b/llvm/lib/Target/WebAssembly/WebAssemblyISelLowering.cpp
index aac3473311192..3cd923c0ba058 100644
--- a/llvm/lib/Target/WebAssembly/WebAssemblyISelLowering.cpp
+++ b/llvm/lib/Target/WebAssembly/WebAssemblyISelLowering.cpp
@@ -385,11 +385,6 @@ WebAssemblyTargetLowering::WebAssemblyTargetLowering(
 
   setMaxAtomicSizeInBitsSupported(64);
 
-  // Define the emscripten name for return address helper.
-  // TODO: when implementing other Wasm backends, make this generic or only do
-  // this on emscripten depending on what they end up doing.
-  setLibcallName(RTLIB::RETURN_ADDRESS, "emscripten_return_address");
-
   // Always convert switches to br_tables unless there is only one case, which
   // is equivalent to a simple branch. This reduces code size for wasm, and we
   // defer possible jump table optimizations to the VM.

[tlively]

This particular change LGTM, but @nikic, can you explain where the !TT.isWasm above came from? It looks like you added it in dad9e4a.

[nikic]

IIRC to preserve existing behavior and because these libcalls were listed in RuntimeLibcallSignatureTable.

I don't think wasm is typically used with libgcc? Is that supported?

[dschuff]

AFAIK there are no uses of libgcc with wasm.

[sbc100]

RuntimeLibcallInfo needs to be correct outside of codegen contexts.

Can you explain why?

I see other calls to setLibcallName that are happening in ISelLowering.cpp file:

e.g.

llvm/lib/Target/X86/X86ISelLowering.cpp:    setLibcallName(RTLIB::UNWIND_RESUME, "_Unwind_SjLj_Resume");

Are all of these wrong too?

[sbc100]

Should this be if (TT.isOSEmscripten()) ?

[dschuff]

I suspect this predates when we had an emscripten triple, and it should probably be behind isOSEmscripten()

[arsenm]

Not looking to change behavior here, this should be done separately

[arsenm]

RuntimeLibcallInfo needs to be correct outside of codegen contexts.

Can you explain why?

If you're doing LTO of with libc or compiler-rt included in the build, we need to maintain an accurate list of functions that may be required during later codegen

Are all of these wrong too?

Yes

[arsenm]

lld/test/wasm/lto/Inputs/libcall-return-addr.ll is failing. This was last moved in #126880, but this is now fixed and there should be no more cases in webassembly. I guess I could port the test to a different target that still has subtarget dependent libcall choices

[arsenm]

lld/test/wasm/lto/Inputs/libcall-return-addr.ll is failing. This was last moved in #126880, but this is now fixed and there should be no more cases in webassembly. I guess I could port the test to a different target that still has subtarget dependent libcall choices

I've tried to replicate the error on other targets but can't. The original patch seems wasm specific, and should not be reproducible with any libcalls anymore so I'll just delete the test

[arsenm]

Merge activity

• Jun 16, 1:44 AM UTC: A user started a stack merge that includes this pull request via Graphite.
• Jun 16, 1:46 AM UTC: @arsenm merged this pull request with Graphite.