CodeGen: Clean up implementation of vtable initializer builder. NFC.

[pcc]

Summary

• Simplify signature of CreateVTableInitializer function.
• Move vtable component builder to a separate function.
• Remove unnecessary accessors from VTableLayout class.

This is in preparation for a future change that will alter the type of the
vtable initializer.

[pcc]

• Refresh

[rsmith]

include/clang/AST/VTableBuilder.h

222–225

Can you remove these? It looks like they should be unused now.

lib/CodeGen/CGVTables.cpp

527–528

This is just CGM.PtrDiffTy.

588

Do you have any idea why we're doing this? It looks wrong to me. These ABI entry points are exposed and could certainly have their addresses taken and used in this translation unit.

598–615

Please remove the else-after-return throughout here. That'll make it much more obvious to a reader of the code that fallthrough to the next case is not possible.

lib/CodeGen/CGVTables.h

52

I'd prefer these to be two separate declarations, with doc comments.

[pcc]

lib/CodeGen/CGVTables.cpp

588

I introduced this in D18071. Although a translation unit can take the address of one of these functions, that would involve declaring a function with a reserved name, so I believe we'd be allowed to impose restrictions such as unnamed_addr on the address.

include/clang/AST/VTableBuilder.h

private:
   /// need the offsets to be 64-bit, even when on a 32-bit machine).
   int64_t Value;
 };
 
 class VTableLayout {
 public:
   typedef std::pair<uint64_t, ThunkInfo> VTableThunkTy;
 
   typedef const VTableComponent *vtable_component_iterator;
   typedef const VTableThunkTy *vtable_thunk_iterator;
   typedef llvm::iterator_range<vtable_component_iterator>
       vtable_component_range;
 
   typedef llvm::DenseMap<BaseSubobject, uint64_t> AddressPointsMapTy;
 
 private:
   uint64_t NumVTableComponents;
   std::unique_ptr<VTableComponent[]> VTableComponents;
 
   /// \brief Contains thunks needed by vtables, sorted by indices.
public:
   VTableLayout(uint64_t NumVTableComponents,
                const VTableComponent *VTableComponents,
                uint64_t NumVTableThunks,
                const VTableThunkTy *VTableThunks,
                const AddressPointsMapTy &AddressPoints,
                bool IsMicrosoftABI);
   ~VTableLayout();
 
-  uint64_t getNumVTableComponents() const {
-    return NumVTableComponents;
+  ArrayRef<VTableComponent> vtable_components() const {
+    return {VTableComponents.get(), NumVTableComponents};
   }
 
-  vtable_component_range vtable_components() const {
-    return vtable_component_range(vtable_component_begin(),
-                                  vtable_component_end());
-  }
-
-  vtable_component_iterator vtable_component_begin() const {
-    return VTableComponents.get();
-  }
-
-  vtable_component_iterator vtable_component_end() const {
-    return VTableComponents.get() + NumVTableComponents;
-  }
-
-  uint64_t getNumVTableThunks() const { return NumVTableThunks; }
-
-  vtable_thunk_iterator vtable_thunk_begin() const {
-    return VTableThunks.get();
-  }
-
-  vtable_thunk_iterator vtable_thunk_end() const {
-    return VTableThunks.get() + NumVTableThunks;
+  ArrayRef<VTableThunkTy> vtable_thunks() const {
+    return {VTableThunks.get(), NumVTableThunks};
   }
 
   uint64_t getAddressPoint(BaseSubobject Base) const {
     assert(AddressPoints.count(Base) &&
            "Did not find address point!");
 
     uint64_t AddressPoint = AddressPoints.lookup(Base);
     assert(AddressPoint != 0 || IsMicrosoftABI);

lib/CodeGen/CGDebugInfo.cpp

void CGDebugInfo::CollectVTableInfo(const CXXRecordDecl *RD, llvm::DIFile *Unit,
   bool NeedVTableShape = CGM.getCodeGenOpts().EmitCodeView &&
                          CGM.getTarget().getCXXABI().isMicrosoft();
   if (NeedVTableShape) {
     uint64_t PtrWidth =
         CGM.getContext().getTypeSize(CGM.getContext().VoidPtrTy);
     const VTableLayout &VFTLayout =
         CGM.getMicrosoftVTableContext().getVFTableLayout(RD, CharUnits::Zero());
     unsigned VSlotCount =
-        VFTLayout.getNumVTableComponents() - CGM.getLangOpts().RTTIData;
+        VFTLayout.vtable_components().size() - CGM.getLangOpts().RTTIData;
     unsigned VTableWidth = PtrWidth * VSlotCount;
 
     // Create a very wide void* type and insert it directly in the element list.
     llvm::DIType *VTableType =
         DBuilder.createPointerType(nullptr, VTableWidth, 0, "__vtbl_ptr_type");
     EltTys.push_back(VTableType);
 
     // The vptr is a pointer to this special vtable type.

lib/CodeGen/CGVTables.h

class CodeGenVTables {
 
   typedef llvm::DenseMap<BaseSubobjectPairTy, uint64_t>
     SecondaryVirtualPointerIndicesMapTy;
 
   /// SecondaryVirtualPointerIndices - Contains the secondary virtual pointer
   /// indices.
   SecondaryVirtualPointerIndicesMapTy SecondaryVirtualPointerIndices;
 
+  llvm::Constant *PureVirtualFn = nullptr, *DeletedVirtualFn = nullptr;
+
   /// emitThunk - Emit a single thunk.
   void emitThunk(GlobalDecl GD, const ThunkInfo &Thunk, bool ForVTable);
 
   /// maybeEmitThunkForVTable - Emit the given thunk for the vtable if needed by
   /// the ABI.
   void maybeEmitThunkForVTable(GlobalDecl GD, const ThunkInfo &Thunk);
 
+  llvm::Constant *CreateVTableComponent(unsigned Idx,
+                                        const VTableLayout &VTLayout,
+                                        llvm::Constant *RTTI,
+                                        unsigned &NextVTableThunkIndex);
+
 public:
   /// CreateVTableInitializer - Create a vtable initializer for the given record
   /// decl.
   /// \param Components - The vtable components; this is really an array of
   /// VTableComponents.
-  llvm::Constant *CreateVTableInitializer(
-      const CXXRecordDecl *RD, const VTableComponent *Components,
-      unsigned NumComponents, const VTableLayout::VTableThunkTy *VTableThunks,
-      unsigned NumVTableThunks, llvm::Constant *RTTI);
+  llvm::Constant *CreateVTableInitializer(const VTableLayout &VTLayout,
+                                          llvm::Constant *RTTI);
 
   CodeGenVTables(CodeGenModule &CGM);
 
   ItaniumVTableContext &getItaniumVTableContext() {
     return *cast<ItaniumVTableContext>(VTContext);
   }
 
   MicrosoftVTableContext &getMicrosoftVTableContext() {

lib/CodeGen/CGVTables.cpp

void CodeGenVTables::EmitThunks(GlobalDecl GD)
 
   if (!ThunkInfoVector)
     return;
 
   for (const ThunkInfo& Thunk : *ThunkInfoVector)
     emitThunk(GD, Thunk, /*ForVTable=*/false);
 }
 
-llvm::Constant *CodeGenVTables::CreateVTableInitializer(
-    const CXXRecordDecl *RD, const VTableComponent *Components,
-    unsigned NumComponents, const VTableLayout::VTableThunkTy *VTableThunks,
-    unsigned NumVTableThunks, llvm::Constant *RTTI) {
-  SmallVector<llvm::Constant *, 64> Inits;
-
+llvm::Constant *CodeGenVTables::CreateVTableComponent(
+    unsigned Idx, const VTableLayout &VTLayout, llvm::Constant *RTTI,
+    unsigned &NextVTableThunkIndex) {
+  VTableComponent Component = VTLayout.vtable_components()[Idx];
   llvm::Type *Int8PtrTy = CGM.Int8PtrTy;
-  
-  llvm::Type *PtrDiffTy = 
+  auto OffsetConstant = [&](CharUnits Offset) {
+    llvm::Type *PtrDiffTy =
         CGM.getTypes().ConvertType(CGM.getContext().getPointerDiffType());
-
-  unsigned NextVTableThunkIndex = 0;
-
-  llvm::Constant *PureVirtualFn = nullptr, *DeletedVirtualFn = nullptr;
-
-  for (unsigned I = 0; I != NumComponents; ++I) {
-    VTableComponent Component = Components[I];
-
-    llvm::Constant *Init = nullptr;
+    return llvm::ConstantExpr::getIntToPtr(
+        llvm::ConstantInt::get(PtrDiffTy, Offset.getQuantity()), Int8PtrTy);
+  };
 
-    switch (Component.getKind()) {
-    case VTableComponent::CK_VCallOffset:
-      Init = llvm::ConstantInt::get(PtrDiffTy, 
-                                    Component.getVCallOffset().getQuantity());
-      Init = llvm::ConstantExpr::getIntToPtr(Init, Int8PtrTy);
-      break;
-    case VTableComponent::CK_VBaseOffset:
-      Init = llvm::ConstantInt::get(PtrDiffTy, 
-                                    Component.getVBaseOffset().getQuantity());
-      Init = llvm::ConstantExpr::getIntToPtr(Init, Int8PtrTy);
-      break;
-    case VTableComponent::CK_OffsetToTop:
-      Init = llvm::ConstantInt::get(PtrDiffTy, 
-                                    Component.getOffsetToTop().getQuantity());
-      Init = llvm::ConstantExpr::getIntToPtr(Init, Int8PtrTy);
-      break;
-    case VTableComponent::CK_RTTI:
-      Init = llvm::ConstantExpr::getBitCast(RTTI, Int8PtrTy);
-      break;
-    case VTableComponent::CK_FunctionPointer:
-    case VTableComponent::CK_CompleteDtorPointer:
-    case VTableComponent::CK_DeletingDtorPointer: {
-      GlobalDecl GD;
-      
-      // Get the right global decl.
-      switch (Component.getKind()) {
-      default:
-        llvm_unreachable("Unexpected vtable component kind");
-      case VTableComponent::CK_FunctionPointer:
-        GD = Component.getFunctionDecl();
-        break;
-      case VTableComponent::CK_CompleteDtorPointer:
-        GD = GlobalDecl(Component.getDestructorDecl(), Dtor_Complete);
-        break;
-      case VTableComponent::CK_DeletingDtorPointer:
-        GD = GlobalDecl(Component.getDestructorDecl(), Dtor_Deleting);
-        break;
-      }
+    return OffsetConstant(Component.getVCallOffset());
+
+    return OffsetConstant(Component.getVBaseOffset());
+
+    return OffsetConstant(Component.getOffsetToTop());
+
+    return RTTI;
+
 
-      if (CGM.getLangOpts().CUDA) {
-        // Emit NULL for methods we can't codegen on this
-        // side. Otherwise we'd end up with vtable with unresolved
-        // references.
-        const CXXMethodDecl *MD = cast<CXXMethodDecl>(GD.getDecl());
-        // OK on device side: functions w/ __device__ attribute
-        // OK on host side: anything except __device__-only functions.
-        bool CanEmitMethod = CGM.getLangOpts().CUDAIsDevice
-                                 ? MD->hasAttr<CUDADeviceAttr>()
-                                 : (MD->hasAttr<CUDAHostAttr>() ||
-                                    !MD->hasAttr<CUDADeviceAttr>());
-        if (!CanEmitMethod) {
-          Init = llvm::ConstantExpr::getNullValue(Int8PtrTy);
-          break;
-        }
-        // Method is acceptable, continue processing as usual.
-      }
+      bool CanEmitMethod =
+          CGM.getLangOpts().CUDAIsDevice
+              : (MD->hasAttr<CUDAHostAttr>() || !MD->hasAttr<CUDADeviceAttr>());
+      if (!CanEmitMethod)
+        return llvm::ConstantExpr::getNullValue(Int8PtrTy);
 
-      if (cast<CXXMethodDecl>(GD.getDecl())->isPure()) {
-        // We have a pure virtual member function.
-        if (!PureVirtualFn) {
-          llvm::FunctionType *Ty = 
+    auto SpecialVirtualFn = [&](llvm::Constant *&Cache, StringRef Name) {
+      if (!Cache) {
+        llvm::FunctionType *Ty =
             llvm::FunctionType::get(CGM.VoidTy, /*isVarArg=*/false);
-          StringRef PureCallName = CGM.getCXXABI().GetPureVirtualCallName();
-          PureVirtualFn = CGM.CreateRuntimeFunction(Ty, PureCallName);
-          if (auto *F = dyn_cast<llvm::Function>(PureVirtualFn))
-            F->setUnnamedAddr(llvm::GlobalValue::UnnamedAddr::Global);
-          PureVirtualFn = llvm::ConstantExpr::getBitCast(PureVirtualFn,
-                                                         CGM.Int8PtrTy);
-        }
-        Init = PureVirtualFn;
-      } else if (cast<CXXMethodDecl>(GD.getDecl())->isDeleted()) {
-        if (!DeletedVirtualFn) {
-          llvm::FunctionType *Ty =
-            llvm::FunctionType::get(CGM.VoidTy, /*isVarArg=*/false);
-          StringRef DeletedCallName =
-            CGM.getCXXABI().GetDeletedVirtualCallName();
-          DeletedVirtualFn = CGM.CreateRuntimeFunction(Ty, DeletedCallName);
-          if (auto *F = dyn_cast<llvm::Function>(DeletedVirtualFn))
-            F->setUnnamedAddr(llvm::GlobalValue::UnnamedAddr::Global);
-          DeletedVirtualFn = llvm::ConstantExpr::getBitCast(DeletedVirtualFn,
-                                                         CGM.Int8PtrTy);
-        }
-        Init = DeletedVirtualFn;
-      } else {
-        // Check if we should use a thunk.
-        if (NextVTableThunkIndex < NumVTableThunks &&
-            VTableThunks[NextVTableThunkIndex].first == I) {
-          const ThunkInfo &Thunk = VTableThunks[NextVTableThunkIndex].second;
-        
-          maybeEmitThunkForVTable(GD, Thunk);
-          Init = CGM.GetAddrOfThunk(GD, Thunk);
-
-          NextVTableThunkIndex++;
-        } else {
-          llvm::Type *Ty = CGM.getTypes().GetFunctionTypeForVTable(GD);
-        
-          Init = CGM.GetAddrOfFunction(GD, Ty, /*ForVTable=*/true);
-        }
-
-        Init = llvm::ConstantExpr::getBitCast(Init, Int8PtrTy);
-      }
-      break;
-    }
+        Cache = CGM.CreateRuntimeFunction(Ty, Name);
+        if (auto *F = dyn_cast<llvm::Function>(Cache))
+        Cache = llvm::ConstantExpr::getBitCast(Cache, CGM.Int8PtrTy);
+      return Cache;
+    };
+
+    if (cast<CXXMethodDecl>(GD.getDecl())->isPure()) {
+      // We have a pure virtual member function.
+      return SpecialVirtualFn(PureVirtualFn,
+                              CGM.getCXXABI().GetPureVirtualCallName());
+      return SpecialVirtualFn(DeletedVirtualFn,
+                              CGM.getCXXABI().GetDeletedVirtualCallName());
+      if (NextVTableThunkIndex < VTLayout.vtable_thunks().size() &&
+          VTLayout.vtable_thunks()[NextVTableThunkIndex].first == Idx) {
+        const ThunkInfo &Thunk =
+            VTLayout.vtable_thunks()[NextVTableThunkIndex].second;
+        return CGM.GetAddrOfThunk(GD, Thunk);
+        return CGM.GetAddrOfFunction(GD, Ty, /*ForVTable=*/true);
 
-    case VTableComponent::CK_UnusedFunctionPointer:
-      Init = llvm::ConstantExpr::getNullValue(Int8PtrTy);
-      break;
-    };
-    
-    Inits.push_back(Init);
+    return llvm::ConstantExpr::getNullValue(Int8PtrTy);
+  }
+}
+llvm::Constant *
+CodeGenVTables::CreateVTableInitializer(const VTableLayout &VTLayout,
+                                        llvm::Constant *RTTI) {
+  SmallVector<llvm::Constant *, 64> Inits;
+  unsigned NextVTableThunkIndex = 0;
+
+  for (unsigned I = 0, E = VTLayout.vtable_components().size(); I != E; ++I) {
+    llvm::Constant *Init =
+        CreateVTableComponent(I, VTLayout, RTTI, NextVTableThunkIndex);
+    Inits.push_back(llvm::ConstantExpr::getBitCast(Init, CGM.Int8PtrTy));
   }
-  
-  llvm::ArrayType *ArrayType = llvm::ArrayType::get(Int8PtrTy, NumComponents);
+  llvm::ArrayType *ArrayType =
+      llvm::ArrayType::get(CGM.Int8PtrTy, VTLayout.vtable_components().size());
   return llvm::ConstantArray::get(ArrayType, Inits);
 }
 
 llvm::GlobalVariable *
 CodeGenVTables::GenerateConstructionVTable(const CXXRecordDecl *RD, 
                                       const BaseSubobject &Base, 
                                       bool BaseIsVirtual, 
                                    llvm::GlobalVariable::LinkageTypes Linkage,
CodeGenVTables::GenerateConstructionVTable(const CXXRecordDecl *RD,
   // Get the mangled construction vtable name.
   SmallString<256> OutName;
   llvm::raw_svector_ostream Out(OutName);
   cast<ItaniumMangleContext>(CGM.getCXXABI().getMangleContext())
       .mangleCXXCtorVTable(RD, Base.getBaseOffset().getQuantity(),
                            Base.getBase(), Out);
   StringRef Name = OutName.str();
 
-  llvm::ArrayType *ArrayType = 
-    llvm::ArrayType::get(CGM.Int8PtrTy, VTLayout->getNumVTableComponents());
+  llvm::ArrayType *ArrayType =
+      llvm::ArrayType::get(CGM.Int8PtrTy, VTLayout->vtable_components().size());
 
   // Construction vtable symbols are not part of the Itanium ABI, so we cannot
   // guarantee that they actually will be available externally. Instead, when
   // emitting an available_externally VTT, we provide references to an internal
   // linkage construction vtable. The ABI only requires complete-object vtables
   // to be the same for all instances of a type, not construction vtables.
   if (Linkage == llvm::GlobalVariable::AvailableExternallyLinkage)
     Linkage = llvm::GlobalVariable::InternalLinkage;
 
   // Create the variable that will hold the construction vtable.
   llvm::GlobalVariable *VTable = 
     CGM.CreateOrReplaceCXXRuntimeVariable(Name, ArrayType, Linkage);
   CGM.setGlobalVisibility(VTable, RD);
 
   // V-tables are always unnamed_addr.
   VTable->setUnnamedAddr(llvm::GlobalValue::UnnamedAddr::Global);
 
   llvm::Constant *RTTI = CGM.GetAddrOfRTTIDescriptor(
       CGM.getContext().getTagDeclType(Base.getBase()));
 
   // Create and set the initializer.
-  llvm::Constant *Init = CreateVTableInitializer(
-      Base.getBase(), VTLayout->vtable_component_begin(),
-      VTLayout->getNumVTableComponents(), VTLayout->vtable_thunk_begin(),
-      VTLayout->getNumVTableThunks(), RTTI);
+  llvm::Constant *Init = CreateVTableInitializer(*VTLayout, RTTI);
   VTable->setInitializer(Init);
   
   CGM.EmitVTableTypeMetadata(VTable, *VTLayout.get());
 
   return VTable;
 }
 
 static bool shouldEmitAvailableExternallyVTable(const CodeGenModule &CGM,

lib/CodeGen/ItaniumCXXABI.cpp

void ItaniumCXXABI::emitVTableDefinitions(CodeGenVTables &CGVT,
 
   ItaniumVTableContext &VTContext = CGM.getItaniumVTableContext();
   const VTableLayout &VTLayout = VTContext.getVTableLayout(RD);
   llvm::GlobalVariable::LinkageTypes Linkage = CGM.getVTableLinkage(RD);
   llvm::Constant *RTTI =
       CGM.GetAddrOfRTTIDescriptor(CGM.getContext().getTagDeclType(RD));
 
   // Create and set the initializer.
-  llvm::Constant *Init = CGVT.CreateVTableInitializer(
-      RD, VTLayout.vtable_component_begin(), VTLayout.getNumVTableComponents(),
-      VTLayout.vtable_thunk_begin(), VTLayout.getNumVTableThunks(), RTTI);
+  llvm::Constant *Init = CGVT.CreateVTableInitializer(VTLayout, RTTI);
   VTable->setInitializer(Init);
 
   // Set the correct linkage.
   VTable->setLinkage(Linkage);
 
   if (CGM.supportsCOMDAT() && VTable->isWeakForLinker())
     VTable->setComdat(CGM.getModule().getOrInsertComdat(VTable->getName()));
 
llvm::GlobalVariable *ItaniumCXXABI::getAddrOfVTable(const CXXRecordDecl *RD,
   CGM.addDeferredVTable(RD);
 
   SmallString<256> Name;
   llvm::raw_svector_ostream Out(Name);
   getMangleContext().mangleCXXVTable(RD, Out);
 
   ItaniumVTableContext &VTContext = CGM.getItaniumVTableContext();
   llvm::ArrayType *ArrayType = llvm::ArrayType::get(
-      CGM.Int8PtrTy, VTContext.getVTableLayout(RD).getNumVTableComponents());
+      CGM.Int8PtrTy, VTContext.getVTableLayout(RD).vtable_components().size());
 
   VTable = CGM.CreateOrReplaceCXXRuntimeVariable(
       Name, ArrayType, llvm::GlobalValue::ExternalLinkage);
   VTable->setUnnamedAddr(llvm::GlobalValue::UnnamedAddr::Global);
 
   if (RD->hasAttr<DLLImportAttr>())
     VTable->setDLLStorageClass(llvm::GlobalValue::DLLImportStorageClass);
   else if (RD->hasAttr<DLLExportAttr>())

lib/CodeGen/MicrosoftCXXABI.cpp

for (VPtrInfo *Info : VFPtrs) {
     const VTableLayout &VTLayout =
       VFTContext.getVFTableLayout(RD, Info->FullOffsetInMDC);
 
     llvm::Constant *RTTI = nullptr;
     if (any_of(VTLayout.vtable_components(),
                [](const VTableComponent &VTC) { return VTC.isRTTIKind(); }))
       RTTI = getMSCompleteObjectLocator(RD, Info);
 
-    llvm::Constant *Init = CGVT.CreateVTableInitializer(
-        RD, VTLayout.vtable_component_begin(),
-        VTLayout.getNumVTableComponents(), VTLayout.vtable_thunk_begin(),
-        VTLayout.getNumVTableThunks(), RTTI);
+    llvm::Constant *Init = CGVT.CreateVTableInitializer(VTLayout, RTTI);
 
     VTable->setInitializer(Init);
 
     emitVTableTypeMetadata(Info, RD, VTable);
   }
 }
 
 bool MicrosoftCXXABI::isVirtualOffsetNeededForVTableField(
VTable = VTableAliasIsRequred
                  ? cast<llvm::GlobalVariable>(
                        cast<llvm::GlobalAlias>(VFTable)->getBaseObject())
                  : cast<llvm::GlobalVariable>(VFTable);
     return VTable;
   }
 
   uint64_t NumVTableSlots =
       VTContext.getVFTableLayout(RD, VFPtr->FullOffsetInMDC)
-          .getNumVTableComponents();
+          .vtable_components()
+          .size();
   llvm::GlobalValue::LinkageTypes VTableLinkage =
       VTableAliasIsRequred ? llvm::GlobalValue::PrivateLinkage : VFTableLinkage;
 
   StringRef VTableName = VTableAliasIsRequred ? StringRef() : VFTableName.str();
 
   llvm::ArrayType *VTableType =
       llvm::ArrayType::get(CGM.Int8PtrTy, NumVTableSlots);