This is an archive of the discontinued LLVM Phabricator instance.

Remove the Module pointer from GCStrategy and GCMetadataPrinter
ClosedPublic

Authored by reames on Dec 8 2014, 10:58 AM.

Details

Summary

In the current implementation, GCStrategy is a part of the ownership structure for the gc metadata which describes a Module. It also contains a reference to the module in question. As a result, GCStrategy instances are essentially Module specific.

I plan to transition away from this design. Instead, a GCStrategy will be owned by the LLVMContext. It will be a lightweight policy object which contains no information about the Modules or Functions involved, but can be easily reached given a Function.

The first step in this transition is to remove the direct Module reference from GCStrategy. This also requires removing the single user of this reference, the GCMetadataPrinter hierarchy. In theory, this will allow the lifetime of the printers to be scoped to the LLVMContext as well, but in practice, I'm not actually changing that. (Yet?)

An alternate design would have been to move the direct Module reference into the GCMetadataPrinter and change the keying of the owning maps to explicitly key off both GCStrategy and Module. I'm open to doing it that way instead, but didn't see much value in preserving the per Module association for GCMetadataPrinters.

The next change in this sequence will be to start unwinding the intertwined ownership between GCStrategy, GCModuleInfo, and GCFunctionInfo.

Diff Detail

Event Timeline

reames updated this revision to Diff 17043.Dec 8 2014, 10:58 AM
reames retitled this revision from to Remove the Module pointer from GCStrategy and GCMetadataPrinter.
reames updated this object.
reames edited the test plan for this revision. (Show Details)
reames added a reviewer: whitequark.
reames added a subscriber: Unknown Object (MLST).
whitequark accepted this revision.Dec 9 2014, 12:10 AM
whitequark edited edge metadata.

LGTM

This revision is now accepted and ready to land.Dec 9 2014, 12:10 AM
reames closed this revision.Dec 9 2014, 3:58 PM
reames updated this revision to Diff 17099.

Closed by commit rL223859 (authored by @reames).