Page MenuHomePhabricator

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

Check latch block terminator instruction is a branch instruction
Needs ReviewPublic

Authored by zinob on Oct 19 2016, 5:13 PM.



Diff Detail


Event Timeline

zinob updated this revision to Diff 75258.Oct 19 2016, 5:13 PM
zinob retitled this revision from to Check latch block terminator instruction is a branch instruction.
zinob updated this object.
zinob added reviewers: grosser, Unknown Object (User), Meinersbur, chrisj.
zinob set the repository for this revision to rL LLVM.
zinob added a subscriber: pollydev.
etherzhhb added inline comments.

I think we can return false right after "BranchInst *BI = dyn_cast<BranchInst>(TI);" if BI is nullptr.

grosser edited edge metadata.Oct 19 2016, 6:44 PM

Also, any chance we can get a test case for this one?

Meinersbur added inline comments.Oct 20 2016, 4:56 AM

If I see correctly, this also fixes a leak?

Does this mean in none of our tests buildConditionSets returns false because ISL would have noticed leaked objects?


Yes, it seems there is indeed a leak. I remember I saw it in my static analysis run, but did not get to report it:

Yes unit test misses this path. I will try to narrow down a test for it.

jdoerfert added inline comments.

@zinob Please add the full diff context so people can see surrounding code and notice that the !BI check is already performed 4 lines prior [In the else case BI is not null].
@etherzhhb There is already a check but otherwise you'r right :)

grosser resigned from this revision.Nov 29 2016, 12:00 PM
grosser removed a reviewer: grosser.

This has been resolved in r286426 "Do not allow switch statements in loop latches". I cannot close this myself. Zino can you please close.

Meinersbur resigned from this revision.Mar 8 2017, 3:41 AM