[lld] r233128 - PECOFF: Reduce import table size.

[Rui Ueyama]

We could, but now LLD cannot produce duplicate entries, so we cannot make
it emit two versions and compare them. I could write a test that verifies
that file size of output is less than some value, but file size could
change for other reasons, I'm reluctant to write such test.

Maybe we could write a test to check if some string is contained only once
in a file?

On Tue, Mar 24, 2015 at 4:39 PM, Rafael Espíndola <
rafael.espindola at gmail.com> wrote:

> well, you could test that the binary is now smaller, no?
>
> On 24 March 2015 at 19:23, Rui Ueyama <ruiu at google.com> wrote:
> > The existing test cases using llvm-readobj should suffice. Output from
> > readobj didn't change, as the tool doesn't care whether or not the string
> > it's printing is an alias of something.
> >
> > On Tue, Mar 24, 2015 at 4:21 PM, Rafael Espíndola
> > <rafael.espindola at gmail.com> wrote:
> >>
> >> testcase? :-)
> >>
> >> On 24 March 2015 at 18:43, Rui Ueyama <ruiu at google.com> wrote:
> >> > Author: ruiu
> >> > Date: Tue Mar 24 17:43:58 2015
> >> > New Revision: 233128
> >> >
> >> > URL: http://llvm.org/viewvc/llvm-project?rev=233128&view=rev
> >> > Log:
> >> > PECOFF: Reduce import table size.
> >> >
> >> > Import Lookup Table in Import Directory Table has the same contents
> >> > as Hint/Name Table. Symbol names imported from DLLs are pointed by
> >> > both Import Directory Table and Hint/Name Table. We had duplicate
> >> > strings there.
> >> >
> >> > This patch eliminates that duplication to make the table smaller.
> >> > This should reduce binary size by the sum of lengths of imported
> >> > symbols.
> >> >
> >> > Modified:
> >> >     lld/trunk/lib/ReaderWriter/PECOFF/IdataPass.cpp
> >> >
> >> > Modified: lld/trunk/lib/ReaderWriter/PECOFF/IdataPass.cpp
> >> > URL:
> >> >
> http://llvm.org/viewvc/llvm-project/lld/trunk/lib/ReaderWriter/PECOFF/IdataPass.cpp?rev=233128&r1=233127&r2=233128&view=diff
> >> >
> >> >
> ==============================================================================
> >> > --- lld/trunk/lib/ReaderWriter/PECOFF/IdataPass.cpp (original)
> >> > +++ lld/trunk/lib/ReaderWriter/PECOFF/IdataPass.cpp Tue Mar 24
> 17:43:58
> >> > 2015
> >> > @@ -69,6 +69,7 @@ static std::vector<ImportTableEntryAtom
> >> >  createImportTableAtoms(IdataContext &context,
> >> >                         const std::vector<COFFSharedLibraryAtom *>
> >> > &sharedAtoms,
> >> >                         bool shouldAddReference, StringRef
> sectionName,
> >> > +                       std::map<StringRef, HintNameAtom *>
> >> > &hintNameCache,
> >> >                         llvm::BumpPtrAllocator &alloc) {
> >> >    std::vector<ImportTableEntryAtom *> ret;
> >> >    for (COFFSharedLibraryAtom *atom : sharedAtoms) {
> >> > @@ -81,9 +82,16 @@ createImportTableAtoms(IdataContext &con
> >> >      } else {
> >> >        // Import by name
> >> >        entry = new (alloc) ImportTableEntryAtom(context, 0,
> >> > sectionName);
> >> > -      HintNameAtom *hintName =
> >> > -          new (alloc) HintNameAtom(context, atom->hint(),
> >> > atom->importName());
> >> > -      addDir32NBReloc(entry, hintName, context.ctx.getMachineType(),
> >> > 0);
> >> > +      HintNameAtom *hintNameAtom;
> >> > +      auto it = hintNameCache.find(atom->importName());
> >> > +      if (it == hintNameCache.end()) {
> >> > +        hintNameAtom = new (alloc) HintNameAtom(
> >> > +            context, atom->hint(), atom->importName());
> >> > +        hintNameCache[atom->importName()] = hintNameAtom;
> >> > +      } else {
> >> > +        hintNameAtom = it->second;
> >> > +      }
> >> > +      addDir32NBReloc(entry, hintNameAtom,
> >> > context.ctx.getMachineType(), 0);
> >> >      }
> >> >      ret.push_back(entry);
> >> >      if (shouldAddReference)
> >> > @@ -104,10 +112,13 @@ void ImportDirectoryAtom::addRelocations
> >> >    // same. The PE/COFF loader overwrites the import address tables
> with
> >> > the
> >> >    // pointers to the referenced items after loading the executable
> into
> >> >    // memory.
> >> > +  std::map<StringRef, HintNameAtom *> hintNameCache;
> >> >    std::vector<ImportTableEntryAtom *> importLookupTables =
> >> > -      createImportTableAtoms(context, sharedAtoms, false, ".idata.t",
> >> > _alloc);
> >> > +      createImportTableAtoms(context, sharedAtoms, false, ".idata.t",
> >> > +                             hintNameCache, _alloc);
> >> >    std::vector<ImportTableEntryAtom *> importAddressTables =
> >> > -      createImportTableAtoms(context, sharedAtoms, true, ".idata.a",
> >> > _alloc);
> >> > +      createImportTableAtoms(context, sharedAtoms, true, ".idata.a",
> >> > +                             hintNameCache, _alloc);
> >> >
> >> >    addDir32NBReloc(this, importLookupTables[0],
> >> > context.ctx.getMachineType(),
> >> >                    offsetof(ImportDirectoryTableEntry,
> >> > ImportLookupTableRVA));
> >> > @@ -157,8 +168,10 @@ void DelayImportDirectoryAtom::addReloca
> >> >    // as (non-delay) import table's Import Lookup Table. Contains
> >> >    // imported function names. This is a parallel array of
> AddressTable
> >> >    // field.
> >> > +  std::map<StringRef, HintNameAtom *> hintNameCache;
> >> >    std::vector<ImportTableEntryAtom *> nameTable =
> >> > -      createImportTableAtoms(context, sharedAtoms, false, ".didat",
> >> > _alloc);
> >> > +      createImportTableAtoms(
> >> > +          context, sharedAtoms, false, ".didat", hintNameCache,
> >> > _alloc);
> >> >    addDir32NBReloc(
> >> >        this, nameTable[0], context.ctx.getMachineType(),
> >> >        offsetof(delay_import_directory_table_entry,
> >> > DelayImportNameTable));
> >> >
> >> >
> >> > _______________________________________________
> >> > llvm-commits mailing list
> >> > llvm-commits at cs.uiuc.edu
> >> > http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
> >
> >
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20150324/353d62d6/attachment.html>