This is an archive of the discontinued LLVM Phabricator instance.

[LTO] Fix the logic for dropping unnamed_addr
ClosedPublic

Authored by davide on Aug 30 2016, 9:19 AM.

Diff Detail

Repository
rL LLVM

Event Timeline

davide updated this revision to Diff 69713.Aug 30 2016, 9:19 AM
davide retitled this revision from to [LTO] Fix the logic for dropping unnamed_addr.
davide updated this object.
davide added a reviewer: rafael.
davide added a subscriber: llvm-commits.
rafael edited edge metadata.Aug 30 2016, 9:31 AM

testing email

The change to a test looks wrong.

test/ELF/lto/unnamed-addr-comdat.ll
11 ↗(On Diff #69713)

Why is it being dropped in this case?

Found the thinko, it should be

// Merge in the new unnamed_addr attribute.
if (WasInserted)
  S->HasUnnamedAddr = HasUnnamedAddr;
else
  S->HasUnnamedAddr &= HasUnnamedAddr;

Otherwise HasUnnamedAddr is not set correctly in the case you pointed out. Updating

davide updated this revision to Diff 69735.Aug 30 2016, 11:36 AM
davide edited edge metadata.
rafael added inline comments.Aug 30 2016, 11:42 AM
ELF/SymbolTable.cpp
222 ↗(On Diff #69735)

Maybe change this to true.

245 ↗(On Diff #69735)

And then you don't need this if, no?

davide updated this revision to Diff 69737.Aug 30 2016, 11:48 AM

sure, nice simplification.

rafael accepted this revision.Aug 30 2016, 1:12 PM
rafael edited edge metadata.
This revision is now accepted and ready to land.Aug 30 2016, 1:12 PM
This revision was automatically updated to reflect the committed changes.