This is an archive of the discontinued LLVM Phabricator instance.

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.