Page MenuHomePhabricator

Please use GitHub pull requests for new patches. Avoid migrating existing patches. Phabricator shutdown timeline

Have isKnownNotFullPoison be smarter around control flow

Authored by sanjoy on Apr 17 2016, 8:52 PM.



(... while still not using a PostDomTree)

The way we use isKnownNotFullPoison from SCEV today, the new CFG walking
logic will not trigger for any realistic cases -- it will kick in only
for situations where we could have merged the contiguous basic blocks
anyway[0], since the poison generating instruction dominates all of its
non-PHI uses (which are the only uses we consider right now).

However, having this change in place will allow a later bugfix to break
fewer llvm-lit tests.

[0]: i.e. cases where block A branches to block B and B is A's only
successor and A is B's only predecessor.

Diff Detail


Event Timeline

sanjoy updated this revision to Diff 54029.Apr 17 2016, 8:52 PM
sanjoy retitled this revision from to Have isKnownNotFullPoison be smarter around control flow.
sanjoy updated this object.
sanjoy added reviewers: broune, bjarke.roune.
sanjoy added a subscriber: llvm-commits.
broune accepted this revision.Apr 21 2016, 5:14 PM
broune edited edge metadata.
broune added inline comments.
3413 ↗(On Diff #54029)

As far as I can tell, this code for Br is not in the official repository, yet it is not listed as a changed line in the review.

Maybe I'm just confused about how diffs are shown or this was added very recently?

3442 ↗(On Diff #54029)

Would it make sense to have a limit on how many instructions/basic blocks this will consider?

3462 ↗(On Diff #54029)

Could there be braces for this multi-line if?

This revision is now accepted and ready to land.Apr 21 2016, 5:14 PM
sanjoy added inline comments.Apr 21 2016, 5:18 PM
3413 ↗(On Diff #54029)

This is my fault. I had this revision based off of D19211 but didn't note it in the revision.

sanjoy marked an inline comment as done.Apr 21 2016, 11:39 PM
sanjoy added inline comments.
3442 ↗(On Diff #54029)

Added a limit of MaxDepth blocks (for the lack of a better limit).

This revision was automatically updated to reflect the committed changes.