On Aug 22, 2013 12:45 AM, "Timur Iskhodzhanov" <[email protected]> wrote: > > > > > 2013/8/22 David Blaikie <[email protected]> >> >> On Wed, Aug 21, 2013 at 10:33 AM, Timur Iskhodzhanov >> <[email protected]> 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!
Yeah, we have the gdb 7.5 test suite running for things we never considered testing in check-all. Whenever it finds bugs we add check-all test cases to verify ythe bug fix. You're not expected to run the gdb suite, especially on this change that looks rather innocuous for debug info. > >> >> > >> > 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); I expect this would probably make the tests pass, but I admit I would be a little confused as to why it's needed/will nerd to think about it further. If there are cases where we will never emit the vtable for a dynamic class, even though that class is used, I want to understand how we get away with that without breaking the program, etc. Thanks for the suggestion/pointer > here with a comment why it's needed. > Writing a "check-clang" test is also encouraged :) Yep, as mentioned, we add them when we realize there's something to test, it's just impractical to do so preemptively for the whole gdb test suite. > >> >> > // 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 >> > [email protected] >> > http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits > >
_______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
