r197845 - Eliminate the ItaniumVTableContext object from CodeGenVTables

Timur Iskhodzhanov timurrrr at google.com
Tue Dec 31 04:09:09 PST 2013


Awesome, LGTM!

2013/12/21 Reid Kleckner <reid at kleckner.net>:
> Author: rnk
> Date: Fri Dec 20 17:58:52 2013
> New Revision: 197845
>
> URL: http://llvm.org/viewvc/llvm-project?rev=197845&view=rev
> Log:
> Eliminate the ItaniumVTableContext object from CodeGenVTables
>
> Now CodeGenVTables has only one VTableContext object, which is either
> Itanium or Microsoft.
>
> Fixes a FIXME with no functionality change intended.
>
> Ideally we could avoid the downcasts by pushing the things that
> reference the Itanium vtable context into ItaniumCXXABI.cpp, but we're
> not there yet.
>
> Modified:
>     cfe/trunk/include/clang/AST/VTableBuilder.h
>     cfe/trunk/lib/AST/VTableBuilder.cpp
>     cfe/trunk/lib/CodeGen/CGVTT.cpp
>     cfe/trunk/lib/CodeGen/CGVTables.cpp
>     cfe/trunk/lib/CodeGen/CGVTables.h
>
> Modified: cfe/trunk/include/clang/AST/VTableBuilder.h
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/VTableBuilder.h?rev=197845&r1=197844&r2=197845&view=diff
> ==============================================================================
> --- cfe/trunk/include/clang/AST/VTableBuilder.h (original)
> +++ cfe/trunk/include/clang/AST/VTableBuilder.h Fri Dec 20 17:58:52 2013
> @@ -270,6 +270,10 @@ class VTableContextBase {
>  public:
>    typedef SmallVector<ThunkInfo, 1> ThunkInfoVectorTy;
>
> +  bool isMicrosoft() const { return IsMicrosoftABI; }
> +
> +  virtual ~VTableContextBase() {}
> +
>  protected:
>    typedef llvm::DenseMap<const CXXMethodDecl *, ThunkInfoVectorTy> ThunksMapTy;
>
> @@ -280,7 +284,7 @@ protected:
>    /// offset offsets, thunks etc) for the given record decl.
>    virtual void computeVTableRelatedInformation(const CXXRecordDecl *RD) = 0;
>
> -  virtual ~VTableContextBase() {}
> +  VTableContextBase(bool MS) : IsMicrosoftABI(MS) {}
>
>  public:
>    virtual const ThunkInfoVectorTy *getThunkInfo(GlobalDecl GD) {
> @@ -297,11 +301,12 @@ public:
>
>      return &I->second;
>    }
> +
> +  bool IsMicrosoftABI;
>  };
>
>  class ItaniumVTableContext : public VTableContextBase {
>  private:
> -  bool IsMicrosoftABI;
>
>    /// \brief Contains the index (relative to the vtable address point)
>    /// where the function pointer for a virtual function is stored.
> @@ -355,6 +360,10 @@ public:
>    /// Base must be a virtual base class or an unambiguous base.
>    CharUnits getVirtualBaseOffsetOffset(const CXXRecordDecl *RD,
>                                         const CXXRecordDecl *VBase);
> +
> +  static bool classof(const VTableContextBase *VT) {
> +    return !VT->isMicrosoft();
> +  }
>  };
>
>  struct VFPtrInfo {
> @@ -481,7 +490,8 @@ private:
>    void computeVBTableRelatedInformation(const CXXRecordDecl *RD);
>
>  public:
> -  MicrosoftVTableContext(ASTContext &Context) : Context(Context) {}
> +  MicrosoftVTableContext(ASTContext &Context)
> +      : VTableContextBase(/*MS=*/true), Context(Context) {}
>
>    ~MicrosoftVTableContext() { llvm::DeleteContainerSeconds(VFTableLayouts); }
>
> @@ -512,6 +522,8 @@ public:
>             "VBase must be a vbase of Derived");
>      return VBTableIndices[Pair];
>    }
> +
> +  static bool classof(const VTableContextBase *VT) { return VT->isMicrosoft(); }
>  };
>  }
>
>
> Modified: cfe/trunk/lib/AST/VTableBuilder.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/VTableBuilder.cpp?rev=197845&r1=197844&r2=197845&view=diff
> ==============================================================================
> --- cfe/trunk/lib/AST/VTableBuilder.cpp (original)
> +++ cfe/trunk/lib/AST/VTableBuilder.cpp Fri Dec 20 17:58:52 2013
> @@ -2282,8 +2282,7 @@ VTableLayout::VTableLayout(uint64_t NumV
>  VTableLayout::~VTableLayout() { }
>
>  ItaniumVTableContext::ItaniumVTableContext(ASTContext &Context)
> -  : IsMicrosoftABI(Context.getTargetInfo().getCXXABI().isMicrosoft()) {
> -}
> +    : VTableContextBase(/*MS=*/false) {}
>
>  ItaniumVTableContext::~ItaniumVTableContext() {
>    llvm::DeleteContainerSeconds(VTableLayouts);
> @@ -2348,8 +2347,6 @@ static VTableLayout *CreateVTableLayout(
>
>  void
>  ItaniumVTableContext::computeVTableRelatedInformation(const CXXRecordDecl *RD) {
> -  assert(!IsMicrosoftABI && "Shouldn't be called in this ABI!");
> -
>    const VTableLayout *&Entry = VTableLayouts[RD];
>
>    // Check if we've computed this information before.
>
> Modified: cfe/trunk/lib/CodeGen/CGVTT.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGVTT.cpp?rev=197845&r1=197844&r2=197845&view=diff
> ==============================================================================
> --- cfe/trunk/lib/CodeGen/CGVTT.cpp (original)
> +++ cfe/trunk/lib/CodeGen/CGVTT.cpp Fri Dec 20 17:58:52 2013
> @@ -66,7 +66,8 @@ CodeGenVTables::EmitVTTDefinition(llvm::
>      if (VTTVT.getBase() == RD) {
>        // Just get the address point for the regular vtable.
>        AddressPoint =
> -          ItaniumVTContext.getVTableLayout(RD).getAddressPoint(i->VTableBase);
> +          getItaniumVTableContext().getVTableLayout(RD).getAddressPoint(
> +              i->VTableBase);
>        assert(AddressPoint != 0 && "Did not find vtable address point!");
>      } else {
>        AddressPoint = VTableAddressPoints[i->VTableIndex].lookup(i->VTableBase);
>
> Modified: cfe/trunk/lib/CodeGen/CGVTables.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGVTables.cpp?rev=197845&r1=197844&r2=197845&view=diff
> ==============================================================================
> --- cfe/trunk/lib/CodeGen/CGVTables.cpp (original)
> +++ cfe/trunk/lib/CodeGen/CGVTables.cpp Fri Dec 20 17:58:52 2013
> @@ -29,14 +29,11 @@
>  using namespace clang;
>  using namespace CodeGen;
>
> -CodeGenVTables::CodeGenVTables(CodeGenModule &CGM)
> -  : CGM(CGM), ItaniumVTContext(CGM.getContext()) {
> -  if (CGM.getTarget().getCXXABI().isMicrosoft()) {
> -    // FIXME: Eventually, we should only have one of V*TContexts available.
> -    // Today we use both in the Microsoft ABI as MicrosoftVFTableContext
> -    // is not completely supported in CodeGen yet.
> -    MicrosoftVTContext.reset(new MicrosoftVTableContext(CGM.getContext()));
> -  }
> +CodeGenVTables::CodeGenVTables(CodeGenModule &CGM) : CGM(CGM) {
> +  if (CGM.getTarget().getCXXABI().isMicrosoft())
> +    VTContext.reset(new MicrosoftVTableContext(CGM.getContext()));
> +  else
> +    VTContext.reset(new ItaniumVTableContext(CGM.getContext()));
>  }
>
>  llvm::Constant *CodeGenModule::GetAddrOfThunk(GlobalDecl GD,
> @@ -465,12 +462,8 @@ void CodeGenVTables::EmitThunks(GlobalDe
>    if (isa<CXXDestructorDecl>(MD) && GD.getDtorType() == Dtor_Base)
>      return;
>
> -  const VTableContextBase::ThunkInfoVectorTy *ThunkInfoVector;
> -  if (MicrosoftVTContext.isValid()) {
> -    ThunkInfoVector = MicrosoftVTContext->getThunkInfo(GD);
> -  } else {
> -    ThunkInfoVector = ItaniumVTContext.getThunkInfo(GD);
> -  }
> +  const VTableContextBase::ThunkInfoVectorTy *ThunkInfoVector =
> +      VTContext->getThunkInfo(GD);
>
>    if (!ThunkInfoVector)
>      return;
> @@ -608,7 +601,7 @@ CodeGenVTables::GenerateConstructionVTab
>      DI->completeClassData(Base.getBase());
>
>    OwningPtr<VTableLayout> VTLayout(
> -      ItaniumVTContext.createConstructionVTableLayout(
> +      getItaniumVTableContext().createConstructionVTableLayout(
>            Base.getBase(), Base.getBaseOffset(), BaseIsVirtual, RD));
>
>    // Add the address points.
>
> Modified: cfe/trunk/lib/CodeGen/CGVTables.h
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGVTables.h?rev=197845&r1=197844&r2=197845&view=diff
> ==============================================================================
> --- cfe/trunk/lib/CodeGen/CGVTables.h (original)
> +++ cfe/trunk/lib/CodeGen/CGVTables.h Fri Dec 20 17:58:52 2013
> @@ -31,10 +31,8 @@ namespace CodeGen {
>  class CodeGenVTables {
>    CodeGenModule &CGM;
>
> -  // FIXME: Consider moving ItaniumVTContext and MicrosoftVTContext into
> -  // respective CXXABI classes?
> -  ItaniumVTableContext ItaniumVTContext;
> -  OwningPtr<MicrosoftVTableContext> MicrosoftVTContext;
> +  // FIXME: Consider moving VTContext into respective CXXABI classes?
> +  OwningPtr<VTableContextBase> VTContext;
>
>    /// VTableAddressPointsMapTy - Address points for a single vtable.
>    typedef llvm::DenseMap<BaseSubobject, uint64_t> VTableAddressPointsMapTy;
> @@ -72,10 +70,12 @@ public:
>
>    CodeGenVTables(CodeGenModule &CGM);
>
> -  ItaniumVTableContext &getItaniumVTableContext() { return ItaniumVTContext; }
> +  ItaniumVTableContext &getItaniumVTableContext() {
> +    return *cast<ItaniumVTableContext>(VTContext.get());
> +  }
>
>    MicrosoftVTableContext &getMicrosoftVTableContext() {
> -    return *MicrosoftVTContext.get();
> +    return *cast<MicrosoftVTableContext>(VTContext.get());
>    }
>
>    /// getSubVTTIndex - Return the index of the sub-VTT for the base class of the
>
>
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits



More information about the cfe-commits mailing list