Store intrinsic ID by value in Function instead of a string lookup. NFC.

On 64-bit targets, Function has 4-bytes of padding in its struct layout.

This uses the space for the intrinsic ID. It is set and recalculated whenever the function name is set.  This is similar to the current behavior which clears the function from the intrinsic ID cache when its renamed.

The intrinsic cache itself is removed as the only purpose was to speedup calls to getIntrinsicID() which now just reading the new field in the struct.

Reviewed by Duncan.  http://reviews.llvm.org/D9836

llvm-svn: 237642

include/llvm/IR/Function.h

@@ -143,13 +143,17 @@ public:
   /// intrinsic, or if the pointer is null.  This value is always defined to be
   /// zero to allow easy checking for whether a function is intrinsic or not.
   /// The particular intrinsic functions which correspond to this value are
-  /// defined in llvm/Intrinsics.h.  Results are cached in the LLVM context,
-  /// subsequent requests for the same ID return results much faster from the
-  /// cache.
-  ///
-  unsigned getIntrinsicID() const LLVM_READONLY;
+  /// defined in llvm/Intrinsics.h.
+  unsigned getIntrinsicID() const LLVM_READONLY { return IntID; }
   bool isIntrinsic() const { return getName().startswith("llvm."); }
 
+  /// \brief Recalculate the ID for this function if it is an Intrinsic defined
+  /// in llvm/Intrinsics.h.  Sets the intrinsic ID to Intrinsic::not_intrinsic
+  /// if the name of this function does not match an intrinsic in that header.
+  /// Note, this method does not need to be called directly, as it is called
+  /// from Value::setName() whenever the name of this function changes.
+  void recalculateIntrinsicID();
+
   /// getCallingConv()/setCallingConv(CC) - These method get and set the
   /// calling convention of this function.  The enum values for the known
   /// calling conventions are defined in CallingConv.h.

include/llvm/IR/GlobalValue.h

@@ -28,6 +28,10 @@ class Comdat;
 class PointerType;
 class Module;
 
+namespace Intrinsic {
+  enum ID : unsigned;
+};
+
 class GlobalValue : public Constant {
   GlobalValue(const GlobalValue &) = delete;
 public:
@@ -66,7 +70,7 @@ protected:
       : Constant(Ty, VTy, Ops, NumOps), Linkage(Linkage),
         Visibility(DefaultVisibility), UnnamedAddr(0),
         DllStorageClass(DefaultStorageClass),
-        ThreadLocal(NotThreadLocal), Parent(nullptr) {
+        ThreadLocal(NotThreadLocal), IntID((Intrinsic::ID)0U), Parent(nullptr) {
     setName(Name);
   }
 
@@ -84,7 +88,16 @@ private:
   // Give subclasses access to what otherwise would be wasted padding.
   // (19 + 3 + 2 + 1 + 2 + 5) == 32.
   unsigned SubClassData : 19;
+
 protected:
+  /// \brief The intrinsic ID for this subclass (which must be a Function).
+  ///
+  /// This member is defined by this class, but not used for anything.
+  /// Subclasses can use it to store their intrinsic ID, if they have one.
+  ///
+  /// This is stored here to save space in Function on 64-bit hosts.
+  Intrinsic::ID IntID;
+
   static const unsigned GlobalValueSubClassDataBits = 19;
   unsigned getGlobalValueSubClassData() const {
     return SubClassData;

include/llvm/IR/Intrinsics.h

@@ -32,7 +32,7 @@ class AttributeSet;
 /// function known by LLVM. The enum values are returned by
 /// Function::getIntrinsicID().
 namespace Intrinsic {
-  enum ID {
+  enum ID : unsigned {
     not_intrinsic = 0,   // Must be zero
 
     // Get the intrinsic enums generated from Intrinsics.td

include/llvm/IR/Value.h

@@ -216,6 +216,7 @@ public:
 
 private:
   void destroyValueName();
+  void setNameImpl(const Twine &Name);
 
 public:
   /// \brief Return a constant reference to the value's name.

lib/IR/Function.cpp

@@ -266,9 +266,10 @@ Function::Function(FunctionType *Ty, LinkageTypes Linkage, const Twine &name,
     ParentModule->getFunctionList().push_back(this);
 
   // Ensure intrinsics have the right parameter attributes.
-  if (unsigned IID = getIntrinsicID())
-    setAttributes(Intrinsic::getAttributes(getContext(), Intrinsic::ID(IID)));
-
+  // Note, the IntID field will have been set in Value::setName if this function
+  // name is a valid intrinsic ID.
+  if (IntID)
+    setAttributes(Intrinsic::getAttributes(getContext(), IntID));
 }
 
 Function::~Function() {
@@ -280,10 +281,6 @@ Function::~Function() {
 
   // Remove the function from the on-the-side GC table.
   clearGC();
-
-  // Remove the intrinsicID from the Cache.
-  if (getValueName() && isIntrinsic())
-    getContext().pImpl->IntrinsicIDCache.erase(this);
 }
 
 void Function::BuildLazyArguments() const {
@@ -446,27 +443,13 @@ static Intrinsic::ID lookupIntrinsicID(const ValueName *ValName) {
   return Intrinsic::not_intrinsic;
 }
 
-/// getIntrinsicID - This method returns the ID number of the specified
-/// function, or Intrinsic::not_intrinsic if the function is not an
-/// intrinsic, or if the pointer is null.  This value is always defined to be
-/// zero to allow easy checking for whether a function is intrinsic or not.  The
-/// particular intrinsic functions which correspond to this value are defined in
-/// llvm/Intrinsics.h.  Results are cached in the LLVM context, subsequent
-/// requests for the same ID return results much faster from the cache.
-///
-unsigned Function::getIntrinsicID() const {
+void Function::recalculateIntrinsicID() {
   const ValueName *ValName = this->getValueName();
-  if (!ValName || !isIntrinsic())
-    return 0;
-
-  LLVMContextImpl::IntrinsicIDCacheTy &IntrinsicIDCache =
-    getContext().pImpl->IntrinsicIDCache;
-  if (!IntrinsicIDCache.count(this)) {
-    unsigned Id = lookupIntrinsicID(ValName);
-    IntrinsicIDCache[this]=Id;
-    return Id;
+  if (!ValName || !isIntrinsic()) {
+    IntID = Intrinsic::not_intrinsic;
+    return;
   }
-  return IntrinsicIDCache[this];
+  IntID = lookupIntrinsicID(ValName);
 }
 
 /// Returns a stable mangling for the type specified for use in the name

lib/IR/LLVMContextImpl.h

@@ -1000,11 +1000,6 @@ public:
   /// instructions in different blocks at the same location.
   DenseMap<std::pair<const char *, unsigned>, unsigned> DiscriminatorTable;
 
-  /// IntrinsicIDCache - Cache of intrinsic name (string) to numeric ID mappings
-  /// requested in this context
-  typedef DenseMap<const Function*, unsigned> IntrinsicIDCacheTy;
-  IntrinsicIDCacheTy IntrinsicIDCache;
-
   /// \brief Mapping from a function to its prefix data, which is stored as the
   /// operand of an unparented ReturnInst so that the prefix data has a Use.
   typedef DenseMap<const Function *, ReturnInst *> PrefixDataMapTy;

lib/IR/Value.cpp

@@ -166,7 +166,7 @@ StringRef Value::getName() const {
   return getValueName()->getKey();
 }
 
-void Value::setName(const Twine &NewName) {
+void Value::setNameImpl(const Twine &NewName) {
   // Fast path for common IRBuilder case of setName("") when there is no name.
   if (NewName.isTriviallyEmpty() && !hasName())
     return;
@@ -187,9 +187,6 @@ void Value::setName(const Twine &NewName) {
   if (getSymTab(this, ST))
     return;  // Cannot set a name on this value (e.g. constant).
 
-  if (Function *F = dyn_cast<Function>(this))
-    getContext().pImpl->IntrinsicIDCache.erase(F);
-
   if (!ST) { // No symbol table to update?  Just do the change.
     if (NameRef.empty()) {
       // Free the name for this value.
@@ -222,6 +219,12 @@ void Value::setName(const Twine &NewName) {
   setValueName(ST->createValueName(NameRef, this));
 }
 
+void Value::setName(const Twine &NewName) {
+  setNameImpl(NewName);
+  if (Function *F = dyn_cast<Function>(this))
+    F->recalculateIntrinsicID();
+}
+
 void Value::takeName(Value *V) {
   ValueSymbolTable *ST = nullptr;
   // If this value has a name, drop it.