Handle more edge cases in intrinsic name binary search

I tried to make the AMDGPU intrinsic info table use this instead of
another StringMatcher, and some issues arose.

llvm-svn: 258871

llvm/include/llvm/IR/Intrinsics.h

@@ -69,6 +69,13 @@ namespace Intrinsic {
   /// the intrinsic.
   Function *getDeclaration(Module *M, ID id, ArrayRef<Type*> Tys = None);
 
+  /// Looks up Name in NameTable via binary search. NameTable must be sorted
+  /// and all entries must start with "llvm.".  If NameTable contains an exact
+  /// match for Name or a prefix of Name followed by a dot, its index in
+  /// NameTable is returned. Otherwise, -1 is returned.
+  int lookupLLVMIntrinsicByName(ArrayRef<const char *> NameTable,
+                                StringRef Name);
+
   /// Map a GCC builtin name to an intrinsic ID.
   ID getIntrinsicForGCCBuiltin(const char *Prefix, const char *BuiltinName);
 

llvm/lib/IR/Function.cpp

@@ -414,53 +414,14 @@ static const char * const IntrinsicNameTable[] = {
 #undef GET_INTRINSIC_NAME_TABLE
 };
 
-static int lookupLLVMIntrinsicByName(ArrayRef<const char *> NameTable,
-                                     StringRef Name) {
-  // Do a binary search over the table of intrinsic names.
-  const char *const *NameEntry =
-      std::lower_bound(NameTable.begin(), NameTable.end(), Name.data(),
-                       [](const char *LHS, const char *RHS) {
-                         // Don't compare the first 5 characters, they are
-                         // always "llvm.".
-                         return strcmp(LHS + 5, RHS + 5) < 0;
-                       });
-  unsigned Idx = NameEntry - NameTable.begin();
-
-  // Check if this is a direct match.
-  if (Idx < NameTable.size() && strcmp(Name.data(), NameTable[Idx]) == 0)
-    return Idx;
-
-  // Otherwise, back up one entry to look for a prefix of Name where the next
-  // character in Name is a dot.
-  if (Idx == 0)
-    return -1;
-  --Idx;
-  bool CheckPrefixes = true;
-  while (CheckPrefixes) {
-    StringRef FoundName = NameTable[Idx];
-    if (Name.startswith(FoundName) && Name[FoundName.size()] == '.')
-      return Idx;
-    if (Idx == 0)
-      return -1;
-    --Idx;
-    // We have to keep scanning backwards until the previous entry is not a
-    // prefix of the current entry. Consider a key of llvm.foo.f64 and a table
-    // of llvm.foo and llvm.foo.bar.
-    CheckPrefixes = FoundName.startswith(NameTable[Idx]);
-  }
-
-  return -1;
-}
-
 /// \brief This does the actual lookup of an intrinsic ID which
 /// matches the given function name.
 static Intrinsic::ID lookupIntrinsicID(const ValueName *ValName) {
   StringRef Name = ValName->getKey();
-  assert(Name.data()[Name.size()] == '\0' && "non-null terminated ValueName");
 
   ArrayRef<const char *> NameTable(&IntrinsicNameTable[1],
                                    std::end(IntrinsicNameTable));
-  int Idx = lookupLLVMIntrinsicByName(NameTable, Name);
+  int Idx = Intrinsic::lookupLLVMIntrinsicByName(NameTable, Name);
   Intrinsic::ID ID = static_cast<Intrinsic::ID>(Idx + 1);
   if (ID == Intrinsic::not_intrinsic)
     return ID;

llvm/lib/IR/IntrinsicInst.cpp

@@ -25,6 +25,7 @@
 #include "llvm/IR/Constants.h"
 #include "llvm/IR/GlobalVariable.h"
 #include "llvm/IR/Metadata.h"
+#include "llvm/Support/raw_ostream.h"
 using namespace llvm;
 
 //===----------------------------------------------------------------------===//
@@ -79,3 +80,41 @@ const Value *DbgValueInst::getValue() const {
 }
 
 Value *DbgValueInst::getValue() { return getValueImpl(getArgOperand(0)); }
+
+int llvm::Intrinsic::lookupLLVMIntrinsicByName(ArrayRef<const char *> NameTable,
+                                               StringRef Name) {
+  assert(Name.startswith("llvm."));
+
+  // Do successive binary searches of the dotted name components. For
+  // "llvm.gc.experimental.statepoint.p1i8.p1i32", we will find the range of
+  // intrinsics starting with "llvm.gc", then "llvm.gc.experimental", then
+  // "llvm.gc.experimental.statepoint", and then we will stop as the range is
+  // size 1. During the search, we can skip the prefix that we already know is
+  // identical. By using strncmp we consider names with differing suffixes to
+  // be part of the equal range.
+  size_t CmpStart = 0;
+  size_t CmpEnd = 4; // Skip the "llvm" component.
+  const char *const *Low = NameTable.begin();
+  const char *const *High = NameTable.end();
+  const char *const *LastLow = Low;
+  while (CmpEnd < Name.size() && High - Low > 0) {
+    CmpStart = CmpEnd;
+    CmpEnd = Name.find('.', CmpStart + 1);
+    CmpEnd = CmpEnd == StringRef::npos ? Name.size() : CmpEnd;
+    auto Cmp = [CmpStart, CmpEnd](const char *LHS, const char *RHS) {
+      return strncmp(LHS + CmpStart, RHS + CmpStart, CmpEnd - CmpStart) < 0;
+    };
+    LastLow = Low;
+    std::tie(Low, High) = std::equal_range(Low, High, Name.data(), Cmp);
+  }
+  if (High - Low > 0)
+    LastLow = Low;
+
+  if (LastLow == NameTable.end())
+    return -1;
+  StringRef NameFound = *LastLow;
+  if (Name == NameFound ||
+      (Name.startswith(NameFound) && Name[NameFound.size()] == '.'))
+    return LastLow - NameTable.begin();
+  return -1;
+}

llvm/unittests/IR/CMakeLists.txt

@@ -14,6 +14,7 @@ set(IRSources
   DominatorTreeTest.cpp
   IRBuilderTest.cpp
   InstructionsTest.cpp
+  IntrinsicsTest.cpp
   LegacyPassManagerTest.cpp
   MDBuilderTest.cpp
   MetadataTest.cpp

llvm/unittests/IR/IntrinsicsTest.cpp

@@ -0,0 +1,40 @@
+//===- llvm/unittest/IR/IntrinsicsTest.cpp - ------------------------------===//
+//
+//                     The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+
+#include "llvm/IR/IntrinsicInst.h"
+#include "gtest/gtest.h"
+
+using namespace llvm;
+
+namespace {
+
+static const char *const NameTable1[] = {
+  "llvm.foo",
+  "llvm.foo.a",
+  "llvm.foo.b",
+  "llvm.foo.b.a",
+  "llvm.foo.c",
+};
+
+TEST(IntrinNameLookup, Basic) {
+  int I = Intrinsic::lookupLLVMIntrinsicByName(NameTable1, "llvm.foo");
+  EXPECT_EQ(0, I);
+  I = Intrinsic::lookupLLVMIntrinsicByName(NameTable1, "llvm.foo.f64");
+  EXPECT_EQ(0, I);
+  I = Intrinsic::lookupLLVMIntrinsicByName(NameTable1, "llvm.foo.b");
+  EXPECT_EQ(2, I);
+  I = Intrinsic::lookupLLVMIntrinsicByName(NameTable1, "llvm.foo.b.a");
+  EXPECT_EQ(3, I);
+  I = Intrinsic::lookupLLVMIntrinsicByName(NameTable1, "llvm.foo.c");
+  EXPECT_EQ(4, I);
+  I = Intrinsic::lookupLLVMIntrinsicByName(NameTable1, "llvm.foo.c.f64");
+  EXPECT_EQ(4, I);
+}
+
+} // end namespace