grimar (George Rimar)
User

Projects

User does not belong to any projects.

User Details

User Since
Sep 21 2015, 12:36 AM (134 w, 6 d)

Recent Activity

Fri, Apr 13

grimar added a comment to D45571: [ELF] - Speedup MergeInputSection::splitStrings.

It should also be possible to template Hash.h over the returned type so that some clients can explicitly request a 32 or 64 bit hash. Not sure if that change would be accepted.

Fri, Apr 13, 12:11 PM
grimar added inline comments to D45571: [ELF] - Speedup MergeInputSection::splitStrings.
Fri, Apr 13, 4:27 AM
grimar updated the diff for D45571: [ELF] - Speedup MergeInputSection::splitStrings.
  • Removed excessive change.
Fri, Apr 13, 4:27 AM
grimar added a comment to D45571: [ELF] - Speedup MergeInputSection::splitStrings.

One interesting thing about the current setup is that we first read the bytes in the string looking for the 0 that terminates the string. We then read them again to hash them. At least with a simple hash like djb (what is implemented in hashGnu) it should not be too hard to read each byte once. Would you mind giving that a try?

Fri, Apr 13, 3:45 AM
grimar accepted D45548: Avoid hash table lookup when sorting local symbols..

LGTM with Rafael's comment addressed.

Fri, Apr 13, 1:39 AM
grimar added a comment to D45548: Avoid hash table lookup when sorting local symbols..

The results I got

master: 2.819319468 seconds time elapsed
This patch: 2.735310955 seconds time elapsed
D45519: 2.624760166 seconds time elapsed
combined: 2.604495001 seconds time elapsed

So I think we want both.

Fri, Apr 13, 1:38 AM
grimar added a comment to D45571: [ELF] - Speedup MergeInputSection::splitStrings.

What is your operating system and CPU?

Fri, Apr 13, 1:25 AM
grimar added a comment to D45571: [ELF] - Speedup MergeInputSection::splitStrings.

I just noticed that hash_short will read at most 64 bytes of the string.

This could cause a pathological case if many symbols have a common prefix, no?

Fri, Apr 13, 1:24 AM

Thu, Apr 12

grimar added inline comments to D45571: [ELF] - Speedup MergeInputSection::splitStrings.
Thu, Apr 12, 12:16 PM
grimar updated the summary of D45571: [ELF] - Speedup MergeInputSection::splitStrings.
Thu, Apr 12, 8:14 AM
grimar created D45571: [ELF] - Speedup MergeInputSection::splitStrings.
Thu, Apr 12, 8:14 AM
grimar updated the diff for D45516: [ELF] - Refactor lazy symbol duplicated code..
  • Reimplemented, addressed comments.
Thu, Apr 12, 4:13 AM
grimar added a comment to D45548: Avoid hash table lookup when sorting local symbols..

Numbers I got after re-profiling this place today:

Thu, Apr 12, 2:51 AM
grimar requested changes to D45548: Avoid hash table lookup when sorting local symbols..

Will not help I believe. Please see my answer in the D45519 thread.

Thu, Apr 12, 1:53 AM
grimar added a comment to D45519: [ELF] - Change the way of sorting local symbols..

Since the order of two files is arbitrary, how about just giving each InputFile a number when it is created ?

That's exactly what I was thinking. :) I'll send you guys a patch.

Thu, Apr 12, 1:52 AM
grimar added inline comments to D45508: Implement --ctors-in-init-array..
Thu, Apr 12, 1:47 AM

Wed, Apr 11

grimar created D45519: [ELF] - Change the way of sorting local symbols..
Wed, Apr 11, 7:47 AM
grimar created D45516: [ELF] - Refactor lazy symbol duplicated code..
Wed, Apr 11, 5:51 AM
grimar updated the diff for D45375: [ELF] - Introduce synthetic file for linker script symbols..
  • Addressed review comments.
Wed, Apr 11, 4:06 AM
grimar added inline comments to D45508: Implement --ctors-in-init-array..
Wed, Apr 11, 3:45 AM
grimar added inline comments to D45508: Implement --ctors-in-init-array..
Wed, Apr 11, 3:38 AM
grimar committed rLLD329787: [ELF] - Reorder local symbols..
[ELF] - Reorder local symbols.
Wed, Apr 11, 2:27 AM
grimar committed rL329787: [ELF] - Reorder local symbols..
[ELF] - Reorder local symbols.
Wed, Apr 11, 2:27 AM
grimar closed D45325: [ELF] - Reorder local symbols..
Wed, Apr 11, 2:27 AM
grimar added a comment to D45325: [ELF] - Reorder local symbols..

LGTM

I think this is fine. It is a bit funny that we count the section symbols as being part of the first file that had that section, but that is unlikely to be a problem.

Wed, Apr 11, 2:19 AM
grimar updated the diff for D45166: [ELF] - Introduce helper for iterating over linker commands..
  • Used filter<> in a few more places.
Wed, Apr 11, 2:10 AM
grimar committed rLLD329785: [ELF] - Simplify. NFC..
[ELF] - Simplify. NFC.
Wed, Apr 11, 2:07 AM
grimar committed rL329785: [ELF] - Simplify. NFC..
[ELF] - Simplify. NFC.
Wed, Apr 11, 2:06 AM

Tue, Apr 10

grimar added inline comments to D45490: [ADT] - Add llvm::make_mapped_range.
Tue, Apr 10, 9:07 AM
grimar added inline comments to D45166: [ELF] - Introduce helper for iterating over linker commands..
Tue, Apr 10, 8:09 AM
grimar added a dependent revision for D45490: [ADT] - Add llvm::make_mapped_range: D45166: [ELF] - Introduce helper for iterating over linker commands..
Tue, Apr 10, 8:04 AM
grimar created D45490: [ADT] - Add llvm::make_mapped_range.
Tue, Apr 10, 8:04 AM
grimar added a dependency for D45166: [ELF] - Introduce helper for iterating over linker commands.: D45490: [ADT] - Add llvm::make_mapped_range.
Tue, Apr 10, 8:04 AM
grimar updated the diff for D45166: [ELF] - Introduce helper for iterating over linker commands..
  • Used the approach suggested.
Tue, Apr 10, 8:04 AM
grimar added inline comments to D45325: [ELF] - Reorder local symbols..
Tue, Apr 10, 4:22 AM
grimar updated the diff for D45325: [ELF] - Reorder local symbols..
  • Addressed review comments.
Tue, Apr 10, 4:21 AM
grimar added inline comments to D45434: [ELF] - Eliminate AssertCommand..
Tue, Apr 10, 3:09 AM
grimar updated the diff for D45434: [ELF] - Eliminate AssertCommand..
  • Addressed review comments.
Tue, Apr 10, 3:09 AM
grimar committed rLLD329678: [ELF] - Do not crash when trying to order --defsym/linker script symbols..
[ELF] - Do not crash when trying to order --defsym/linker script symbols.
Tue, Apr 10, 2:48 AM
grimar committed rL329678: [ELF] - Do not crash when trying to order --defsym/linker script symbols..
[ELF] - Do not crash when trying to order --defsym/linker script symbols.
Tue, Apr 10, 2:48 AM
grimar closed D45440: [ELF] - Do not crash when trying to order --defsym/linker script symbols..
Tue, Apr 10, 2:48 AM
grimar added a comment to D45325: [ELF] - Reorder local symbols..

Also, could we merge the partition and the sort?

I thought the same thing, but I guess separating partition and sort is faster than merging them into one sort function call. Partition is O(n) and sort is O(n log n). So, IIUC, partitioning into local and global symbols first and then sorting only the local part should be faster than sorting the entire vector.

Sounds good. Lets keep both for now.

Tue, Apr 10, 2:05 AM

Mon, Apr 9

grimar updated the diff for D45375: [ELF] - Introduce synthetic file for linker script symbols..
  • Simplify, added one more test case.
Mon, Apr 9, 7:57 AM
grimar added a comment to D45375: [ELF] - Introduce synthetic file for linker script symbols..

I'm not too excited about doing this. It looks like it opens a can of worms, even if the change itself is small.

Mon, Apr 9, 7:56 AM
grimar created D45440: [ELF] - Do not crash when trying to order --defsym/linker script symbols..
Mon, Apr 9, 7:06 AM
grimar committed rLLD329563: [ELF] - Simplify test case. NFC..
[ELF] - Simplify test case. NFC.
Mon, Apr 9, 6:17 AM
grimar committed rL329563: [ELF] - Simplify test case. NFC..
[ELF] - Simplify test case. NFC.
Mon, Apr 9, 6:17 AM
grimar committed rLLD329560: [ELF] - Stop setting OutSecOff too early..
[ELF] - Stop setting OutSecOff too early.
Mon, Apr 9, 6:05 AM
grimar committed rL329560: [ELF] - Stop setting OutSecOff too early..
[ELF] - Stop setting OutSecOff too early.
Mon, Apr 9, 6:05 AM
grimar closed D45368: [ELF] - Stop setting OutSecOff too early..
Mon, Apr 9, 6:04 AM
grimar committed rL329559: [ELF] - Fix cref.s test case..
[ELF] - Fix cref.s test case.
Mon, Apr 9, 5:49 AM
grimar committed rLLD329559: [ELF] - Fix cref.s test case..
[ELF] - Fix cref.s test case.
Mon, Apr 9, 5:49 AM
grimar closed D45159: [ELF] - Fix cref.s test case..
Mon, Apr 9, 5:49 AM
grimar updated the diff for D45325: [ELF] - Reorder local symbols..
  • Rebased.
  • Simplified.
Mon, Apr 9, 5:15 AM
grimar committed rL329557: [ELF] - Allow LLD to produce file symbols..
[ELF] - Allow LLD to produce file symbols.
Mon, Apr 9, 4:47 AM
grimar committed rLLD329557: [ELF] - Allow LLD to produce file symbols..
[ELF] - Allow LLD to produce file symbols.
Mon, Apr 9, 4:47 AM
grimar closed D45261: [ELF] - Allow LLD to produce file symbols..
Mon, Apr 9, 4:46 AM
grimar created D45434: [ELF] - Eliminate AssertCommand..
Mon, Apr 9, 4:27 AM
grimar added inline comments to D44780: [ELF] - Implement linker script OVERLAYs..
Mon, Apr 9, 1:50 AM
grimar updated the diff for D44780: [ELF] - Implement linker script OVERLAYs..
  • Addressed review comments.
Mon, Apr 9, 1:50 AM

Fri, Apr 6

grimar created D45375: [ELF] - Introduce synthetic file for linker script symbols..
Fri, Apr 6, 8:43 AM
grimar added a reviewer for D45159: [ELF] - Fix cref.s test case.: espindola.
Fri, Apr 6, 7:34 AM
grimar created D45368: [ELF] - Stop setting OutSecOff too early..
Fri, Apr 6, 6:59 AM
grimar added inline comments to D44780: [ELF] - Implement linker script OVERLAYs..
Fri, Apr 6, 3:21 AM
grimar updated the diff for D44780: [ELF] - Implement linker script OVERLAYs..
  • Addressed review comments.
Fri, Apr 6, 3:21 AM
grimar added a comment to D45195: Add --check-library-dependency to maintain compatibility with other linkers.

Or maybe warn-backrefs (similarly to warn-common, warn-symbol-ordering and warn-unresolved-symbols).

Fri, Apr 6, 2:36 AM
grimar added a comment to D45195: Add --check-library-dependency to maintain compatibility with other linkers.

FWIW, --detect-backrefs sounds better for me than --backrefs.

Fri, Apr 6, 2:23 AM

Thu, Apr 5

grimar updated the diff for D45325: [ELF] - Reorder local symbols..
  • Finished unfinished comment in the test case.
Thu, Apr 5, 8:15 AM
grimar added a reviewer for D45325: [ELF] - Reorder local symbols.: pcc.
Thu, Apr 5, 8:11 AM
grimar created D45325: [ELF] - Reorder local symbols..
Thu, Apr 5, 8:11 AM
grimar added a comment to D45261: [ELF] - Allow LLD to produce file symbols..

STT_FILE
Conventionally, the symbol's name gives the name of the source file associated with the object file. A file symbol has STB_LOCAL binding, its section index is SHN_ABS, and it precedes the other STB_LOCAL symbols for the file, if it is present.

Yes, looks like we have to fix PR36716 in one go.

Thu, Apr 5, 8:05 AM
grimar updated the diff for D44780: [ELF] - Implement linker script OVERLAYs..
  • Rebased + simplified a bit.
Thu, Apr 5, 5:34 AM
grimar committed rL329275: [ELF] - Eliminate Target::isPicRel method..
[ELF] - Eliminate Target::isPicRel method.
Thu, Apr 5, 5:10 AM
grimar committed rLLD329275: [ELF] - Eliminate Target::isPicRel method..
[ELF] - Eliminate Target::isPicRel method.
Thu, Apr 5, 5:10 AM
grimar closed D45248: [ELF] - Eliminate Target::isPicRel method..
Thu, Apr 5, 5:10 AM
grimar created D45314: [ELF] - (-Map file) Implement printing of LMA for assignments outside of section declarations..
Thu, Apr 5, 4:55 AM
grimar committed rL329272: [ELF] - Reveal more information in -Map file about assignments..
[ELF] - Reveal more information in -Map file about assignments.
Thu, Apr 5, 4:30 AM
grimar committed rLLD329272: [ELF] - Reveal more information in -Map file about assignments..
[ELF] - Reveal more information in -Map file about assignments.
Thu, Apr 5, 4:30 AM
grimar closed D44894: [ELF] - Reveal more information in -Map file about assignments..
Thu, Apr 5, 4:30 AM
grimar committed rL329271: [ELF] - Print LMA in a -Map file..
[ELF] - Print LMA in a -Map file.
Thu, Apr 5, 3:56 AM
grimar committed rLLD329271: [ELF] - Print LMA in a -Map file..
[ELF] - Print LMA in a -Map file.
Thu, Apr 5, 3:56 AM
grimar closed D44899: [ELF] - Print LMA in a -Map file..
Thu, Apr 5, 3:56 AM
grimar added inline comments to D44780: [ELF] - Implement linker script OVERLAYs..
Thu, Apr 5, 3:26 AM
grimar updated the diff for D44780: [ELF] - Implement linker script OVERLAYs..
  • Addressed review comment.
Thu, Apr 5, 3:26 AM
grimar accepted D45264: [ELF] Don't put NOLOAD section into PT_LOAD.

LGTM

Thu, Apr 5, 1:58 AM · lld

Wed, Apr 4

grimar updated the diff for D45261: [ELF] - Allow LLD to produce file symbols..
  • Remove the file-sym.s test case.
Wed, Apr 4, 8:51 AM
grimar updated the summary of D45261: [ELF] - Allow LLD to produce file symbols..
Wed, Apr 4, 8:48 AM
grimar added a comment to D45261: [ELF] - Allow LLD to produce file symbols..

BTW, how come lld is larger than clang in your measurements?

Wed, Apr 4, 8:48 AM
grimar added a comment to D45261: [ELF] - Allow LLD to produce file symbols..

BTW, how come lld is larger than clang in your measurements?

Wed, Apr 4, 8:46 AM
grimar updated the summary of D45248: [ELF] - Eliminate Target::isPicRel method..
Wed, Apr 4, 8:38 AM
grimar updated the diff for D45248: [ELF] - Eliminate Target::isPicRel method..
  • Rebased.
  • Use R_*_NONE,
Wed, Apr 4, 8:37 AM
grimar updated the summary of D45261: [ELF] - Allow LLD to produce file symbols..
Wed, Apr 4, 8:24 AM
grimar committed rLLD329203: [ELF] - X86_64: Use white list for relocations checked by isPicRel..
[ELF] - X86_64: Use white list for relocations checked by isPicRel.
Wed, Apr 4, 8:24 AM
grimar committed rL329203: [ELF] - X86_64: Use white list for relocations checked by isPicRel..
[ELF] - X86_64: Use white list for relocations checked by isPicRel.
Wed, Apr 4, 8:24 AM
grimar closed D45252: [ELF] - X86_64: Use white list for relocations checked by isPicRel..
Wed, Apr 4, 8:24 AM
grimar created D45261: [ELF] - Allow LLD to produce file symbols..
Wed, Apr 4, 8:22 AM
grimar committed rL329180: [ELF] - Use early return. NFC..
[ELF] - Use early return. NFC.
Wed, Apr 4, 5:40 AM
grimar committed rLLD329180: [ELF] - Use early return. NFC..
[ELF] - Use early return. NFC.
Wed, Apr 4, 5:40 AM
grimar added a comment to D45248: [ELF] - Eliminate Target::isPicRel method..

btw, instead of Optional<> it could just return RelType R_*_NONE I think,
which is always zero for all targets it seems.
I am not sure that would make more sense though.

Wed, Apr 4, 4:59 AM
grimar created D45252: [ELF] - X86_64: Use white list for relocations checked by isPicRel..
Wed, Apr 4, 4:49 AM