Page MenuHomePhabricator

Remove the gc.root's findCustomSafePoints mechanism
ClosedPublic

Authored by reames on Jan 15 2015, 11:56 AM.

Details

Summary

Searching all of the existing gc.root implementations I'm aware of (all three of them), there was exactly one use of this mechanism, and that was to implement a performance improvement that should have been applied to the default lowering.

Having this function is requiring a dependency on a CodeGen class (MachineFunction), in a class which is otherwise completely independent of CodeGen. I could solve this differently, but given that I see absolutely no value in preserving this mechanism, I'd like to just get rid of it.

The code change is pretty straight forward, but I wanted to get input before submission because this is the first time I'm intentionally breaking previously supported gc.root functionality. Given 3.6 has branched, I believe this is a good time to do this.

This is blocking an update to http://reviews.llvm.org/D6811

Diff Detail

Repository
rL LLVM

Event Timeline

reames updated this revision to Diff 18241.Jan 15 2015, 11:56 AM
reames retitled this revision from to Remove the gc.root's findCustomSafePoints mechanism.
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 added inline comments.Jan 15 2015, 1:19 PM
lib/CodeGen/ErlangGC.cpp
59 ↗(On Diff #18241)

Do I understand it correctly that the GC_LABEL MI will be inserted elsewhere for this safepoint?

Not quite sure what you're asking...

The behaviour of the ErlangGC did not change. It's still using
post-call labels. (Or at least, I didn't *intend* to change it. If I
did, please point it out.) It's just reaching that result through the
(slightly modified) generic codepath.

whitequark accepted this revision.Jan 15 2015, 2:14 PM
whitequark edited edge metadata.

Ok, I see. LGTM.

This revision is now accepted and ready to land.Jan 15 2015, 2:14 PM
This revision was automatically updated to reflect the committed changes.