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

[Rafael Espíndola]

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
>
>