Done in r77030 - Thanks Fariborz
On Jul 24, 2009, at 5:04 PM, Daniel Dunbar wrote: > On Wed, Jul 22, 2009 at 1:25 PM, Fariborz > Jahanian<[email protected]> wrote: >> Author: fjahanian >> Date: Wed Jul 22 15:25:00 2009 >> New Revision: 76776 >> >> URL: http://llvm.org/viewvc/llvm-project?rev=76776&view=rev >> Log: >> Improved on performance of the algorithm for proper ordering of >> ctor's initialization of bases and fields. > > Cool > >> + llvm::DenseMap<const void *, CXXBaseOrMemberInitializer*> >> AllBaseFields; >> + >> + for (unsigned i = 0; i < NumInitializers; i++) { >> + CXXBaseOrMemberInitializer *Member = Initializers[i]; >> + const void * Key = Member->isBaseInitializer() ? >> + reinterpret_cast<const void *>( >> + Member->getBaseClass()- >> >getAsRecordType()) : >> + reinterpret_cast<const void *>(Member->getMember()); >> + AllBaseFields[Key] = Member; >> + } > > I think this is more readable as: > -- > for ... { > CXXBaseOrMemberInitializer *Member = Initializers[i]; > if (Member->isBaseInitializer()) > AllBaseFields[Member->getBaseClass()->getAsRecordType()] = > Member; > else > AllBaseFields[Member->getMember()] = Member; > } > -- > >> + >> // Push virtual bases before others. >> for (CXXRecordDecl::base_class_iterator VBase = >> ClassDecl->vbases_begin(), >> E = ClassDecl->vbases_end(); VBase != E; ++VBase) { >> - const Type * T = VBase->getType()->getAsRecordType(); >> - unsigned int i = 0; >> - for (i = 0; i < NumInitializers; i++) { >> - CXXBaseOrMemberInitializer *Member = Initializers[i]; >> - if (Member->isBaseInitializer() && >> - Member->getBaseClass()->getAsRecordType() == T) { >> - AllToInit.push_back(Member); >> - break; >> - } >> - } >> - if (i == NumInitializers) { >> + const void *Key = reinterpret_cast<const void *>( >> + VBase->getType()- >> >getAsRecordType()); > > I don't think this case is necessary? > >> + if (AllBaseFields[Key]) >> + AllToInit.push_back(AllBaseFields[Key]); > > This should be > -- > if (CXXBaseOrMemberInitializer *Value = AllBaseFields.lookup(Key)) > AllToInit.push_back(Value) > -- > Otherwise it looks up the value twice, and inserts a null binding > into the map. > >> + const void *Key = reinterpret_cast<const void *>( >> + Base->getType()- >> >getAsRecordType()); >> + if (AllBaseFields[Key]) >> + AllToInit.push_back(AllBaseFields[Key]); > > Same two comments as before. > >> + const void * Key = reinterpret_cast<const void *>(*Field); >> + if (AllBaseFields[Key]) { >> + AllToInit.push_back(AllBaseFields[Key]); >> + continue; > > Ditto. > > - Daniel _______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
