r188909 - [CGF] Get rid of passing redundant VTable pointer around in CodeGenFunction::InitializeVTablePointer[s]

[Timur Iskhodzhanov]

2013/8/22 David Blaikie <dblaikie at gmail.com>

> On Wed, Aug 21, 2013 at 10:33 AM, Timur Iskhodzhanov
> <timurrrr at google.com> wrote:
> > Author: timurrrr
> > Date: Wed Aug 21 12:33:16 2013
> > New Revision: 188909
> >
> > URL: http://llvm.org/viewvc/llvm-project?rev=188909&view=rev
> > Log:
> > [CGF] Get rid of passing redundant VTable pointer around in
> CodeGenFunction::InitializeVTablePointer[s]
>
> Strangely, this appears to have regressed debug info (probably based
> on a debug info feature I implemented over the weekend).
>
> http://lab.llvm.org:8011/builders/clang-x86_64-ubuntu-gdb-75/builds/8031
>
> I don't really know how your change connects to this, but I'll
> investigate, etc - no action required on your part, just wanted to
> note it down here in case anyone was looking at the bots, seeing weird
> debug info, etc.


Ouch, check-all doesn't detect that...
Thanks for taking this!


>  >
> > Modified:
> >     cfe/trunk/lib/CodeGen/CGClass.cpp
> >     cfe/trunk/lib/CodeGen/CodeGenFunction.h
> >
> > Modified: cfe/trunk/lib/CodeGen/CGClass.cpp
> > URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGClass.cpp?rev=188909&r1=188908&r2=188909&view=diff
> >
> ==============================================================================
> > --- cfe/trunk/lib/CodeGen/CGClass.cpp (original)
> > +++ cfe/trunk/lib/CodeGen/CGClass.cpp Wed Aug 21 12:33:16 2013
> > @@ -1852,7 +1852,6 @@ void
> >  CodeGenFunction::InitializeVTablePointer(BaseSubobject Base,
> >                                           const CXXRecordDecl
> *NearestVBase,
> >                                           CharUnits
> OffsetFromNearestVBase,
> > -                                         llvm::Constant *VTable,
> >                                           const CXXRecordDecl
> *VTableClass) {
> >    const CXXRecordDecl *RD = Base.getBase();
> >
> > @@ -1875,6 +1874,7 @@ CodeGenFunction::InitializeVTablePointer
> >      // And load the address point from the VTT.
> >      VTableAddressPoint = Builder.CreateLoad(VTT);
> >    } else {
> > +    llvm::Constant *VTable =
> CGM.getVTables().GetAddrOfVTable(VTableClass);
> >      uint64_t AddressPoint =
> >
>  CGM.getVTableContext().getVTableLayout(VTableClass).getAddressPoint(Base);
> >      VTableAddressPoint =
> > @@ -1919,7 +1919,6 @@ CodeGenFunction::InitializeVTablePointer
> >                                            const CXXRecordDecl
> *NearestVBase,
> >                                            CharUnits
> OffsetFromNearestVBase,
> >                                            bool
> BaseIsNonVirtualPrimaryBase,
> > -                                          llvm::Constant *VTable,
> >                                            const CXXRecordDecl
> *VTableClass,
> >                                            VisitedVirtualBasesSetTy&
> VBases) {
> >    // If this base is a non-virtual primary base the address point has
> already
> > @@ -1927,7 +1926,7 @@ CodeGenFunction::InitializeVTablePointer
> >    if (!BaseIsNonVirtualPrimaryBase) {
> >      // Initialize the vtable pointer for this base.
> >      InitializeVTablePointer(Base, NearestVBase, OffsetFromNearestVBase,
> > -                            VTable, VTableClass);
> > +                            VTableClass);
> >    }
> >
> >    const CXXRecordDecl *RD = Base.getBase();
> > @@ -1970,7 +1969,7 @@ CodeGenFunction::InitializeVTablePointer
> >                               I->isVirtual() ? BaseDecl : NearestVBase,
> >                               BaseOffsetFromNearestVBase,
> >                               BaseDeclIsNonVirtualPrimaryBase,
> > -                             VTable, VTableClass, VBases);
> > +                             VTableClass, VBases);
> >    }
> >  }
> >
> > @@ -1979,16 +1978,12 @@ void CodeGenFunction::InitializeVTablePo
> >    if (!RD->isDynamicClass())
> >      return;
> >
> > -  // Get the VTable.
> > -  llvm::Constant *VTable = CGM.getVTables().GetAddrOfVTable(RD);
> > -
>

You might consider adding

  CGM.getVTables().GenerateClassData(RD);

here with a comment why it's needed.
Writing a "check-clang" test is also encouraged :)


>  >    // Initialize the vtable pointers for this class and all of its
> bases.
> >    VisitedVirtualBasesSetTy VBases;
> >    InitializeVTablePointers(BaseSubobject(RD, CharUnits::Zero()),
> >                             /*NearestVBase=*/0,
> >                             /*OffsetFromNearestVBase=*/CharUnits::Zero(),
> > -                           /*BaseIsNonVirtualPrimaryBase=*/false,
> > -                           VTable, RD, VBases);
> > +                           /*BaseIsNonVirtualPrimaryBase=*/false, RD,
> VBases);
> >  }
> >
> >  llvm::Value *CodeGenFunction::GetVTablePtr(llvm::Value *This,
> >
> > Modified: cfe/trunk/lib/CodeGen/CodeGenFunction.h
> > URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CodeGenFunction.h?rev=188909&r1=188908&r2=188909&view=diff
> >
> ==============================================================================
> > --- cfe/trunk/lib/CodeGen/CodeGenFunction.h (original)
> > +++ cfe/trunk/lib/CodeGen/CodeGenFunction.h Wed Aug 21 12:33:16 2013
> > @@ -1180,7 +1180,6 @@ public:
> >    void InitializeVTablePointer(BaseSubobject Base,
> >                                 const CXXRecordDecl *NearestVBase,
> >                                 CharUnits OffsetFromNearestVBase,
> > -                               llvm::Constant *VTable,
> >                                 const CXXRecordDecl *VTableClass);
> >
> >    typedef llvm::SmallPtrSet<const CXXRecordDecl *, 4>
> VisitedVirtualBasesSetTy;
> > @@ -1188,7 +1187,6 @@ public:
> >                                  const CXXRecordDecl *NearestVBase,
> >                                  CharUnits OffsetFromNearestVBase,
> >                                  bool BaseIsNonVirtualPrimaryBase,
> > -                                llvm::Constant *VTable,
> >                                  const CXXRecordDecl *VTableClass,
> >                                  VisitedVirtualBasesSetTy& VBases);
> >
> >
> >
> > _______________________________________________
> > cfe-commits mailing list
> > cfe-commits at cs.uiuc.edu
> > http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20130822/647c8cb9/attachment.html>