Page MenuHomePhabricator

Check latch block terminator instruction is a branch instruction
Needs ReviewPublic

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

Details

Reviewers
chrisj

Diff Detail

Repository
rL LLVM

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.
lib/Analysis/ScopInfo.cpp
2795

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
lib/Analysis/ScopInfo.cpp
2798–2799

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?

@Meinersbur

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:

http://www-oldurls.inf.ethz.ch/personal/tgrosser/polly-static-analysis-delicm/report-7a9ee7.html#EndPath

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

jdoerfert added inline comments.
lib/Analysis/ScopInfo.cpp
2795

@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