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

git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@237642 91177308-0d34-0410-b5e6-96231b3b80d8

diff --git a/include/llvm/IR/Function.h b/include/llvm/IR/Function.h
index c084895fedf714ca94ecdb59b68bab8b87b373d7..955d77c0aefa88b4eaee0f8f0a6a14b62f3f11b5 100644 (file)
--- a/include/llvm/IR/Function.h
+++ b/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.

diff --git a/include/llvm/IR/GlobalValue.h b/include/llvm/IR/GlobalValue.h
index 6d46a465ee718f4ede8c3ddb74cce81434417ff6..92d531f198788d53d08cafff33830ff9f8ddeead 100644 (file)
--- a/include/llvm/IR/GlobalValue.h
+++ b/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;

diff --git a/include/llvm/IR/Intrinsics.h b/include/llvm/IR/Intrinsics.h
index a66bd2cd7d99352a4cf7ad8b1ac4dbdb2cb9aac6..e12ccace25cab386e411b345b8d03fafcc60c2f0 100644 (file)
--- a/include/llvm/IR/Intrinsics.h
+++ b/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

diff --git a/include/llvm/IR/Value.h b/include/llvm/IR/Value.h
index c03cb8852404583f518067c9e29bd7c33c194338..3bf2943a8980a612b9bc8a8bbc9fb74e666ea769 100644 (file)
--- a/include/llvm/IR/Value.h
+++ b/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.

diff --git a/lib/IR/Function.cpp b/lib/IR/Function.cpp
index 71e07ea04cb081ee03cea4f68d26e918cf70e7a6..cbba2ee90a1335d903fe3c5e9927759144d5ffce 100644 (file)
--- a/lib/IR/Function.cpp
+++ b/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

diff --git a/lib/IR/LLVMContextImpl.h b/lib/IR/LLVMContextImpl.h
index 09c28838fe8f63970c1686320b83d0113790cb4c..95292509f505b3bb228d838d673b8b6065ce666f 100644 (file)
--- a/lib/IR/LLVMContextImpl.h
+++ b/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;

diff --git a/lib/IR/Value.cpp b/lib/IR/Value.cpp
index 18f6b1e9c3596eaf0b43815bb1e796a27d9de3d6..fd0ed31ccc623482fd41325b87949711ccf18fec 100644 (file)
--- a/lib/IR/Value.cpp
+++ b/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.