RuntimeLibcalls: Generate table of libcall name lengths

[arsenm]

Avoids strlen when constructing the returned StringRef. We were emitting
these in the libcall name lookup anyway, so split out the offsets for
general use.

Currently emitted as a separate table, not sure if it would be better
to change the string offset table to store pairs of offset and width
instead.

[arsenm]

• RuntimeLibcalls: Fix building hash table with duplicate entries #153801
• RuntimeLibcalls: Generate table of libcall name lengths #153210 👈 (View in Graphite)
• RuntimeLibcalls: Return StringRef for libcall names #153209
• TableGen: Emit static hash table for runtime libcalls #150192
• IRSymtab: Use StringSet instead of DenseMap for preserved symbols #149836
• main

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

[llvmbot]

@llvm/pr-subscribers-llvm-ir
@llvm/pr-subscribers-tablegen

@llvm/pr-subscribers-llvm-binary-utilities

Author: Matt Arsenault (arsenm)

Changes

Avoids strlen when constructing the returned StringRef. We were emitting
these in the libcall name lookup anyway, so split out the offsets for
general use.

Currently emitted as a separate table, not sure if it would be better
to change the string offset table to store pairs of offset and width
instead.

---

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

3 Files Affected:

• (modified) llvm/include/llvm/IR/RuntimeLibcalls.h (+4-1)
• (modified) llvm/test/TableGen/RuntimeLibcallEmitter.td (+17-5)
• (modified) llvm/utils/TableGen/Basic/RuntimeLibcallsEmitter.cpp (+14-8)

diff --git a/llvm/include/llvm/IR/RuntimeLibcalls.h b/llvm/include/llvm/IR/RuntimeLibcalls.h
index 23948dcfa562c..7f7291f0369c5 100644
--- a/llvm/include/llvm/IR/RuntimeLibcalls.h
+++ b/llvm/include/llvm/IR/RuntimeLibcalls.h
@@ -85,7 +85,9 @@ struct RuntimeLibcallsInfo {
   static StringRef getLibcallImplName(RTLIB::LibcallImpl CallImpl) {
     if (CallImpl == RTLIB::Unsupported)
       return StringRef();
-    return RuntimeLibcallImplNameTable[RuntimeLibcallNameOffsetTable[CallImpl]];
+    return StringRef(RuntimeLibcallImplNameTable.getCString(
+                         RuntimeLibcallNameOffsetTable[CallImpl]),
+                     RuntimeLibcallNameSizeTable[CallImpl]);
   }
 
   /// Return the lowering's selection of implementation call for \p Call
@@ -182,6 +184,7 @@ struct RuntimeLibcallsInfo {
   LLVM_ABI static const char RuntimeLibcallImplNameTableStorage[];
   LLVM_ABI static const StringTable RuntimeLibcallImplNameTable;
   LLVM_ABI static const uint16_t RuntimeLibcallNameOffsetTable[];
+  LLVM_ABI static const uint8_t RuntimeLibcallNameSizeTable[];
 
   /// Map from a concrete LibcallImpl implementation to its RTLIB::Libcall kind.
   LLVM_ABI static const RTLIB::Libcall ImplToLibcall[RTLIB::NumLibcallImpls];
diff --git a/llvm/test/TableGen/RuntimeLibcallEmitter.td b/llvm/test/TableGen/RuntimeLibcallEmitter.td
index 4128219ae2813..41d219652af7c 100644
--- a/llvm/test/TableGen/RuntimeLibcallEmitter.td
+++ b/llvm/test/TableGen/RuntimeLibcallEmitter.td
@@ -137,6 +137,18 @@ def BlahLibrary : SystemRuntimeLibrary<isBlahArch, (add calloc, LibraryWithCondi
 // CHECK-NEXT:   54, // sqrtl
 // CHECK-NEXT:   54, // sqrtl
 // CHECK-NEXT: };
+// CHECK-EMPTY:
+// CHECK-NEXT: const uint8_t RTLIB::RuntimeLibcallsInfo::RuntimeLibcallNameSizeTable[] = {
+// CHECK-NEXT:   9,
+// CHECK-NEXT:   9,
+// CHECK-NEXT:   9,
+// CHECK-NEXT:   9,
+// CHECK-NEXT:   5,
+// CHECK-NEXT:   6,
+// CHECK-NEXT:   5,
+// CHECK-NEXT:   5,
+// CHECK-NEXT: };
+// CHECK-EMPTY:
 // CHECK-NEXT: const RTLIB::Libcall llvm::RTLIB::RuntimeLibcallsInfo::ImplToLibcall[RTLIB::NumLibcallImpls] = {
 // CHECK-NEXT: RTLIB::UNKNOWN_LIBCALL, // RTLIB::Unsupported
 // CHECK-NEXT: RTLIB::MEMCPY, // RTLIB::___memcpy
@@ -162,11 +174,11 @@ def BlahLibrary : SystemRuntimeLibrary<isBlahArch, (add calloc, LibraryWithCondi
 // CHECK-NEXT: }
 
 // CHECK: iota_range<RTLIB::LibcallImpl> RTLIB::RuntimeLibcallsInfo::lookupLibcallImplNameImpl(StringRef Name) {
-// CHECK: static constexpr std::pair<uint16_t, uint16_t> HashTableNameToEnum[16] = {
-// CHECK: {2, 9}, // 0x000000705301b8, ___memset
-// CHECK: {0, 0},
-// CHECK: {6, 6}, // 0x0000001417a2af, calloc
-// CHECK: {0, 0},
+// CHECK: static constexpr uint16_t HashTableNameToEnum[16] = {
+// CHECK: 2, // 0x000000705301b8, ___memset
+// CHECK: 0,
+// CHECK: 6, // 0x0000001417a2af, calloc
+// CHECK: 0,
 // CHECK: };
 
 // CHECK: unsigned Idx = (hash(Name) % 8) * 2;
diff --git a/llvm/utils/TableGen/Basic/RuntimeLibcallsEmitter.cpp b/llvm/utils/TableGen/Basic/RuntimeLibcallsEmitter.cpp
index 0423139f21e44..f0a5cef7bd1cb 100644
--- a/llvm/utils/TableGen/Basic/RuntimeLibcallsEmitter.cpp
+++ b/llvm/utils/TableGen/Basic/RuntimeLibcallsEmitter.cpp
@@ -463,15 +463,13 @@ void RuntimeLibcallEmitter::emitNameMatchHashTable(
 
   // Emit pair of RTLIB::LibcallImpl, size of the string name. It's important to
   // avoid strlen on the string table entries.
-  OS << "  static constexpr std::pair<uint16_t, uint16_t> HashTableNameToEnum["
-     << Lookup.size() << "] = {\n";
+  OS << "  static constexpr uint16_t HashTableNameToEnum[" << Lookup.size()
+     << "] = {\n";
 
   for (auto [FuncName, Hash, TableVal] : Lookup) {
-    OS << "    {" << TableVal << ", " << FuncName.size() << "},";
-
-    if (TableVal != 0) {
+    OS << "    " << TableVal << ',';
+    if (TableVal != 0)
       OS << " // " << format_hex(Hash, 16) << ", " << FuncName;
-    }
 
     OS << '\n';
   }
@@ -482,11 +480,12 @@ void RuntimeLibcallEmitter::emitNameMatchHashTable(
      << ";\n\n"
         "  for (int I = 0; I != "
      << Collisions << R"(; ++I) {
-    auto [Entry, StringSize] = HashTableNameToEnum[Idx + I];
+    const uint16_t Entry = HashTableNameToEnum[Idx + I];
     const uint16_t StrOffset = RuntimeLibcallNameOffsetTable[Entry];
+    const uint8_t StrSize = RuntimeLibcallNameSizeTable[Entry];
     StringRef Str(
       &RTLIB::RuntimeLibcallsInfo::RuntimeLibcallImplNameTableStorage[StrOffset],
-      StringSize);
+      StrSize);
     if (Str == Name)
       return libcallImplNameHit(Entry, StrOffset);
   }
@@ -522,6 +521,13 @@ const uint16_t RTLIB::RuntimeLibcallsInfo::RuntimeLibcallNameOffsetTable[] = {
   }
   OS << "};\n";
 
+  OS << R"(
+const uint8_t RTLIB::RuntimeLibcallsInfo::RuntimeLibcallNameSizeTable[] = {
+)";
+  for (const RuntimeLibcallImpl &LibCallImpl : RuntimeLibcallImplDefList)
+    OS << "  " << LibCallImpl.getLibcallFuncName().size() << ",\n";
+  OS << "};\n\n";
+
   // Emit the reverse mapping from implementation libraries to RTLIB::Libcall
   OS << "const RTLIB::Libcall llvm::RTLIB::RuntimeLibcallsInfo::"
         "ImplToLibcall[RTLIB::NumLibcallImpls] = {\n"

[nikic]

LGTM with the test failure fixed.

[llvm-ci]

LLVM Buildbot has detected a new failure on builder clang-aarch64-quick running on linaro-clang-aarch64-quick while building llvm at step 5 "ninja check 1".

Full details are available at: https://lab.llvm.org/buildbot/#/builders/65/builds/21273

Here is the relevant piece of the build log for the reference

Step 5 (ninja check 1) failure: stage 1 checked (failure)
******************** TEST 'Clangd Unit Tests :: ./ClangdTests/162/331' FAILED ********************
Script(shard):
--
GTEST_OUTPUT=json:/home/tcwg-buildbot/worker/clang-aarch64-quick/stage1/tools/clang/tools/extra/clangd/unittests/./ClangdTests-Clangd Unit Tests-264046-162-331.json GTEST_SHUFFLE=0 GTEST_TOTAL_SHARDS=331 GTEST_SHARD_INDEX=162 /home/tcwg-buildbot/worker/clang-aarch64-quick/stage1/tools/clang/tools/extra/clangd/unittests/./ClangdTests
--

Note: This is test shard 163 of 331.
[==========] Running 4 tests from 4 test suites.
[----------] Global test environment set-up.
[----------] 1 test from CompletionTest
[ RUN      ] CompletionTest.MacroFromPreamble
Built preamble of size 707872 for file /clangd-test/TestTU.cpp version null in 0.10 seconds
Code complete: fuzzyFind({
  "AnyScope": false,
  "Limit": null,
  "PreferredTypes": [],
  "ProximityPaths": [
    "/clangd-test/TestTU.cpp"
  ],
  "Query": "CLANGD_",
  "RestrictForCodeCompletion": true,
  "Scopes": [
    ""
  ]
})
Code complete: sema context Statement, query scopes [] (AnyScope=false), expected type <none>
Code complete: 2 results from Sema, 1 from Index, 0 matched, 0 from identifiers, 3 returned.
[       OK ] CompletionTest.MacroFromPreamble (114 ms)
[----------] 1 test from CompletionTest (114 ms total)

[----------] 1 test from FileIndexTest
[ RUN      ] FileIndexTest.ReferencesInMainFileWithPreamble
Built preamble of size 710652 for file /clangd-test/TestTU.cpp version null in 0.07 seconds
indexed file AST for /clangd-test/TestTU.cpp version null:
  symbol slab: 1 symbols, 4448 bytes
  ref slab: 2 symbols, 2 refs, 4272 bytes
  relations slab: 0 relations, 24 bytes
Build dynamic index for main-file symbols with estimated memory usage of 11576 bytes
Built preamble of size 710652 for file /clangd-test/TestTU.cpp version null in 0.04 seconds
indexed preamble AST for /clangd-test/TestTU.cpp version null:
  symbol slab: 2 symbols, 4680 bytes
  ref slab: 0 symbols, 0 refs, 128 bytes
  relations slab: 0 relations, 24 bytes
[       OK ] FileIndexTest.ReferencesInMainFileWithPreamble (138 ms)
[----------] 1 test from FileIndexTest (139 ms total)

[----------] 1 test from JSONTransportTest
[ RUN      ] JSONTransportTest.EndOfFile
<<< {"jsonrpc":"2.0","method":"call","params":1234}

...