-
Notifications
You must be signed in to change notification settings - Fork 14.2k
WebAssembly: Stop directly using RuntimeLibcalls.def #143054
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
This stack of pull requests is managed by Graphite. Learn more about stacking. |
@llvm/pr-subscribers-llvm-ir @llvm/pr-subscribers-backend-webassembly Author: Matt Arsenault (arsenm) ChangesConstruct RuntimeLibcallsInfo instead of manually creating a map. This is also not great, since it's setting a static map which Full diff: https://github.com/llvm/llvm-project/pull/143054.diff 1 Files Affected:
diff --git a/llvm/lib/Target/WebAssembly/WebAssemblyRuntimeLibcallSignatures.cpp b/llvm/lib/Target/WebAssembly/WebAssemblyRuntimeLibcallSignatures.cpp
index ce795d3dedc6a..9622b5a54dc62 100644
--- a/llvm/lib/Target/WebAssembly/WebAssemblyRuntimeLibcallSignatures.cpp
+++ b/llvm/lib/Target/WebAssembly/WebAssemblyRuntimeLibcallSignatures.cpp
@@ -528,23 +528,20 @@ RuntimeLibcallSignatureTable &getRuntimeLibcallSignatures() {
// constructor for use with a static variable
struct StaticLibcallNameMap {
StringMap<RTLIB::Libcall> Map;
- StaticLibcallNameMap() {
- static const std::pair<const char *, RTLIB::Libcall> NameLibcalls[] = {
-#define HANDLE_LIBCALL(code, name) {(const char *)name, RTLIB::code},
-#include "llvm/IR/RuntimeLibcalls.def"
-#undef HANDLE_LIBCALL
- };
- for (const auto &NameLibcall : NameLibcalls) {
- if (NameLibcall.first != nullptr &&
- getRuntimeLibcallSignatures().Table[NameLibcall.second] !=
- unsupported) {
- assert(!Map.contains(NameLibcall.first) &&
+ StaticLibcallNameMap(const Triple &TT) {
+ // FIXME: This is broken if there are ever different triples compiled with
+ // different libcalls.
+ RTLIB::RuntimeLibcallsInfo RTCI(TT);
+ for (int I = 0; I < RTLIB::UNKNOWN_LIBCALL; ++I) {
+ RTLIB::Libcall LC = static_cast<RTLIB::Libcall>(I);
+ const char *NameLibcall = RTCI.getLibcallName(LC);
+ if (NameLibcall != nullptr &&
+ getRuntimeLibcallSignatures().Table[LC] != unsupported) {
+ assert(!Map.contains(NameLibcall) &&
"duplicate libcall names in name map");
- Map[NameLibcall.first] = NameLibcall.second;
+ Map[NameLibcall] = LC;
}
}
-
- Map["emscripten_return_address"] = RTLIB::RETURN_ADDRESS;
}
};
@@ -940,7 +937,7 @@ void WebAssembly::getLibcallSignature(const WebAssemblySubtarget &Subtarget,
StringRef Name,
SmallVectorImpl<wasm::ValType> &Rets,
SmallVectorImpl<wasm::ValType> &Params) {
- static StaticLibcallNameMap LibcallNameMap;
+ static StaticLibcallNameMap LibcallNameMap(Subtarget.getTargetTriple());
auto &Map = LibcallNameMap.Map;
auto Val = Map.find(Name);
#ifndef NDEBUG
|
assert(!Map.contains(NameLibcall.first) && | ||
StaticLibcallNameMap(const Triple &TT) { | ||
// FIXME: This is broken if there are ever different triples compiled with | ||
// different libcalls. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we store the map in the subtarget instead?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This belongs in TargetLowering, which is owned by the subtarget. I'll probably end up fixing this in the next step after moving the definitions to tablegen
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The signatures should be a static, generated table
9405d81
to
7a49ea2
Compare
ac49d23
to
9fd32f4
Compare
} | ||
} | ||
|
||
Map["emscripten_return_address"] = RTLIB::RETURN_ADDRESS; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How is this handled in the new version?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
RuntimeLibcallsInfo directly sets this, this was already moved in the parent PR
7a49ea2
to
509a810
Compare
Construct RuntimeLibcallsInfo instead of manually creating a map. This was repeating the setting of the RETURN_ADDRESS. This removes an obstacle to generating libcall information with tablegen. This is also not great, since it's setting a static map which would be broken if there were ever a triple with a different libcall configuration.
509a810
to
71f0bc6
Compare
ping |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks reasonable to me. It seems unlikely that we'd add major new subtarget-specific builtins any time soon.
Construct RuntimeLibcallsInfo instead of manually creating a map.
This was repeating the setting of the RETURN_ADDRESS. This removes
an obstacle to generating libcall information with tablegen.
This is also not great, since it's setting a static map which
would be broken if there were ever a triple with a different libcall
configuration.