-
Notifications
You must be signed in to change notification settings - Fork 14.8k
RuntimeLibcalls: Return StringRef for libcall names #153209
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
RuntimeLibcalls: Return StringRef for libcall names #153209
Conversation
@llvm/pr-subscribers-lto @llvm/pr-subscribers-tablegen Author: Matt Arsenault (arsenm) ChangesDoes not yet fully propagate this down into the TargetLowering Full diff: https://github.com/llvm/llvm-project/pull/153209.diff 8 Files Affected:
diff --git a/llvm/benchmarks/RuntimeLibcalls.cpp b/llvm/benchmarks/RuntimeLibcalls.cpp
index 47f68abff1e0d..81a5a24ec8f93 100644
--- a/llvm/benchmarks/RuntimeLibcalls.cpp
+++ b/llvm/benchmarks/RuntimeLibcalls.cpp
@@ -22,10 +22,8 @@ static constexpr unsigned MaxFuncNameSize = 53;
static std::vector<StringRef> getLibcallNameStringRefs() {
std::vector<StringRef> Names(RTLIB::NumLibcallImpls);
// Keep the strlens on the StringRef construction out of the benchmark loop.
- for (RTLIB::LibcallImpl LC : RTLIB::libcall_impls()) {
- const char *Name = RTLIB::RuntimeLibcallsInfo::getLibcallImplName(LC);
- Names[LC] = StringRef(Name);
- }
+ for (RTLIB::LibcallImpl LC : RTLIB::libcall_impls())
+ Names[LC] = RTLIB::RuntimeLibcallsInfo::getLibcallImplName(LC);
return Names;
}
diff --git a/llvm/include/llvm/CodeGen/TargetLowering.h b/llvm/include/llvm/CodeGen/TargetLowering.h
index 172b01a649810..5700f93805333 100644
--- a/llvm/include/llvm/CodeGen/TargetLowering.h
+++ b/llvm/include/llvm/CodeGen/TargetLowering.h
@@ -3550,15 +3550,19 @@ class LLVM_ABI TargetLoweringBase {
/// Get the libcall routine name for the specified libcall.
const char *getLibcallName(RTLIB::Libcall Call) const {
- return Libcalls.getLibcallName(Call);
+ // FIXME: Return StringRef
+ return Libcalls.getLibcallName(Call).data();
}
/// Get the libcall routine name for the specified libcall implementation
- const char *getLibcallImplName(RTLIB::LibcallImpl Call) const {
- return Libcalls.getLibcallImplName(Call);
+ static StringRef getLibcallImplName(RTLIB::LibcallImpl Call) {
+ return RTLIB::RuntimeLibcallsInfo::getLibcallImplName(Call);
}
- const char *getMemcpyName() const { return Libcalls.getMemcpyName(); }
+ const char *getMemcpyName() const {
+ // FIXME: Return StringRef
+ return Libcalls.getMemcpyName().data();
+ }
/// Get the comparison predicate that's to be used to test the result of the
/// comparison libcall against zero. This should only be used with
diff --git a/llvm/include/llvm/IR/RuntimeLibcalls.h b/llvm/include/llvm/IR/RuntimeLibcalls.h
index d4505fc4a3043..23948dcfa562c 100644
--- a/llvm/include/llvm/IR/RuntimeLibcalls.h
+++ b/llvm/include/llvm/IR/RuntimeLibcalls.h
@@ -77,17 +77,15 @@ struct RuntimeLibcallsInfo {
/// Get the libcall routine name for the specified libcall.
// FIXME: This should be removed. Only LibcallImpl should have a name.
- const char *getLibcallName(RTLIB::Libcall Call) const {
+ StringRef getLibcallName(RTLIB::Libcall Call) const {
return getLibcallImplName(LibcallImpls[Call]);
}
/// Get the libcall routine name for the specified libcall implementation.
- // FIXME: Change to return StringRef
- static const char *getLibcallImplName(RTLIB::LibcallImpl CallImpl) {
+ static StringRef getLibcallImplName(RTLIB::LibcallImpl CallImpl) {
if (CallImpl == RTLIB::Unsupported)
- return nullptr;
- return RuntimeLibcallImplNameTable[RuntimeLibcallNameOffsetTable[CallImpl]]
- .data();
+ return StringRef();
+ return RuntimeLibcallImplNameTable[RuntimeLibcallNameOffsetTable[CallImpl]];
}
/// Return the lowering's selection of implementation call for \p Call
@@ -119,8 +117,9 @@ struct RuntimeLibcallsInfo {
/// Return a function name compatible with RTLIB::MEMCPY, or nullptr if fully
/// unsupported.
- const char *getMemcpyName() const {
- if (const char *Memcpy = getLibcallName(RTLIB::MEMCPY))
+ StringRef getMemcpyName() const {
+ StringRef Memcpy = getLibcallName(RTLIB::MEMCPY);
+ if (Memcpy != StringRef())
return Memcpy;
// Fallback to memmove if memcpy isn't available.
diff --git a/llvm/lib/CodeGen/PreISelIntrinsicLowering.cpp b/llvm/lib/CodeGen/PreISelIntrinsicLowering.cpp
index 9fa96e7372961..96c9cde622b45 100644
--- a/llvm/lib/CodeGen/PreISelIntrinsicLowering.cpp
+++ b/llvm/lib/CodeGen/PreISelIntrinsicLowering.cpp
@@ -145,7 +145,7 @@ static bool lowerObjCCall(Function &F, RTLIB::LibcallImpl NewFn,
// FIXME: When RuntimeLibcalls is an analysis, check if the function is really
// supported, and go through RTLIB::Libcall.
- const char *NewFnName = RTLIB::RuntimeLibcallsInfo::getLibcallImplName(NewFn);
+ StringRef NewFnName = RTLIB::RuntimeLibcallsInfo::getLibcallImplName(NewFn);
// If we haven't already looked up this function, check to see if the
// program already contains a function with this name.
diff --git a/llvm/lib/LTO/LTO.cpp b/llvm/lib/LTO/LTO.cpp
index 0323b4d433b87..35d24c17bbd93 100644
--- a/llvm/lib/LTO/LTO.cpp
+++ b/llvm/lib/LTO/LTO.cpp
@@ -1422,7 +1422,7 @@ SmallVector<const char *> LTO::getRuntimeLibcallSymbols(const Triple &TT) {
for (RTLIB::LibcallImpl Impl : LibcallImpls) {
if (Impl != RTLIB::Unsupported)
- LibcallSymbols.push_back(Libcalls.getLibcallImplName(Impl));
+ LibcallSymbols.push_back(Libcalls.getLibcallImplName(Impl).data());
}
return LibcallSymbols;
diff --git a/llvm/lib/Target/WebAssembly/WebAssemblyRuntimeLibcallSignatures.cpp b/llvm/lib/Target/WebAssembly/WebAssemblyRuntimeLibcallSignatures.cpp
index 4548a7520b3b4..45b0e7dc12263 100644
--- a/llvm/lib/Target/WebAssembly/WebAssemblyRuntimeLibcallSignatures.cpp
+++ b/llvm/lib/Target/WebAssembly/WebAssemblyRuntimeLibcallSignatures.cpp
@@ -533,8 +533,8 @@ struct StaticLibcallNameMap {
// different libcalls.
RTLIB::RuntimeLibcallsInfo RTCI(TT);
for (RTLIB::Libcall LC : RTLIB::libcalls()) {
- const char *NameLibcall = RTCI.getLibcallName(LC);
- if (NameLibcall != nullptr &&
+ StringRef NameLibcall = RTCI.getLibcallName(LC);
+ if (!NameLibcall.empty() &&
getRuntimeLibcallSignatures().Table[LC] != unsupported) {
assert(!Map.contains(NameLibcall) &&
"duplicate libcall names in name map");
diff --git a/llvm/lib/Transforms/Utils/DeclareRuntimeLibcalls.cpp b/llvm/lib/Transforms/Utils/DeclareRuntimeLibcalls.cpp
index 540039b7d2cbd..0642d51cd2c21 100644
--- a/llvm/lib/Transforms/Utils/DeclareRuntimeLibcalls.cpp
+++ b/llvm/lib/Transforms/Utils/DeclareRuntimeLibcalls.cpp
@@ -30,7 +30,7 @@ PreservedAnalyses DeclareRuntimeLibcallsPass::run(Module &M,
FunctionType *FuncTy =
FunctionType::get(Type::getVoidTy(Ctx), {}, /*IsVarArgs=*/true);
- const char *FuncName = RTLCI.getLibcallImplName(Impl);
+ StringRef FuncName = RTLCI.getLibcallImplName(Impl);
M.getOrInsertFunction(FuncName, FuncTy);
}
diff --git a/llvm/unittests/IR/RuntimeLibcallsTest.cpp b/llvm/unittests/IR/RuntimeLibcallsTest.cpp
index 1ef79e74f9a4b..ce56a071fdc5e 100644
--- a/llvm/unittests/IR/RuntimeLibcallsTest.cpp
+++ b/llvm/unittests/IR/RuntimeLibcallsTest.cpp
@@ -23,7 +23,7 @@ TEST(RuntimeLibcallsTest, LibcallImplByName) {
RTLIB::RuntimeLibcallsInfo::lookupLibcallImplName("unsupported").empty());
for (RTLIB::LibcallImpl LC : RTLIB::libcall_impls()) {
- const char *Name = RTLIB::RuntimeLibcallsInfo::getLibcallImplName(LC);
+ StringRef Name = RTLIB::RuntimeLibcallsInfo::getLibcallImplName(LC);
EXPECT_FALSE(
RTLIB::RuntimeLibcallsInfo::lookupLibcallImplName(Name).empty());
}
|
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.
These are constants as far as I'm aware so it's legal to just take the pointer. We can fix the others later.
abe316d
to
31fb6d4
Compare
b8b7a46
to
fb4e9c5
Compare
31fb6d4
to
b38597a
Compare
a961210 reverted a change to use a binary search on the string name table because it was too slow. This replaces it with a static string hash table based on the known set of libcall names. Microbenchmarking shows this is similarly fast to using DenseMap. It's possibly slightly slower than using StringSet, though these aren't an exact comparison. This also saves on the one time use construction of the map, so it could be better in practice. This search isn't simple set check, since it does find the range of possible matches with the same name. There's also an additional check for whether the current target supports the name. The runtime constructed set doesn't require this, since it only adds the symbols live for the target. Followed algorithm from this post http://0x80.pl/notesen/2023-04-30-lookup-in-strings.html I'm also thinking the 2 special case global symbols should just be added to RuntimeLibcalls. There are also other global references emitted in the backend that aren't tracked; we probably should just use this as a centralized database for all compiler selected symbols.
b38597a
to
60fa7a3
Compare
fb4e9c5
to
8896713
Compare
Does not yet fully propagate this down into the TargetLowering uses, many of which are relying on null checks on the returned value.
60fa7a3
to
93d86a5
Compare
Windows upload timeout |
…#153210)" This reverts commit 9a14b1d. Revert "RuntimeLibcalls: Return StringRef for libcall names (llvm#153209)" This reverts commit cb1228f. Revert "TableGen: Emit statically generated hash table for runtime libcalls (llvm#150192)" This reverts commit 769a905. Reverted three changes because of a CMake error while building llvm-nm as reported in the following PR: llvm#150192 (comment)
#153864) …210)" This reverts commit 9a14b1d. Revert "RuntimeLibcalls: Return StringRef for libcall names (#153209)" This reverts commit cb1228f. Revert "TableGen: Emit statically generated hash table for runtime libcalls (#150192)" This reverts commit 769a905. Reverted three changes because of a CMake error while building llvm-nm as reported in the following PR: #150192 (comment)
Does not yet fully propagate this down into the TargetLowering uses, many of which are relying on null checks on the returned value.
Does not yet fully propagate this down into the TargetLowering
uses, many of which are relying on null checks on the returned
value.