This is an archive of the discontinued LLVM Phabricator instance.

[RS4GC] Refactor code for Rematerializing in presence of phi. NFC
ClosedPublic

Authored by anna on Sep 20 2016, 1:08 PM.

Details

Summary

This is an NFC refactoring change as a precursor to the actual fix for rematerializing in
presence of phi.
https://reviews.llvm.org/D24399

Pasted from review:
findRematerializableChainToBasePointer changed to return the root of the
chain. instead of true or false.
move the PHI matching logic into the caller by inspecting the root return value.
This includes an assertion that the alternate root is in the liveset for the
call.

Tested with current RS4GC tests.

Diff Detail

Event Timeline

anna updated this revision to Diff 71972.Sep 20 2016, 1:08 PM
anna retitled this revision from to [RS4GC] Refactor code for Rematerializing in presence of phi. NFC.
anna updated this object.
anna added reviewers: reames, sanjoy.
anna added a subscriber: llvm-commits.
reames added inline comments.Sep 20 2016, 1:28 PM
lib/Transforms/Scalar/RewriteStatepointsForGC.cpp
1818

Not quite what I had in mind, but this works. I'd expected you to remove the BaseValue parameter at the same time. I'd expected the return value to basically be the first instruction we couldn't prove was part of a rematerializable chain. The caller would then look to see if that was something it knew how to handle.

1913

The nesting here gets confusing. I'd suggest extracting a isEquivelentPHI helper. This might already exist in fact.

anna updated this revision to Diff 71986.Sep 20 2016, 2:30 PM

[RS4GC] Refactor code for Rematerializing in presence of phi. NFC
Addressed both review comments above.

reames accepted this revision.Sep 20 2016, 2:35 PM
reames edited edge metadata.

LGTM. The semantic fix will be much more obvious when rebased on top of this. Thanks!

This revision is now accepted and ready to land.Sep 20 2016, 2:35 PM
This revision was automatically updated to reflect the committed changes.