This is an archive of the discontinued LLVM Phabricator instance.

ELF: Make --print-icf-sections output deterministic.
ClosedPublic

Authored by pcc on Jul 26 2018, 3:12 PM.

Details

Summary

The icf-safe.s test currently fails on 32-bit platforms because it uses
the --print-icf-sections flag and depends on the output appearing in
a specific order. However, this flag causes the output to depend on
the order of the sections in the Sections array, which depends on the
hash values returned from hash_combine, which happen to be different
for that test between 32-bit and 64-bit platforms.

This change makes the output deterministic by textually sorting the
printed messages.

Diff Detail

Repository
rL LLVM

Event Timeline

pcc created this revision.Jul 26 2018, 3:12 PM
ruiu added a comment.Jul 26 2018, 3:18 PM

I think we should use a deterministic hash function such as xxHash64 instead of hash_combine to fix this issue, as I believe it's easier and more robust.

pcc added a comment.Jul 26 2018, 3:23 PM

I think we should use a deterministic hash function such as xxHash64 instead of hash_combine to fix this issue, as I believe it's easier and more robust.

So hashing the section contents instead of contents + flags etc like we're currently doing? Seems reasonable, I'll give that a try.

ruiu added a comment.Jul 26 2018, 3:45 PM

If there's an easy way of mixing flags and other info to a hash value of contents, I think there's no reason not to do that, but perhaps just hashing contents is enough.

pcc updated this revision to Diff 157602.Jul 26 2018, 4:26 PM
  • Switch to xxhash
ruiu accepted this revision.Jul 26 2018, 4:29 PM

LGTM

This revision is now accepted and ready to land.Jul 26 2018, 4:29 PM
pcc added a comment.Jul 26 2018, 4:30 PM

I suspect that we aren't gaining much from the flags/relocation count/section size because all of these properties typically depend on the section contents anyway, so I left them out.

This revision was automatically updated to reflect the committed changes.