[IR] Add TargetExtTypeClass

[nhaehnle]

Summary

This allows for extensibility in how the target type info is obtained,
serving two purposes:

1. Small step towards "Dialects in LLVM", in providing extensibility for users of LLVM. Users of LLVM can now define their own "target" extension types (now a somewhat misleading name) with custom layout and properties, to represent high-level concepts that are lowered away before code reaches the actual backend.

2. By storing a pointer to the type class in the type itself, we can avoid string comparisons when looking up type info.

[jsilvanus]

Is the patch missing changes in "llvm/IR/TargetExtType.h"?

[jsilvanus]

llvm/lib/IR/LLVMContext.cpp

397

Maybe add an assert that TypeClass is not yet registered?

[nhaehnle]

Major update, adding IR printing, parsing and bitcode writing/reading of the type info.

Also adds the ability to register a verifier for target types, which is used by aarch64.svcount.

[nhaehnle]

Remove two TODOs that have been done.

[arsenm]

llvm/docs/LangRef.rst

3802–3803 ↗

(On Diff #521314)

I don't think the LangRef should refer to doxygen for things like this and should explicitly list the properties

llvm/lib/AsmParser/LLParser.cpp

3259 ↗

(On Diff #521314)

Missing assembler tests for the various error cases

llvm/lib/Bitcode/Writer/BitcodeWriter.cpp

3992 ↗

(On Diff #521314)

Use tuple unbinding syntax instead of .first/.second

4009 ↗

(On Diff #521314)

Seems like it should be report_fatal_error

[nhaehnle]

llvm/docs/LangRef.rst

3802–3803 ↗

(On Diff #521314)

That text was pre-existing, but I'll replace it.

llvm/lib/Bitcode/Writer/BitcodeWriter.cpp

4009 ↗

(On Diff #521314)

This is the bitcode *writer*, so getting here is always a bug in the code, not a bug in any input (extending sdata::Value without updating this code here).

[nhaehnle]

• explicitly list properties in LangRef
• add negative parser/assembler tests
• use tuple binding syntax

[Flakebi]

llvm/test/Assembler/invalid-typeinfo.ll

51 ↗

(On Diff #522078)

What happens when adding a target type with the same name twice?

[nhaehnle]

llvm/test/Assembler/invalid-typeinfo.ll

51 ↗

(On Diff #522078)

This should be accepted as long as there's no conflict in the type info. I'm adding a test.

[nhaehnle]

Rebase, layout type defaults to void

[Flakebi]

A question inline, looks good to me otherwise.

llvm/docs/BitCodeFormat.rst

568 ↗

(On Diff #532901)

Is the XXXXYXXXXX string intended or accidental?

[nhaehnle]

llvm/docs/BitCodeFormat.rst

568 ↗

(On Diff #532901)

Whoops :) That is very much accidental. I meant to update that line but then managed to miss it on my final scan.

[nhaehnle]

Missed a spot...

[Flakebi]

Thanks! LGTM

[nikic]

Could you please add a test where a target type is first used and later type info for it is provided? I'd like to make sure this is an error.

[nikic]

llvm/include/llvm/Bitcode/LLVMBitCodes.h

738 ↗

(On Diff #532954)

What is SDATA_SYMBOL used for, in the context of this patch? It doesn't look like the corresponding IR handling in D150370 supports this.

[nhaehnle]

Could you please add a test where a target type is first used and later type info for it is provided? I'd like to make sure this is an error.

Added the conflicting-typeinfos3.ll subtest which cover this (if I understood you correctly).

[nhaehnle]

llvm/include/llvm/Bitcode/LLVMBitCodes.h

738 ↗

(On Diff #532954)

It isn't used yet. Symbols-as-values is something that I added when exploring how to put enums into metadata, and I didn't remove it here because it does seem useful.

[nikic]

In D147697#4435720, @nhaehnle wrote:

Could you please add a test where a target type is first used and later type info for it is provided? I'd like to make sure this is an error.

Added the conflicting-typeinfos3.ll subtest which cover this (if I understood you correctly).

Yes, that's what I had in mind. Thanks!

llvm/docs/LangRef.rst

3837 ↗

(On Diff #532954)

This could use some clarification on how the properties for target types with different arguments relate, if at all. If you define properties for target("mytype"), do those also apply to target("mytype", i32)?

Do you need to repeat the properties for every combination of arguments in the IR, or is there some inheritance?

llvm/include/llvm/Bitcode/LLVMBitCodes.h

738 ↗

(On Diff #532954)

In that case, please drop it until it is used for something (and thus testable).

[nhaehnle]

• Drop symbols-as-values for now
• LangRef clarifications

llvm/docs/LangRef.rst

3837 ↗

(On Diff #532954)

They don't relate and there is no inheritance, every target type is distinct and has its own properties. I did briefly consider the possibility of inheritance, but it seemed to quickly get very complex and not that useful. I'm going to try to clarify that.

[nikic]

llvm/docs/LangRef.rst

3837 ↗

(On Diff #532954)

I see. I think this point could use some input from people who are using target types, in particular @jcranmer-intel.

The way I originally expected this to work is that the target type properties are independent of the type arguments entirely, so that target("mytype") and all target("mytype", Ti, Ni) share the same properties.

I'm not familiar enough with how these types are used to say whether independent properties for each argument combination makes sense or not.

[nhaehnle]

Rebase

[jcranmer-intel]

llvm/docs/LangRef.rst

3837 ↗

(On Diff #532954)

All of the use cases I have had have the property that properties are based entirely on the string property. Indeed, one property that could be useful to check is the number of integer/type arguments!

That said, there are some cases where varying type parameters might change the properties of the type--suppose something like target("complex", float) versus target("complex", double) or target("decfp", 64) versus target("decfp", 32). Although for many of those types, I do question if they should be target extension types in the first place.

llvm/include/llvm/IR/TargetExtType.h

51–59 ↗

(On Diff #533500)

I question the need for these to be function pointers rather than instance properties. (The verifier function does need to be its own function).

[nhaehnle]

It seems this comes down to this question: Should the type properties be per type name or should they be per type?

But so far, it seems that nobody has very strong feelings in either direction. Personally, I'd err on the side of flexibility (hence the current shape of this change), but I don't feel strongly about it. How do we decide such things? :)

llvm/docs/LangRef.rst

3837 ↗

(On Diff #532954)

There's a question of how strongly you feel about this and how it would affect the runtime representation.

If we did change the properties to be per type name instead of per type, we should probably also store them per type name.

llvm/include/llvm/IR/TargetExtType.h

51–59 ↗

(On Diff #533500)

This is basically the same question as before. If we decide that these properties should be fixed on a per-type-name basis, then yes, we can just make them member variables here. Otherwise, they need to be fallbacks.

[nhaehnle]

llvm/include/llvm/IR/TargetExtType.h

51–59 ↗

(On Diff #533500)

s/fallbacks/callbacks/

llvm/include/llvm/IR/DerivedTypes.h

 #include <cassert>
 #include <cstdint>
 
 namespace llvm {
 
 class Value;
 class APInt;
 class LLVMContext;
+class TargetExtTypeClass;
 
 /// Class to represent integer types. Note that this class is also used to
 /// represent the built-in integer types: Int1Ty, Int8Ty, Int16Ty, Int32Ty and
 /// Int64Ty.
 /// Integer representation type
 class IntegerType : public Type {
   friend class LLVMContextImpl;
 

 
 /// Class to represent target extensions types, which are generally
 /// unintrospectable from target-independent optimizations.
 ///
 /// Target extension types have a string name, and optionally have type and/or
 /// integer parameters. The exact meaning of any parameters is dependent on the
 /// target.
 class TargetExtType : public Type {
-  TargetExtType(LLVMContext &C, StringRef Name, ArrayRef<Type *> Types,
-                ArrayRef<unsigned> Ints);
+  TargetExtType(LLVMContext &C, const TargetExtTypeClass *Class, StringRef Name,
+                ArrayRef<Type *> Types, ArrayRef<unsigned> Ints);
 
+  const TargetExtTypeClass *Class;
   // These strings are ultimately owned by the context.
   StringRef Name;
   unsigned *IntParams;
 
 public:
   TargetExtType(const TargetExtType &) = delete;
   TargetExtType &operator=(const TargetExtType &) = delete;
 
   /// Return a target extension type having the specified name and optional
   /// type and integer parameters.
   static TargetExtType *get(LLVMContext &Context, StringRef Name,
                             ArrayRef<Type *> Types = std::nullopt,
                             ArrayRef<unsigned> Ints = std::nullopt);
 
+  /// Return a target extension type of a known type class, having the specified
+  /// name and optional type and integer parameters.
+  static TargetExtType *get(LLVMContext &Context,
+                            const TargetExtTypeClass *Class, StringRef Name,
+                            ArrayRef<Type *> Types = std::nullopt,
+                            ArrayRef<unsigned> Ints = std::nullopt);
+
+  /// Return the type class of this target extension type, or null if none has
+  /// been registered.
+  const TargetExtTypeClass *getClass() const { return Class; }
+
   /// Return the name for this target extension type. Two distinct target
   /// extension types may have the same name if their type or integer parameters
   /// differ.
   StringRef getName() const { return Name; }
 
   /// Return the type parameters for this particular target extension type. If
   /// there are no parameters, an empty array is returned.
   ArrayRef<Type *> type_params() const {

llvm/include/llvm/IR/LLVMContext.h

 class Function;
 class Instruction;
 class LLVMContextImpl;
 class Module;
 class OptPassGate;
 template <typename T> class SmallVectorImpl;
 template <typename T> class StringMapEntry;
 class StringRef;
+class TargetExtTypeClass;
 class Twine;
 class LLVMRemarkStreamer;
 
 namespace remarks {
 class RemarkStreamer;
 }
 
 namespace SyncScope {
#undef LLVM_FIXED_MD_KIND
   /// times, but only with the same value. Note that creating a pointer type or
   /// otherwise querying the opaque pointer mode performs an implicit set to
   /// the default value.
   void setOpaquePointers(bool Enable) const;
 
   /// Whether typed pointers are supported. If false, all pointers are opaque.
   bool supportsTypedPointers() const;
 
+  /// Register a custom extension type class.
+  void registerTargetExtTypeClass(const TargetExtTypeClass *TypeClass);
+
 private:
   // Module needs access to the add/removeModule methods.
   friend class Module;
 
   /// addModule - Register a module as being instantiated in this context.  If
   /// the context is deleted, the module will be deleted as well.
   void addModule(Module*);
 

llvm/lib/IR/LLVMContext.cpp

 #include "LLVMContextImpl.h"
 #include "llvm/ADT/SmallVector.h"
 #include "llvm/ADT/StringMap.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/ADT/Twine.h"
 #include "llvm/IR/DiagnosticInfo.h"
 #include "llvm/IR/DiagnosticPrinter.h"
 #include "llvm/IR/LLVMRemarkStreamer.h"
+#include "llvm/IR/TargetExtType.h"
 #include "llvm/Remarks/RemarkStreamer.h"
 #include "llvm/Support/Casting.h"
 #include "llvm/Support/ErrorHandling.h"
 #include "llvm/Support/raw_ostream.h"
 #include <cassert>
 #include <cstdlib>
 #include <string>
 #include <utility>
assert(SingleThreadSSID == SyncScope::SingleThread &&
          "singlethread synchronization scope ID drifted!");
   (void)SingleThreadSSID;
 
   SyncScope::ID SystemSSID =
       pImpl->getOrInsertSyncScopeID("");
   assert(SystemSSID == SyncScope::System &&
          "system synchronization scope ID drifted!");
   (void)SystemSSID;
+
+  static const TargetExtTypeClass SpirvTypes(
+      "spirv.", true, [](const TargetExtType *T) {
+        return TargetTypeInfo(Type::getInt8PtrTy(T->getContext(), 0),
+                              TargetExtType::HasZeroInit,
+                              TargetExtType::CanBeGlobal);
+      });
+  registerTargetExtTypeClass(&SpirvTypes);
+
+  static const TargetExtTypeClass Aarch64SVCount(
+      "aarch64.svcount", false, [](const TargetExtType *T) {
+        return TargetTypeInfo(
+            ScalableVectorType::get(Type::getInt1Ty(T->getContext()), 16));
+      });
+  registerTargetExtTypeClass(&Aarch64SVCount);
 }
 
 LLVMContext::~LLVMContext() { delete pImpl; }
 
 void LLVMContext::addModule(Module *M) {
   pImpl->OwnedModules.insert(M);
 }
 

 
 void LLVMContext::setOpaquePointers(bool Enable) const {
   pImpl->setOpaquePointers(Enable);
 }
 
 bool LLVMContext::supportsTypedPointers() const {
   return !pImpl->getOpaquePointers();
 }
+
+void LLVMContext::registerTargetExtTypeClass(
+    const TargetExtTypeClass *TypeClass) {
+  assert(!pImpl->TargetExtTypeClassesFrozen);
+  pImpl->TargetExtTypeClasses.push_back(TypeClass);
+}

llvm/lib/IR/LLVMContextImpl.h

#include "llvm/IR/Metadata.def"
 
   /// Maintain the GC name for each function.
   ///
   /// This saves allocating an additional word in Function for programs which
   /// do not use GC (i.e., most programs) at the cost of increased overhead for
   /// clients which do use GC.
   DenseMap<const Function *, std::string> GCNames;
 
+  SmallVector<const TargetExtTypeClass *> TargetExtTypeClasses;
+  bool TargetExtTypeClassesFrozen = false;
+
   /// Flag to indicate if Value (other than GlobalValue) retains their name or
   /// not.
   bool DiscardValueNames = false;
 
   LLVMContextImpl(LLVMContext &C);
   ~LLVMContextImpl();
 
   /// Destroy the ConstantArrays if they are not used.

llvm/lib/IR/Type.cpp

 #include "llvm/ADT/APInt.h"
 #include "llvm/ADT/SmallString.h"
 #include "llvm/ADT/StringMap.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/IR/Constant.h"
 #include "llvm/IR/Constants.h"
 #include "llvm/IR/DerivedTypes.h"
 #include "llvm/IR/LLVMContext.h"
+#include "llvm/IR/TargetExtType.h"
 #include "llvm/IR/Value.h"
 #include "llvm/Support/Casting.h"
 #include "llvm/Support/TypeSize.h"
 #include "llvm/Support/raw_ostream.h"
 #include <cassert>
 #include <utility>
 
 using namespace llvm;

 bool PointerType::isLoadableOrStorableType(Type *ElemTy) {
   return isValidElementType(ElemTy) && !ElemTy->isFunctionTy();
 }
 
 //===----------------------------------------------------------------------===//
 //                       TargetExtType Implementation
 //===----------------------------------------------------------------------===//
 
-TargetExtType::TargetExtType(LLVMContext &C, StringRef Name,
-                             ArrayRef<Type *> Types, ArrayRef<unsigned> Ints)
-    : Type(C, TargetExtTyID), Name(C.pImpl->Saver.save(Name)) {
+TargetExtTypeClass::TargetExtTypeClass(StringRef Name, bool NameIsPrefix,
+                                       GetTypeInfoFn *GetTypeInfo)
+    : Name(Name.str()), NameIsPrefix(NameIsPrefix), GetTypeInfo(GetTypeInfo) {
+  assert(!Name.empty());
+  assert(NameIsPrefix == Name.ends_with("."));
+}
+
+const TargetExtTypeClass *TargetExtTypeClass::find(LLVMContext &Ctx,
+                                                   StringRef Name) {
+  Ctx.pImpl->TargetExtTypeClassesFrozen = true;
+
+  for (const TargetExtTypeClass *Class : Ctx.pImpl->TargetExtTypeClasses) {
+    if ((Class->NameIsPrefix && Name.starts_with(Class->Name)) ||
+        (!Class->NameIsPrefix && Name == Class->Name)) {
+      return Class;
+    }
+  }
+
+  return nullptr;
+}
+
+TargetExtType::TargetExtType(LLVMContext &C, const TargetExtTypeClass *Class,
+                             StringRef Name, ArrayRef<Type *> Types,
+                             ArrayRef<unsigned> Ints)
+    : Type(C, TargetExtTyID), Class(Class), Name(C.pImpl->Saver.save(Name)) {
   NumContainedTys = Types.size();
 
   // Parameter storage immediately follows the class in allocation.
   Type **Params = reinterpret_cast<Type **>(this + 1);
   ContainedTys = Params;
   for (Type *T : Types)
     *Params++ = T;
 
   setSubclassData(Ints.size());
   unsigned *IntParamSpace = reinterpret_cast<unsigned *>(Params);
   IntParams = IntParamSpace;
   for (unsigned IntParam : Ints)
     *IntParamSpace++ = IntParam;
 }
 
 TargetExtType *TargetExtType::get(LLVMContext &C, StringRef Name,
                                   ArrayRef<Type *> Types,
                                   ArrayRef<unsigned> Ints) {
+  const auto *Class = TargetExtTypeClass::find(C, Name);
+  return get(C, Class, Name, Types, Ints);
+}
+
+TargetExtType *TargetExtType::get(LLVMContext &C,
+                                  const TargetExtTypeClass *Class,
+                                  StringRef Name, ArrayRef<Type *> Types,
+                                  ArrayRef<unsigned> Ints) {
   const TargetExtTypeKeyInfo::KeyTy Key(Name, Types, Ints);
   TargetExtType *TT;
   // Since we only want to allocate a fresh target type in case none is found
   // and we don't want to perform two lookups (one for checking if existent and
   // one for inserting the newly allocated one), here we instead lookup based on
   // Key and update the reference to the target type in-place to a newly
   // allocated one if not found.
   auto Insertion = C.pImpl->TargetExtTypes.insert_as(nullptr, Key);
   if (Insertion.second) {
     // The target type was not found. Allocate one and update TargetExtTypes
     // in-place.
     TT = (TargetExtType *)C.pImpl->Alloc.Allocate(
         sizeof(TargetExtType) + sizeof(Type *) * Types.size() +
             sizeof(unsigned) * Ints.size(),
         alignof(TargetExtType));
-    new (TT) TargetExtType(C, Name, Types, Ints);
+    new (TT) TargetExtType(C, Class, Name, Types, Ints);
     *Insertion.first = TT;
   } else {
     // The target type was found. Just return it.
     TT = *Insertion.first;
   }
   return TT;
 }
 
-namespace {
-struct TargetTypeInfo {
-  Type *LayoutType;
-  uint64_t Properties;
-
-  template <typename... ArgTys>
-  TargetTypeInfo(Type *LayoutType, ArgTys... Properties)
-      : LayoutType(LayoutType), Properties((0 | ... | Properties)) {}
-};
-} // anonymous namespace
-
 static TargetTypeInfo getTargetTypeInfo(const TargetExtType *Ty) {
-  LLVMContext &C = Ty->getContext();
-  StringRef Name = Ty->getName();
-  if (Name.startswith("spirv."))
-    return TargetTypeInfo(Type::getInt8PtrTy(C, 0), TargetExtType::HasZeroInit,
-                          TargetExtType::CanBeGlobal);
-
-  // Opaque types in the AArch64 name space.
-  if (Name == "aarch64.svcount")
-    return TargetTypeInfo(ScalableVectorType::get(Type::getInt1Ty(C), 16));
-
-  return TargetTypeInfo(Type::getVoidTy(C));
+  if (const auto *Class = Ty->getClass())
+    return Class->getTypeInfo(Ty);
+  return TargetTypeInfo(Type::getVoidTy(Ty->getContext()));
 }
 
 Type *TargetExtType::getLayoutType() const {
   return getTargetTypeInfo(this).LayoutType;
 }
 
 bool TargetExtType::hasProperty(Property Prop) const {
   uint64_t Properties = getTargetTypeInfo(this).Properties;
   return (Properties & Prop) == Prop;
 }