Devirtualize Constant::destroyConstant.

This reorganizes destroyConstant and destroyConstantImpl.

Now there is only destroyConstant in Constant itself, while
subclasses are required to implement destroyConstantImpl.

destroyConstantImpl no longer calls delete but is instead only
responsible for removing the constant from any maps in which it
is contained.

Reviewed by Duncan Exon Smith.

llvm-svn: 240471

llvm/include/llvm/IR/Constant.h

@@ -47,7 +47,6 @@ protected:
   Constant(Type *ty, ValueTy vty, Use *Ops, unsigned NumOps)
     : User(ty, vty, Ops, NumOps) {}
 
-  void destroyConstantImpl();
   void replaceUsesOfWithOnConstantImpl(Constant *Replacement);
 
 public:
@@ -126,14 +125,14 @@ public:
   /// vector of constant integers, all equal, and the common value is returned.
   const APInt &getUniqueInteger() const;
 
-  /// destroyConstant - Called if some element of this constant is no longer
+  /// Called if some element of this constant is no longer valid.
-  /// valid.  At this point only other constants may be on the use_list for this
+  /// At this point only other constants may be on the use_list for this
   /// constant.  Any constants on our Use list must also be destroy'd.  The
   /// implementation must be sure to remove the constant from the list of
-  /// available cached constants.  Implementations should call
+  /// available cached constants.  Implementations should implement
-  /// destroyConstantImpl as the last thing they do, to destroy all users and
+  /// destroyConstantImpl to remove constants from any pools/maps they are
-  /// delete this.
+  /// contained it.
-  virtual void destroyConstant() { llvm_unreachable("Not reached!"); }
+  void destroyConstant();
 
   //// Methods for support type inquiry through isa, cast, and dyn_cast:
   static inline bool classof(const Value *V) {

llvm/include/llvm/IR/Constants.h

@@ -50,6 +50,10 @@ class ConstantInt : public Constant {
   ConstantInt(const ConstantInt &) = delete;
   ConstantInt(IntegerType *Ty, const APInt& V);
   APInt Val;
+
+  friend class Constant;
+  void destroyConstantImpl();
+
 protected:
   // allocate space for exactly zero operands
   void *operator new(size_t s) {
@@ -231,6 +235,10 @@ class ConstantFP : public Constant {
   void *operator new(size_t, unsigned) = delete;
   ConstantFP(const ConstantFP &) = delete;
   friend class LLVMContextImpl;
+
+  friend class Constant;
+  void destroyConstantImpl();
+
 protected:
   ConstantFP(Type *Ty, const APFloat& V);
 protected:
@@ -297,6 +305,10 @@ public:
 class ConstantAggregateZero : public Constant {
   void *operator new(size_t, unsigned) = delete;
   ConstantAggregateZero(const ConstantAggregateZero &) = delete;
+
+  friend class Constant;
+  void destroyConstantImpl();
+
 protected:
   explicit ConstantAggregateZero(Type *ty)
     : Constant(ty, ConstantAggregateZeroVal, nullptr, 0) {}
@@ -308,8 +320,6 @@ protected:
 public:
   static ConstantAggregateZero *get(Type *Ty);
 
-  void destroyConstant() override;
-
   /// getSequentialElement - If this CAZ has array or vector type, return a zero
   /// with the right element type.
   Constant *getSequentialElement() const;
@@ -343,6 +353,10 @@ public:
 class ConstantArray : public Constant {
   friend struct ConstantAggrKeyType<ConstantArray>;
   ConstantArray(const ConstantArray &) = delete;
+
+  friend class Constant;
+  void destroyConstantImpl();
+
 protected:
   ConstantArray(ArrayType *T, ArrayRef<Constant *> Val);
 public:
@@ -363,7 +377,6 @@ public:
     return cast<ArrayType>(Value::getType());
   }
 
-  void destroyConstant() override;
   void replaceUsesOfWithOnConstant(Value *From, Value *To, Use *U) override;
 
   /// Methods for support type inquiry through isa, cast, and dyn_cast:
@@ -385,6 +398,10 @@ DEFINE_TRANSPARENT_OPERAND_ACCESSORS(ConstantArray, Constant)
 class ConstantStruct : public Constant {
   friend struct ConstantAggrKeyType<ConstantStruct>;
   ConstantStruct(const ConstantStruct &) = delete;
+
+  friend class Constant;
+  void destroyConstantImpl();
+
 protected:
   ConstantStruct(StructType *T, ArrayRef<Constant *> Val);
 public:
@@ -421,7 +438,6 @@ public:
     return cast<StructType>(Value::getType());
   }
 
-  void destroyConstant() override;
   void replaceUsesOfWithOnConstant(Value *From, Value *To, Use *U) override;
 
   /// Methods for support type inquiry through isa, cast, and dyn_cast:
@@ -444,6 +460,10 @@ DEFINE_TRANSPARENT_OPERAND_ACCESSORS(ConstantStruct, Constant)
 class ConstantVector : public Constant {
   friend struct ConstantAggrKeyType<ConstantVector>;
   ConstantVector(const ConstantVector &) = delete;
+
+  friend class Constant;
+  void destroyConstantImpl();
+
 protected:
   ConstantVector(VectorType *T, ArrayRef<Constant *> Val);
 public:
@@ -472,7 +492,6 @@ public:
   /// elements have the same value, return that value. Otherwise return NULL.
   Constant *getSplatValue() const;
 
-  void destroyConstant() override;
   void replaceUsesOfWithOnConstant(Value *From, Value *To, Use *U) override;
 
   /// Methods for support type inquiry through isa, cast, and dyn_cast:
@@ -494,6 +513,10 @@ DEFINE_TRANSPARENT_OPERAND_ACCESSORS(ConstantVector, Constant)
 class ConstantPointerNull : public Constant {
   void *operator new(size_t, unsigned) = delete;
   ConstantPointerNull(const ConstantPointerNull &) = delete;
+
+  friend class Constant;
+  void destroyConstantImpl();
+
 protected:
   explicit ConstantPointerNull(PointerType *T)
     : Constant(T,
@@ -508,8 +531,6 @@ public:
   /// get() - Static factory methods - Return objects of the specified value
   static ConstantPointerNull *get(PointerType *T);
 
-  void destroyConstant() override;
-
   /// getType - Specialize the getType() method to always return an PointerType,
   /// which reduces the amount of casting needed in parts of the compiler.
   ///
@@ -545,6 +566,10 @@ class ConstantDataSequential : public Constant {
   ConstantDataSequential *Next;
   void *operator new(size_t, unsigned) = delete;
   ConstantDataSequential(const ConstantDataSequential &) = delete;
+
+  friend class Constant;
+  void destroyConstantImpl();
+
 protected:
   explicit ConstantDataSequential(Type *ty, ValueTy VT, const char *Data)
     : Constant(ty, VT, nullptr, 0), DataElements(Data), Next(nullptr) {}
@@ -635,8 +660,6 @@ public:
   /// host endianness of the data elements.
   StringRef getRawDataValues() const;
 
-  void destroyConstant() override;
-
   /// Methods for support type inquiry through isa, cast, and dyn_cast:
   ///
   static bool classof(const Value *V) {
@@ -778,6 +801,10 @@ class BlockAddress : public Constant {
   void *operator new(size_t, unsigned) = delete;
   void *operator new(size_t s) { return User::operator new(s, 2); }
   BlockAddress(Function *F, BasicBlock *BB);
+
+  friend class Constant;
+  void destroyConstantImpl();
+
 public:
   /// get - Return a BlockAddress for the specified function and basic block.
   static BlockAddress *get(Function *F, BasicBlock *BB);
@@ -798,7 +825,6 @@ public:
   Function *getFunction() const { return (Function*)Op<0>().get(); }
   BasicBlock *getBasicBlock() const { return (BasicBlock*)Op<1>().get(); }
 
-  void destroyConstant() override;
   void replaceUsesOfWithOnConstant(Value *From, Value *To, Use *U) override;
 
   /// Methods for support type inquiry through isa, cast, and dyn_cast:
@@ -825,6 +851,9 @@ DEFINE_TRANSPARENT_OPERAND_ACCESSORS(BlockAddress, Value)
 class ConstantExpr : public Constant {
   friend struct ConstantExprKeyType;
 
+  friend class Constant;
+  void destroyConstantImpl();
+
 protected:
   ConstantExpr(Type *ty, unsigned Opcode, Use *Ops, unsigned NumOps)
     : Constant(ty, ConstantExprVal, Ops, NumOps) {
@@ -1156,7 +1185,6 @@ public:
   /// would make it harder to remove ConstantExprs altogether.
   Instruction *getAsInstruction();
 
-  void destroyConstant() override;
   void replaceUsesOfWithOnConstant(Value *From, Value *To, Use *U) override;
 
   /// Methods for support type inquiry through isa, cast, and dyn_cast:
@@ -1192,6 +1220,10 @@ DEFINE_TRANSPARENT_OPERAND_ACCESSORS(ConstantExpr, Constant)
 class UndefValue : public Constant {
   void *operator new(size_t, unsigned) = delete;
   UndefValue(const UndefValue &) = delete;
+
+  friend class Constant;
+  void destroyConstantImpl();
+
 protected:
   explicit UndefValue(Type *T) : Constant(T, UndefValueVal, nullptr, 0) {}
 protected:
@@ -1224,8 +1256,6 @@ public:
   /// \brief Return the number of elements in the array, vector, or struct.
   unsigned getNumElements() const;
 
-  void destroyConstant() override;
-
   /// Methods for support type inquiry through isa, cast, and dyn_cast:
   static bool classof(const Value *V) {
     return V->getValueID() == UndefValueVal;

llvm/include/llvm/IR/GlobalValue.h

@@ -90,6 +90,9 @@ private:
   // (19 + 3 + 2 + 1 + 2 + 5) == 32.
   unsigned SubClassData : GlobalValueSubClassDataBits;
 
+  friend class Constant;
+  void destroyConstantImpl();
+
 protected:
   /// \brief The intrinsic ID for this subclass (which must be a Function).
   ///
@@ -334,9 +337,6 @@ public:
 
 /// @}
 
-  /// Override from Constant class.
-  void destroyConstant() override;
-
   /// Return true if the primary definition of this global value is outside of
   /// the current translation unit.
   bool isDeclaration() const;

llvm/lib/IR/Constants.cpp

@@ -276,8 +276,19 @@ Constant *Constant::getAggregateElement(Constant *Elt) const {
   return nullptr;
 }
 
+void Constant::destroyConstant() {
+  /// First call destroyConstantImpl on the subclass.  This gives the subclass
+  /// a chance to remove the constant from any maps/pools it's contained in.
+  switch (getValueID()) {
+  default:
+    llvm_unreachable("Not a constant!");
+#define HANDLE_CONSTANT(Name)                                                  \
+  case Value::Name##Val:                                                       \
+    cast<Name>(this)->destroyConstantImpl();                                   \
+    break;
+#include "llvm/IR/Value.def"
+  }
 
-void Constant::destroyConstantImpl() {
   // When a Constant is destroyed, there may be lingering
   // references to the constant by other constants in the constant pool.  These
   // constants are implicitly dependent on the module that is being deleted,
@@ -287,11 +298,11 @@ void Constant::destroyConstantImpl() {
   //
   while (!use_empty()) {
     Value *V = user_back();
-#ifndef NDEBUG      // Only in -g mode...
+#ifndef NDEBUG // Only in -g mode...
     if (!isa<Constant>(V)) {
       dbgs() << "While deleting: " << *this
-             << "\n\nUse still stuck around after Def is destroyed: "
+             << "\n\nUse still stuck around after Def is destroyed: " << *V
-             << *V << "\n\n";
+             << "\n\n";
     }
 #endif
     assert(isa<Constant>(V) && "References remain to Constant being destroyed");
@@ -608,6 +619,11 @@ ConstantInt *ConstantInt::get(IntegerType* Ty, StringRef Str,
   return get(Ty->getContext(), APInt(Ty->getBitWidth(), Str, radix));
 }
 
+/// Remove the constant from the constant table.
+void ConstantInt::destroyConstantImpl() {
+  llvm_unreachable("You can't ConstantInt->destroyConstantImpl()!");
+}
+
 //===----------------------------------------------------------------------===//
 //                                ConstantFP
 //===----------------------------------------------------------------------===//
@@ -743,6 +759,11 @@ bool ConstantFP::isExactlyValue(const APFloat &V) const {
   return Val.bitwiseIsEqual(V);
 }
 
+/// Remove the constant from the constant table.
+void ConstantFP::destroyConstantImpl() {
+  llvm_unreachable("You can't ConstantInt->destroyConstantImpl()!");
+}
+
 //===----------------------------------------------------------------------===//
 //                   ConstantAggregateZero Implementation
 //===----------------------------------------------------------------------===//
@@ -1366,16 +1387,14 @@ ConstantAggregateZero *ConstantAggregateZero::get(Type *Ty) {
 
 /// destroyConstant - Remove the constant from the constant table.
 ///
-void ConstantAggregateZero::destroyConstant() {
+void ConstantAggregateZero::destroyConstantImpl() {
   getContext().pImpl->CAZConstants.erase(getType());
-  destroyConstantImpl();
 }
 
 /// destroyConstant - Remove the constant from the constant table...
 ///
-void ConstantArray::destroyConstant() {
+void ConstantArray::destroyConstantImpl() {
   getType()->getContext().pImpl->ArrayConstants.remove(this);
-  destroyConstantImpl();
 }
 
 
@@ -1384,16 +1403,14 @@ void ConstantArray::destroyConstant() {
 
 // destroyConstant - Remove the constant from the constant table...
 //
-void ConstantStruct::destroyConstant() {
+void ConstantStruct::destroyConstantImpl() {
   getType()->getContext().pImpl->StructConstants.remove(this);
-  destroyConstantImpl();
 }
 
 // destroyConstant - Remove the constant from the constant table...
 //
-void ConstantVector::destroyConstant() {
+void ConstantVector::destroyConstantImpl() {
   getType()->getContext().pImpl->VectorConstants.remove(this);
-  destroyConstantImpl();
 }
 
 /// getSplatValue - If this is a splat vector constant, meaning that all of
@@ -1432,7 +1449,6 @@ const APInt &Constant::getUniqueInteger() const {
   return cast<ConstantInt>(C)->getValue();
 }
 
-
 //---- ConstantPointerNull::get() implementation.
 //
 
@@ -1446,10 +1462,8 @@ ConstantPointerNull *ConstantPointerNull::get(PointerType *Ty) {
 
 // destroyConstant - Remove the constant from the constant table...
 //
-void ConstantPointerNull::destroyConstant() {
+void ConstantPointerNull::destroyConstantImpl() {
   getContext().pImpl->CPNConstants.erase(getType());
-  // Free the constant and any dangling references to it.
-  destroyConstantImpl();
 }
 
 
@@ -1466,10 +1480,9 @@ UndefValue *UndefValue::get(Type *Ty) {
 
 // destroyConstant - Remove the constant from the constant table.
 //
-void UndefValue::destroyConstant() {
+void UndefValue::destroyConstantImpl() {
   // Free the constant and any dangling references to it.
   getContext().pImpl->UVConstants.erase(getType());
-  destroyConstantImpl();
 }
 
 //---- BlockAddress::get() implementation.
@@ -1512,11 +1525,10 @@ BlockAddress *BlockAddress::lookup(const BasicBlock *BB) {
 
 // destroyConstant - Remove the constant from the constant table.
 //
-void BlockAddress::destroyConstant() {
+void BlockAddress::destroyConstantImpl() {
   getFunction()->getType()->getContext().pImpl
     ->BlockAddresses.erase(std::make_pair(getFunction(), getBasicBlock()));
   getBasicBlock()->AdjustBlockAddressRefCount(-1);
-  destroyConstantImpl();
 }
 
 void BlockAddress::replaceUsesOfWithOnConstant(Value *From, Value *To, Use *U) {
@@ -2372,9 +2384,8 @@ Constant *ConstantExpr::getBinOpAbsorber(unsigned Opcode, Type *Ty) {
 
 // destroyConstant - Remove the constant from the constant table...
 //
-void ConstantExpr::destroyConstant() {
+void ConstantExpr::destroyConstantImpl() {
   getType()->getContext().pImpl->ExprConstants.remove(this);
-  destroyConstantImpl();
 }
 
 const char *ConstantExpr::getOpcodeName() const {
@@ -2496,7 +2507,7 @@ Constant *ConstantDataSequential::getImpl(StringRef Elements, Type *Ty) {
   return *Entry = new ConstantDataVector(Ty, Slot.first().data());
 }
 
-void ConstantDataSequential::destroyConstant() {
+void ConstantDataSequential::destroyConstantImpl() {
   // Remove the constant from the StringMap.
   StringMap<ConstantDataSequential*> &CDSConstants = 
     getType()->getContext().pImpl->CDSConstants;
@@ -2531,9 +2542,6 @@ void ConstantDataSequential::destroyConstant() {
   // If we were part of a list, make sure that we don't delete the list that is
   // still owned by the uniquing map.
   Next = nullptr;
-
-  // Finally, actually delete it.
-  destroyConstantImpl();
 }
 
 /// get() constructors - Return a constant with array type with an element

llvm/lib/IR/Globals.cpp

@@ -42,10 +42,10 @@ void GlobalValue::dematerialize() {
   getParent()->dematerialize(this);
 }
 
-/// Override destroyConstant to make sure it doesn't get called on
+/// Override destroyConstantImpl to make sure it doesn't get called on
 /// GlobalValue's because they shouldn't be treated like other constants.
-void GlobalValue::destroyConstant() {
+void GlobalValue::destroyConstantImpl() {
-  llvm_unreachable("You can't GV->destroyConstant()!");
+  llvm_unreachable("You can't GV->destroyConstantImpl()!");
 }
 
 /// copyAttributesFrom - copy all additional attributes (those not needed to