This is an archive of the discontinued LLVM Phabricator instance.

[PECOFF] Support COMDAT associative sections.
ClosedPublic

Authored by ruiu on Jun 5 2014, 2:06 AM.

Details

Summary

This is a rework of r210240 to do it in the right way. Previously
it depended on dead-stripping pass to remove unneeded atoms, so
if the pass is not enabled it wouldn't work. This patch does not
depends on dead-stripping pass.

Background:
COFF supports a feature similar to ELF's section groups. This
patch implements it.

In ELF, section groups are identified by their names, and they are
treated somewhat differently from regular symbols. In COFF, the
feature is realized in a more straightforward way. A section can
have an annotation saying "if Nth section is linked, link this
section too." The annotation type is SELECT_ASSOCIATE.

Design:
A new method associate() is added to DefinedAtom. The new method
is supposed to return a list of atoms with SELECT_ASSOCIATE. The
Resolver adds live atom's associates() return values to the live
atom list. By default, assocaites() returns an empty list, so the
default behavior is the same as before.

Caveats:
The reason why I added a new member function associate() to
DefinedAtom, rather than adding a new reference type is mainly
performance. In order to collect all references of some type,
we have to scan all references of all atoms, which is more
costly than calling a member function on all atoms.

Diff Detail

Repository
rL LLVM

Event Timeline

ruiu updated this revision to Diff 10127.Jun 5 2014, 2:06 AM
ruiu retitled this revision from to [PECOFF] Support COMDAT associative sections..
ruiu updated this object.
ruiu edited the test plan for this revision. (Show Details)
ruiu added reviewers: Bigcheese, shankarke, atanasyan.
ruiu added a subscriber: Unknown Object (MLST).
shankarke edited edge metadata.Jun 5 2014, 9:47 AM

Could we use references to track the associated atoms, rather than adding a separate member to the defined atom ?

ruiu added a comment.Jun 5 2014, 9:23 PM

As I wrote in the description of this patch, I thought about that and actually tried
implementing, and concluded that reference is not suitable for that. It was slow
and unnecessarily complicated.

atanasyan edited edge metadata.Jun 5 2014, 9:37 PM

Is the assocaites() list serialized to YAML representation if I run lld with the --output-filetype=yaml option?

ruiu added a comment.Jun 5 2014, 9:40 PM

Good point, it doesn't. Will update the patch to serialize it from/to YAML.

ruiu updated this revision to Diff 10165.Jun 6 2014, 12:36 AM
ruiu edited edge metadata.
  • Serialize associates() list from/to YAML

You have to still get native reader/writer to emit this in native format.

Please check with Nick/Bigcheese before committing this change.

ruiu updated this revision to Diff 10338.Jun 11 2014, 4:11 PM

It turned out that dumping associate() contents to Native format
is too much, because it's not easy to dump variable length objects
to Native format. We would have had to define a new chunk for
associated atoms.

So I came back to the original design to use References to represent
"associate" relationships. I believe I found a way to implement it
in an efficient way. PTAL.

Bigcheese accepted this revision.Jun 16 2014, 6:32 PM
Bigcheese edited edge metadata.

lgtm

This revision is now accepted and ready to land.Jun 16 2014, 6:32 PM
ruiu closed this revision.Jun 17 2014, 9:27 AM
ruiu updated this revision to Diff 10505.

Closed by commit rL211106 (authored by @ruiu).