Improve support for non-llvm intrinsics.

[arames]

Summary

These commits improve support for target and non-llvm intrinsics.

• Provide a way to declare intrinsics not prefixed with llvm..
• Expose the intrinsic name lookup code so it can be used by intrinsics not starting with llvm..

[arames]

Rebased on top of tree.

[ab]

Can you explain why this is necessary?

"llvm." is the only reserved prefix for global identifiers; removing that assumption opens the door to problems we could easily avoid (for instance, what happens when I use IR that has functions named 'X.foo' ? Are those doomed to be forbidden from the 'X' backend?)

[rnk]

I'm having a hard time justifying support for this in LLVM. You can already make target-specific intrinsics starting with "llvm.myisa.", so there isn't much gained by this.

[arames]

Following some review with Ahmed (ab), we think it will not make sense for upstream.

include/llvm/IR/Intrinsics.h

#undef GET_INTRINSIC_ENUM_VALUES
   /// it.
   ///
   /// The Tys parameter is for intrinsics with overloaded types (e.g., those
   /// using iAny, fAny, vAny, or iPTRAny).  For a declaration of an overloaded
   /// intrinsic, Tys must provide exactly one type for each overloaded type in
   /// the intrinsic.
   Function *getDeclaration(Module *M, ID id, ArrayRef<Type*> Tys = None);
 
+  /// Looks up Name in NameTable via binary search. NameTable must be sorted.
+  /// 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 lookupIntrinsicByName(ArrayRef<const char *> NameTable, StringRef Name);
+
   /// 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);
+  inline int lookupLLVMIntrinsicByName(ArrayRef<const char *> NameTable,
+                                       StringRef Name) {
+    assert(Name.startswith("llvm."));
+    return lookupIntrinsicByName(NameTable, Name);
+  }
 
   /// Map a GCC builtin name to an intrinsic ID.
   ID getIntrinsicForGCCBuiltin(const char *Prefix, StringRef BuiltinName);
 
   /// Map a MS builtin name to an intrinsic ID.
   ID getIntrinsicForMSBuiltin(const char *Prefix, StringRef BuiltinName);
 
   /// This is a type descriptor which explains the type requirements of an

include/llvm/IR/Intrinsics.td

class Intrinsic<list<LLVMType> ret_types,
                 string name = ""> : SDPatternOperator {
   string LLVMName = name;
   string TargetPrefix = "";   // Set to a prefix for target-specific intrinsics.
   list<LLVMType> RetTypes = ret_types;
   list<LLVMType> ParamTypes = param_types;
   list<IntrinsicProperty> IntrProperties = properties;
 
   bit isTarget = 0;
+  bit SkipLLVMPrefix = 0;
 }
 
 /// GCCBuiltin - If this intrinsic exactly corresponds to a GCC builtin, this
 /// specifies the name of the builtin.  This provides automatic CBE and CFE
 /// support.
 class GCCBuiltin<string name> {
   string GCCBuiltinName = name;
 }

lib/IR/IntrinsicInst.cpp

Value *DbgInfoIntrinsic::getVariableLocation(bool AllowNullOp) const {
   if (auto *V = dyn_cast<ValueAsMetadata>(MD))
     return V->getValue();
 
   // When the value goes to null, it gets replaced by an empty MDNode.
   assert(!cast<MDNode>(MD)->getNumOperands() && "Expected an empty MDNode");
   return nullptr;
 }
 
-int llvm::Intrinsic::lookupLLVMIntrinsicByName(ArrayRef<const char *> NameTable,
-                                               StringRef Name) {
-  assert(Name.startswith("llvm."));
-
+int llvm::Intrinsic::lookupIntrinsicByName(ArrayRef<const char *> NameTable,
   // 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
+  // size 1. 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.
+  size_t CmpEnd = 0;
   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 = Name.find('.', CmpStart == 0 ? 0 : 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)

test/TableGen/intrinsic-skip-llvm-prefix.td

+// RUN: llvm-tblgen -gen-tgt-intrinsic %s | FileCheck %s
+// XFAIL: vg_leak
+
+class IntrinsicProperty;
+
+class ValueType<int size, int value> {
+  string Namespace = "MVT";
+  int Size = size;
+  int Value = value;
+}
+
+class LLVMType<ValueType vt> {
+  ValueType VT = vt;
+}
+
+class Intrinsic<string name = ""> {
+  string LLVMName = name;
+  string TargetPrefix = "";
+  list<LLVMType> RetTypes = [];
+  list<LLVMType> ParamTypes = [];
+  list<IntrinsicProperty> IntrProperties = [];
+  bit isTarget = 0;
+  bit SkipLLVMPrefix = 0;
+}
+
+
+// CHECK-DAG: intrinsic_for_target{{,?}}           // llvm.intrinsic.for.target
+// CHECK-DAG: explicitly_named_intrinsic_1{{,?}}   // llvm.intrinsic.with.explicit.name
+// CHECK-DAG: target_other_intrinsic{{,?}}         // llvm.target.other.intrinsic
+// CHECK-DAG: explicitly_named_intrinsic_3{{,?}}   // llvm.target.intrinsic.with.explicit.name
+// CHECK-DAG: no_llvm_prefix{{,?}}                 // no.llvm.prefix
+// CHECK-DAG: explicitly_named_intrinsic_2{{,?}}   // intrinsic.without.llvm.prefix
+// CHECK-DAG: target_intrinsic{{,?}}               // target.intrinsic
+// CHECK-DAG: explicitly_named_intrinsic_4{{,?}}   // target.intrinsic.without.llvm.prefix
+
+let isTarget = 1 in {
+  def int_intrinsic_for_target : Intrinsic;
+  def int_explicitly_named_intrinsic_1 : Intrinsic<"llvm.intrinsic.with.explicit.name">;
+
+  let TargetPrefix = "target" in {
+    def int_target_other_intrinsic : Intrinsic;
+    def int_explicitly_named_intrinsic_3 : Intrinsic<"llvm.target.intrinsic.with.explicit.name">;
+  }
+
+  let SkipLLVMPrefix = 1 in {
+    def int_no_llvm_prefix : Intrinsic;
+    def int_explicitly_named_intrinsic_2 : Intrinsic<"intrinsic.without.llvm.prefix">;
+  }
+
+  let TargetPrefix = "target", SkipLLVMPrefix = 1 in {
+    def int_target_intrinsic : Intrinsic;
+    def int_explicitly_named_intrinsic_4 : Intrinsic<"target.intrinsic.without.llvm.prefix">;
+  }
+}

unittests/IR/IntrinsicsTest.cpp

TEST(IntrinNameLookup, Basic) {
   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);
 }
 
+TEST(IntrinNameLookup, DontAssumeLLVMPrefix) {
+  int I = Intrinsic::lookupIntrinsicByName(NameTable1, "llvm.foo");
+  EXPECT_EQ(0, I);
+  I = Intrinsic::lookupIntrinsicByName(NameTable1, "llvm.foo.f64");
+  EXPECT_EQ(0, I);
+  I = Intrinsic::lookupIntrinsicByName(NameTable1, "llvm.foo.b");
+  EXPECT_EQ(2, I);
+  I = Intrinsic::lookupIntrinsicByName(NameTable1, "llvm.foo.b.a");
+  EXPECT_EQ(3, I);
+  I = Intrinsic::lookupIntrinsicByName(NameTable1, "llvm.foo.c");
+  EXPECT_EQ(4, I);
+  I = Intrinsic::lookupIntrinsicByName(NameTable1, "llvm.foo.c.f64");
+  EXPECT_EQ(4, I);
+}
+
+static const char *const NameTable2[] = {
+  "p1.foo",
+  "p1.foo.a",
+  "p1.foo.b",
+  "p1.foo.b.a",
+  "p1.foo.c",
+  "p2.foo",
+  "p2.foo.a",
+  "p2.foo.b",
+  "p2.foo.b.a",
+  "p2.foo.c",
+};
+
+TEST(IntrinNameLookup, OtherPrefixes) {
+  int I = Intrinsic::lookupIntrinsicByName(NameTable2, "p1.foo");
+  EXPECT_EQ(0, I);
+  I = Intrinsic::lookupIntrinsicByName(NameTable2, "p1.foo.f64");
+  EXPECT_EQ(0, I);
+  I = Intrinsic::lookupIntrinsicByName(NameTable2, "p1.foo.b");
+  EXPECT_EQ(2, I);
+  I = Intrinsic::lookupIntrinsicByName(NameTable2, "p1.foo.b.a");
+  EXPECT_EQ(3, I);
+  I = Intrinsic::lookupIntrinsicByName(NameTable2, "p1.foo.c");
+  EXPECT_EQ(4, I);
+  I = Intrinsic::lookupIntrinsicByName(NameTable2, "p1.foo.c.f64");
+  EXPECT_EQ(4, I);
+
+  I = Intrinsic::lookupIntrinsicByName(NameTable2, "p2.foo");
+  EXPECT_EQ(5, I);
+  I = Intrinsic::lookupIntrinsicByName(NameTable2, "p2.foo.f64");
+  EXPECT_EQ(5, I);
+  I = Intrinsic::lookupIntrinsicByName(NameTable2, "p2.foo.b");
+  EXPECT_EQ(7, I);
+  I = Intrinsic::lookupIntrinsicByName(NameTable2, "p2.foo.b.a");
+  EXPECT_EQ(8, I);
+  I = Intrinsic::lookupIntrinsicByName(NameTable2, "p2.foo.c");
+  EXPECT_EQ(9, I);
+  I = Intrinsic::lookupIntrinsicByName(NameTable2, "p2.foo.c.f64");
+  EXPECT_EQ(9, I);
+}
+
 } // end namespace

utils/TableGen/CodeGenTarget.cpp

CodeGenIntrinsic::CodeGenIntrinsic(Record *R) {
   if (R->getValue("GCCBuiltinName"))  // Ignore a missing GCCBuiltinName field.
     GCCBuiltinName = R->getValueAsString("GCCBuiltinName");
   if (R->getValue("MSBuiltinName"))   // Ignore a missing MSBuiltinName field.
     MSBuiltinName = R->getValueAsString("MSBuiltinName");
 
   TargetPrefix = R->getValueAsString("TargetPrefix");
   Name = R->getValueAsString("LLVMName");
 
+  bool SkipLLVMPrefix =
+      R->getValue("SkipLLVMPrefix") && R->getValueAsBit("SkipLLVMPrefix");
+  if (SkipLLVMPrefix && !R->getValueAsBit("isTarget"))
+    PrintFatalError(
+        "Skipping the `llvm.` prefix is only allowed for target intrinsics.");
+
+  std::string RequiredPrefix = SkipLLVMPrefix ? "" : "llvm.";
   if (Name == "") {
     // If an explicit name isn't specified, derive one from the DefName.
-    Name = "llvm.";
+    Name = RequiredPrefix;
 
     for (unsigned i = 0, e = EnumName.size(); i != e; ++i)
       Name += (EnumName[i] == '_') ? '.' : EnumName[i];
-  } else {
-    // Verify it starts with "llvm.".
-    if (Name.size() <= 5 ||
-        std::string(Name.begin(), Name.begin() + 5) != "llvm.")
-      PrintFatalError("Intrinsic '" + DefName + "'s name does not start with 'llvm.'!");
   }
 
-  // If TargetPrefix is specified, make sure that Name starts with
-  // "llvm.<targetprefix>.".
-  if (!TargetPrefix.empty()) {
-    if (Name.size() < 6+TargetPrefix.size() ||
-        std::string(Name.begin() + 5, Name.begin() + 6 + TargetPrefix.size())
-        != (TargetPrefix + "."))
-      PrintFatalError("Intrinsic '" + DefName + "' does not start with 'llvm." +
-        TargetPrefix + ".'!");
-  }
+  if (!TargetPrefix.empty())
+    RequiredPrefix += TargetPrefix + ".";
+
+  // Check the prefixes.
+  if (Name.compare(0, RequiredPrefix.size(), RequiredPrefix) != 0)
+    PrintFatalError("Intrinsic '" + DefName + "'s name does not start with '" +
+                    RequiredPrefix + "'!");
 
   // Parse the list of return types.
   std::vector<MVT::SimpleValueType> OverloadedVTs;
   ListInit *TypeList = R->getValueAsListInit("RetTypes");
   for (unsigned i = 0, e = TypeList->size(); i != e; ++i) {
     Record *TyEl = TypeList->getElementAsRecord(i);
     assert(TyEl->isSubClassOf("LLVMType") && "Expected a type!");
     MVT::SimpleValueType VT;