Skip to content

Conversation

@PiJoules
Copy link
Contributor

@PiJoules PiJoules commented May 9, 2025

We have 3 different getters to get the vtable component type. This consolidates them into just the one in LangOpts.

We have 3 different getters to get the vtable component type. This
consolidates them into just the one in LangOpts.
@PiJoules PiJoules requested review from pcc and petrhosek May 9, 2025 19:13
@llvmbot llvmbot added clang Clang issues not falling into any other category clang:frontend Language frontend issues, e.g. anything involving "Sema" clang:codegen IR generation bugs: mangling, exceptions, etc. labels May 9, 2025
@PiJoules
Copy link
Contributor Author

PiJoules commented May 9, 2025

This is a followup to the comment in #126785

@llvmbot
Copy link
Member

llvmbot commented May 9, 2025

@llvm/pr-subscribers-clang-codegen

@llvm/pr-subscribers-clang

Author: None (PiJoules)

Changes

We have 3 different getters to get the vtable component type. This consolidates them into just the one in LangOpts.


Full diff: https://github.com/llvm/llvm-project/pull/139315.diff

7 Files Affected:

  • (modified) clang/include/clang/AST/VTableBuilder.h (+1-21)
  • (modified) clang/lib/AST/ASTContext.cpp (+1-4)
  • (modified) clang/lib/AST/VTableBuilder.cpp (+11-16)
  • (modified) clang/lib/CodeGen/CGClass.cpp (+2-2)
  • (modified) clang/lib/CodeGen/CGVTables.cpp (+9-17)
  • (modified) clang/lib/CodeGen/CGVTables.h (-3)
  • (modified) clang/lib/CodeGen/ItaniumCXXABI.cpp (+11-11)
diff --git a/clang/include/clang/AST/VTableBuilder.h b/clang/include/clang/AST/VTableBuilder.h
index a5de41dbc22f1..cd55a89449e47 100644
--- a/clang/include/clang/AST/VTableBuilder.h
+++ b/clang/include/clang/AST/VTableBuilder.h
@@ -395,17 +395,7 @@ class ItaniumVTableContext : public VTableContextBase {
   void computeVTableRelatedInformation(const CXXRecordDecl *RD) override;
 
 public:
-  enum VTableComponentLayout {
-    /// Components in the vtable are pointers to other structs/functions.
-    Pointer,
-
-    /// Components in the vtable are relative offsets between the vtable and the
-    /// other structs/functions.
-    Relative,
-  };
-
-  ItaniumVTableContext(ASTContext &Context,
-                       VTableComponentLayout ComponentLayout = Pointer);
+  ItaniumVTableContext(ASTContext &Context);
   ~ItaniumVTableContext() override;
 
   const VTableLayout &getVTableLayout(const CXXRecordDecl *RD) {
@@ -457,16 +447,6 @@ class ItaniumVTableContext : public VTableContextBase {
   static bool classof(const VTableContextBase *VT) {
     return !VT->isMicrosoft();
   }
-
-  VTableComponentLayout getVTableComponentLayout() const {
-    return ComponentLayout;
-  }
-
-  bool isPointerLayout() const { return ComponentLayout == Pointer; }
-  bool isRelativeLayout() const { return ComponentLayout == Relative; }
-
-private:
-  VTableComponentLayout ComponentLayout;
 };
 
 /// Holds information about the inheritance path to a virtual base or function
diff --git a/clang/lib/AST/ASTContext.cpp b/clang/lib/AST/ASTContext.cpp
index 0ace0a55afd7a..578798cedf8ff 100644
--- a/clang/lib/AST/ASTContext.cpp
+++ b/clang/lib/AST/ASTContext.cpp
@@ -13068,10 +13068,7 @@ VTableContextBase *ASTContext::getVTableContext() {
     if (ABI.isMicrosoft())
       VTContext.reset(new MicrosoftVTableContext(*this));
     else {
-      auto ComponentLayout = getLangOpts().RelativeCXXABIVTables
-                                 ? ItaniumVTableContext::Relative
-                                 : ItaniumVTableContext::Pointer;
-      VTContext.reset(new ItaniumVTableContext(*this, ComponentLayout));
+      VTContext.reset(new ItaniumVTableContext(*this));
     }
   }
   return VTContext.get();
diff --git a/clang/lib/AST/VTableBuilder.cpp b/clang/lib/AST/VTableBuilder.cpp
index 6c97b8718c65e..2f839a4779acb 100644
--- a/clang/lib/AST/VTableBuilder.cpp
+++ b/clang/lib/AST/VTableBuilder.cpp
@@ -536,8 +536,6 @@ class VCallAndVBaseOffsetBuilder {
     VBaseOffsetOffsetsMapTy;
 
 private:
-  const ItaniumVTableContext &VTables;
-
   /// MostDerivedClass - The most derived class for which we're building vcall
   /// and vbase offsets.
   const CXXRecordDecl *MostDerivedClass;
@@ -586,15 +584,13 @@ class VCallAndVBaseOffsetBuilder {
   CharUnits getCurrentOffsetOffset() const;
 
 public:
-  VCallAndVBaseOffsetBuilder(const ItaniumVTableContext &VTables,
-                             const CXXRecordDecl *MostDerivedClass,
+  VCallAndVBaseOffsetBuilder(const CXXRecordDecl *MostDerivedClass,
                              const CXXRecordDecl *LayoutClass,
                              const FinalOverriders *Overriders,
                              BaseSubobject Base, bool BaseIsVirtual,
                              CharUnits OffsetInLayoutClass)
-      : VTables(VTables), MostDerivedClass(MostDerivedClass),
-        LayoutClass(LayoutClass), Context(MostDerivedClass->getASTContext()),
-        Overriders(Overriders) {
+      : MostDerivedClass(MostDerivedClass), LayoutClass(LayoutClass),
+        Context(MostDerivedClass->getASTContext()), Overriders(Overriders) {
 
     // Add vcall and vbase offsets.
     AddVCallAndVBaseOffsets(Base, BaseIsVirtual, OffsetInLayoutClass);
@@ -674,7 +670,7 @@ CharUnits VCallAndVBaseOffsetBuilder::getCurrentOffsetOffset() const {
   // Under the relative ABI, the offset widths are 32-bit ints instead of
   // pointer widths.
   CharUnits OffsetWidth = Context.toCharUnitsFromBits(
-      VTables.isRelativeLayout()
+      Context.getLangOpts().RelativeCXXABIVTables
           ? 32
           : Context.getTargetInfo().getPointerWidth(LangAS::Default));
   CharUnits OffsetOffset = OffsetWidth * OffsetIndex;
@@ -1317,7 +1313,7 @@ ThisAdjustment ItaniumVTableBuilder::ComputeThisAdjustment(
       // We don't have vcall offsets for this virtual base, go ahead and
       // build them.
       VCallAndVBaseOffsetBuilder Builder(
-          VTables, MostDerivedClass, MostDerivedClass,
+          MostDerivedClass, MostDerivedClass,
           /*Overriders=*/nullptr,
           BaseSubobject(Offset.VirtualBase, CharUnits::Zero()),
           /*BaseIsVirtual=*/true,
@@ -1694,9 +1690,9 @@ void ItaniumVTableBuilder::LayoutPrimaryAndSecondaryVTables(
   VTableIndices.push_back(VTableIndex);
 
   // Add vcall and vbase offsets for this vtable.
-  VCallAndVBaseOffsetBuilder Builder(
-      VTables, MostDerivedClass, LayoutClass, &Overriders, Base,
-      BaseIsVirtualInLayoutClass, OffsetInLayoutClass);
+  VCallAndVBaseOffsetBuilder Builder(MostDerivedClass, LayoutClass, &Overriders,
+                                     Base, BaseIsVirtualInLayoutClass,
+                                     OffsetInLayoutClass);
   Components.append(Builder.components_begin(), Builder.components_end());
 
   // Check if we need to add these vcall offsets.
@@ -2331,9 +2327,8 @@ bool VTableContextBase::hasVtableSlot(const CXXMethodDecl *MD) {
   return MD->isVirtual() && !MD->isImmediateFunction();
 }
 
-ItaniumVTableContext::ItaniumVTableContext(
-    ASTContext &Context, VTableComponentLayout ComponentLayout)
-    : VTableContextBase(/*MS=*/false), ComponentLayout(ComponentLayout) {}
+ItaniumVTableContext::ItaniumVTableContext(ASTContext &Context)
+    : VTableContextBase(/*MS=*/false) {}
 
 ItaniumVTableContext::~ItaniumVTableContext() {}
 
@@ -2362,7 +2357,7 @@ ItaniumVTableContext::getVirtualBaseOffsetOffset(const CXXRecordDecl *RD,
   if (I != VirtualBaseClassOffsetOffsets.end())
     return I->second;
 
-  VCallAndVBaseOffsetBuilder Builder(*this, RD, RD, /*Overriders=*/nullptr,
+  VCallAndVBaseOffsetBuilder Builder(RD, RD, /*Overriders=*/nullptr,
                                      BaseSubobject(RD, CharUnits::Zero()),
                                      /*BaseIsVirtual=*/false,
                                      /*OffsetInLayoutClass=*/CharUnits::Zero());
diff --git a/clang/lib/CodeGen/CGClass.cpp b/clang/lib/CodeGen/CGClass.cpp
index befbfc64a680c..7ac4ca152930e 100644
--- a/clang/lib/CodeGen/CGClass.cpp
+++ b/clang/lib/CodeGen/CGClass.cpp
@@ -254,7 +254,7 @@ ApplyNonVirtualAndVirtualOffset(CodeGenFunction &CGF, Address addr,
   if (!nonVirtualOffset.isZero()) {
     llvm::Type *OffsetType =
         (CGF.CGM.getTarget().getCXXABI().isItaniumFamily() &&
-         CGF.CGM.getItaniumVTableContext().isRelativeLayout())
+         CGF.CGM.getLangOpts().RelativeCXXABIVTables)
             ? CGF.Int32Ty
             : CGF.PtrDiffTy;
     baseOffset =
@@ -2937,7 +2937,7 @@ llvm::Value *CodeGenFunction::EmitVTableTypeCheckedLoad(
       CGM.CreateMetadataIdentifierForType(QualType(RD->getTypeForDecl(), 0));
   llvm::Value *TypeId = llvm::MetadataAsValue::get(CGM.getLLVMContext(), MD);
 
-  auto CheckedLoadIntrinsic = CGM.getVTables().useRelativeLayout()
+  auto CheckedLoadIntrinsic = CGM.getLangOpts().RelativeCXXABIVTables
                                   ? llvm::Intrinsic::type_checked_load_relative
                                   : llvm::Intrinsic::type_checked_load;
   llvm::Value *CheckedLoad = Builder.CreateCall(
diff --git a/clang/lib/CodeGen/CGVTables.cpp b/clang/lib/CodeGen/CGVTables.cpp
index 8d0e8d194f2c9..24deb96889e03 100644
--- a/clang/lib/CodeGen/CGVTables.cpp
+++ b/clang/lib/CodeGen/CGVTables.cpp
@@ -704,17 +704,8 @@ void CodeGenVTables::addRelativeComponent(ConstantArrayBuilder &builder,
                                       /*position=*/vtableAddressPoint);
 }
 
-static bool UseRelativeLayout(const CodeGenModule &CGM) {
-  return CGM.getTarget().getCXXABI().isItaniumFamily() &&
-         CGM.getItaniumVTableContext().isRelativeLayout();
-}
-
-bool CodeGenVTables::useRelativeLayout() const {
-  return UseRelativeLayout(CGM);
-}
-
 llvm::Type *CodeGenModule::getVTableComponentType() const {
-  if (UseRelativeLayout(*this))
+  if (getLangOpts().RelativeCXXABIVTables)
     return Int32Ty;
   return GlobalsInt8PtrTy;
 }
@@ -746,8 +737,9 @@ void CodeGenVTables::addVTableComponent(ConstantArrayBuilder &builder,
                                         bool vtableHasLocalLinkage) {
   auto &component = layout.vtable_components()[componentIndex];
 
+  bool RelativeCXXABIVTables = CGM.getLangOpts().RelativeCXXABIVTables;
   auto addOffsetConstant =
-      useRelativeLayout() ? AddRelativeLayoutOffset : AddPointerLayoutOffset;
+      RelativeCXXABIVTables ? AddRelativeLayoutOffset : AddPointerLayoutOffset;
 
   switch (component.getKind()) {
   case VTableComponent::CK_VCallOffset:
@@ -760,7 +752,7 @@ void CodeGenVTables::addVTableComponent(ConstantArrayBuilder &builder,
     return addOffsetConstant(CGM, builder, component.getOffsetToTop());
 
   case VTableComponent::CK_RTTI:
-    if (useRelativeLayout())
+    if (RelativeCXXABIVTables)
       return addRelativeComponent(builder, rtti, vtableAddressPoint,
                                   vtableHasLocalLinkage,
                                   /*isCompleteDtor=*/false);
@@ -804,7 +796,7 @@ void CodeGenVTables::addVTableComponent(ConstantArrayBuilder &builder,
       // depending on link order, the comdat groups could resolve to the one
       // with the local symbol. As a temporary solution, fill these components
       // with zero. We shouldn't be calling these in the first place anyway.
-      if (useRelativeLayout())
+      if (RelativeCXXABIVTables)
         return llvm::ConstantPointerNull::get(CGM.GlobalsInt8PtrTy);
 
       // For NVPTX devices in OpenMP emit special functon as null pointers,
@@ -856,7 +848,7 @@ void CodeGenVTables::addVTableComponent(ConstantArrayBuilder &builder,
         GD = getItaniumVTableContext().findOriginalMethod(GD);
     }
 
-    if (useRelativeLayout()) {
+    if (RelativeCXXABIVTables) {
       return addRelativeComponent(
           builder, fnPtr, vtableAddressPoint, vtableHasLocalLinkage,
           component.getKind() == VTableComponent::CK_CompleteDtorPointer);
@@ -879,7 +871,7 @@ void CodeGenVTables::addVTableComponent(ConstantArrayBuilder &builder,
   }
 
   case VTableComponent::CK_UnusedFunctionPointer:
-    if (useRelativeLayout())
+    if (RelativeCXXABIVTables)
       return builder.add(llvm::ConstantExpr::getNullValue(CGM.Int32Ty));
     else
       return builder.addNullPointer(CGM.GlobalsInt8PtrTy);
@@ -943,7 +935,7 @@ llvm::GlobalVariable *CodeGenVTables::GenerateConstructionVTable(
                            Base.getBase(), Out);
   SmallString<256> Name(OutName);
 
-  bool UsingRelativeLayout = getItaniumVTableContext().isRelativeLayout();
+  bool UsingRelativeLayout = CGM.getLangOpts().RelativeCXXABIVTables;
   bool VTableAliasExists =
       UsingRelativeLayout && CGM.getModule().getNamedAlias(Name);
   if (VTableAliasExists) {
@@ -1022,7 +1014,7 @@ void CodeGenVTables::RemoveHwasanMetadata(llvm::GlobalValue *GV) const {
 // the original vtable type.
 void CodeGenVTables::GenerateRelativeVTableAlias(llvm::GlobalVariable *VTable,
                                                  llvm::StringRef AliasNameRef) {
-  assert(getItaniumVTableContext().isRelativeLayout() &&
+  assert(CGM.getLangOpts().RelativeCXXABIVTables &&
          "Can only use this if the relative vtable ABI is used");
   assert(!VTable->isDSOLocal() && "This should be called only if the vtable is "
                                   "not guaranteed to be dso_local");
diff --git a/clang/lib/CodeGen/CGVTables.h b/clang/lib/CodeGen/CGVTables.h
index 5c45e355fb145..337c635482fc4 100644
--- a/clang/lib/CodeGen/CGVTables.h
+++ b/clang/lib/CodeGen/CGVTables.h
@@ -150,9 +150,6 @@ class CodeGenVTables {
 
   /// Return the type used as components for a vtable.
   llvm::Type *getVTableComponentType() const;
-
-  /// Return true if the relative vtable layout is used.
-  bool useRelativeLayout() const;
 };
 
 } // end namespace CodeGen
diff --git a/clang/lib/CodeGen/ItaniumCXXABI.cpp b/clang/lib/CodeGen/ItaniumCXXABI.cpp
index 70b53be7e77a3..5d64dfd58accb 100644
--- a/clang/lib/CodeGen/ItaniumCXXABI.cpp
+++ b/clang/lib/CodeGen/ItaniumCXXABI.cpp
@@ -742,7 +742,7 @@ CGCallee ItaniumCXXABI::EmitLoadOfMemberFunctionPointer(
             Builder.CreateCall(CGM.getIntrinsic(IID), {VFPAddr, TypeId});
       }
 
-      if (CGM.getItaniumVTableContext().isRelativeLayout()) {
+      if (CGM.getLangOpts().RelativeCXXABIVTables) {
         VirtualFn = CGF.Builder.CreateCall(
             CGM.getIntrinsic(llvm::Intrinsic::load_relative,
                              {VTableOffset->getType()}),
@@ -1135,7 +1135,7 @@ llvm::Constant *ItaniumCXXABI::BuildMemberPointer(const CXXMethodDecl *MD,
   if (MD->isVirtual()) {
     uint64_t Index = CGM.getItaniumVTableContext().getMethodVTableIndex(MD);
     uint64_t VTableOffset;
-    if (CGM.getItaniumVTableContext().isRelativeLayout()) {
+    if (CGM.getLangOpts().RelativeCXXABIVTables) {
       // Multiply by 4-byte relative offsets.
       VTableOffset = Index * 4;
     } else {
@@ -1596,7 +1596,7 @@ llvm::Value *ItaniumCXXABI::EmitTypeid(CodeGenFunction &CGF,
   llvm::Value *Value = CGF.GetVTablePtr(ThisPtr, CGM.GlobalsInt8PtrTy,
                                         ClassDecl);
 
-  if (CGM.getItaniumVTableContext().isRelativeLayout()) {
+  if (CGM.getLangOpts().RelativeCXXABIVTables) {
     // Load the type info.
     Value = CGF.Builder.CreateCall(
         CGM.getIntrinsic(llvm::Intrinsic::load_relative, {CGM.Int32Ty}),
@@ -1751,7 +1751,7 @@ llvm::Value *ItaniumCXXABI::emitDynamicCastToVoid(CodeGenFunction &CGF,
   auto *ClassDecl =
       cast<CXXRecordDecl>(SrcRecordTy->castAs<RecordType>()->getDecl());
   llvm::Value *OffsetToTop;
-  if (CGM.getItaniumVTableContext().isRelativeLayout()) {
+  if (CGM.getLangOpts().RelativeCXXABIVTables) {
     // Get the vtable pointer.
     llvm::Value *VTable =
         CGF.GetVTablePtr(ThisAddr, CGF.UnqualPtrTy, ClassDecl);
@@ -1803,7 +1803,7 @@ ItaniumCXXABI::GetVirtualBaseClassOffset(CodeGenFunction &CGF,
         "vbase.offset.ptr");
 
   llvm::Value *VBaseOffset;
-  if (CGM.getItaniumVTableContext().isRelativeLayout()) {
+  if (CGM.getLangOpts().RelativeCXXABIVTables) {
     VBaseOffset = CGF.Builder.CreateAlignedLoad(
         CGF.Int32Ty, VBaseOffsetPtr, CharUnits::fromQuantity(4),
         "vbase.offset");
@@ -2051,7 +2051,7 @@ void ItaniumCXXABI::emitVTableDefinitions(CodeGenVTables &CGVT,
     }
   }
 
-  if (VTContext.isRelativeLayout()) {
+  if (CGM.getLangOpts().RelativeCXXABIVTables) {
     CGVT.RemoveHwasanMetadata(VTable);
     if (!VTable->isDSOLocal())
       CGVT.GenerateRelativeVTableAlias(VTable, VTable->getName());
@@ -2162,7 +2162,7 @@ llvm::GlobalVariable *ItaniumCXXABI::getAddrOfVTable(const CXXRecordDecl *RD,
   // them based on the size of the initializer which doesn't make sense as only
   // single values are read.
   LangAS AS = CGM.GetGlobalVarAddressSpace(nullptr);
-  unsigned PAlign = CGM.getItaniumVTableContext().isRelativeLayout()
+  unsigned PAlign = CGM.getLangOpts().RelativeCXXABIVTables
                         ? 32
                         : CGM.getTarget().getPointerAlign(AS);
 
@@ -2202,7 +2202,7 @@ CGCallee ItaniumCXXABI::getVirtualFunctionPointer(CodeGenFunction &CGF,
     CGF.EmitTypeMetadataCodeForVCall(MethodDecl->getParent(), VTable, Loc);
 
     llvm::Value *VFuncLoad;
-    if (CGM.getItaniumVTableContext().isRelativeLayout()) {
+    if (CGM.getLangOpts().RelativeCXXABIVTables) {
       VFuncLoad = CGF.Builder.CreateCall(
           CGM.getIntrinsic(llvm::Intrinsic::load_relative, {CGM.Int32Ty}),
           {VTable, llvm::ConstantInt::get(CGM.Int32Ty, ByteOffset)});
@@ -2370,7 +2370,7 @@ static llvm::Value *performTypeAdjustment(CodeGenFunction &CGF,
     llvm::Value *Offset;
     llvm::Value *OffsetPtr = CGF.Builder.CreateConstInBoundsGEP1_64(
         CGF.Int8Ty, VTablePtr, VirtualAdjustment);
-    if (CGF.CGM.getItaniumVTableContext().isRelativeLayout()) {
+    if (CGF.CGM.getLangOpts().RelativeCXXABIVTables) {
       // Load the adjustment offset from the vtable as a 32-bit int.
       Offset =
           CGF.Builder.CreateAlignedLoad(CGF.Int32Ty, OffsetPtr,
@@ -3963,7 +3963,7 @@ void ItaniumRTTIBuilder::BuildVTablePointer(const Type *Ty,
   llvm::Constant *VTable = nullptr;
 
   // Check if the alias exists. If it doesn't, then get or create the global.
-  if (CGM.getItaniumVTableContext().isRelativeLayout())
+  if (CGM.getLangOpts().RelativeCXXABIVTables)
     VTable = CGM.getModule().getNamedAlias(VTableName);
   if (!VTable) {
     llvm::Type *Ty = llvm::ArrayType::get(CGM.GlobalsInt8PtrTy, 0);
@@ -3976,7 +3976,7 @@ void ItaniumRTTIBuilder::BuildVTablePointer(const Type *Ty,
       CGM.getTypes().ConvertType(CGM.getContext().getPointerDiffType());
 
   // The vtable address point is 2.
-  if (CGM.getItaniumVTableContext().isRelativeLayout()) {
+  if (CGM.getLangOpts().RelativeCXXABIVTables) {
     // The vtable address point is 8 bytes after its start:
     // 4 for the offset to top + 4 for the relative offset to rtti.
     llvm::Constant *Eight = llvm::ConstantInt::get(CGM.Int32Ty, 8);

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

clang:codegen IR generation bugs: mangling, exceptions, etc. clang:frontend Language frontend issues, e.g. anything involving "Sema" clang Clang issues not falling into any other category

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants