This is an archive of the discontinued LLVM Phabricator instance.

[StaticAnalyzer] LoopUnrolling: Excluding loops which splits the state (make more branches)
ClosedPublic

Authored by szepet on Aug 21 2017, 8:20 AM.

Details

Summary

Added check if the execution of the last step of the given unrolled loop has generated more branches. If yes, than treat it as a normal (non-unrolled) loop in the remaining part of the analysis.

Diff Detail

Repository
rL LLVM

Event Timeline

szepet created this revision.Aug 21 2017, 8:20 AM
NoQ edited edge metadata.Aug 24 2017, 7:27 AM

The heuristic makes perfect sense to me.

I think we could use some tests here.

lib/StaticAnalyzer/Core/LoopUnrolling.cpp
224–229 ↗(On Diff #111982)

Could you comment on whether it's possible that both ifs happen (LoopStmt's node has more than one successor) and if it is, why do we behave correctly in this case?

szepet updated this revision to Diff 112710.Aug 25 2017, 10:22 AM

Testfiles and function updated.

szepet added inline comments.Aug 25 2017, 10:24 AM
lib/StaticAnalyzer/Core/LoopUnrolling.cpp
224–229 ↗(On Diff #111982)

Hmm I ve intended to write BlockEntrance instead of BlockEdge and in this case it wont happen that both of the ifStmts is true.

NoQ accepted this revision.Aug 28 2017, 1:08 AM

LGTM!

I guess later it'd be great to comment on every numTimesReached() why is it supposed to be reached exactly that many times, as it's often unclear, especially with tricky control flow. Because if somebody accidentally breaks the test, they'd have no idea what it tests or why it should work this way.

This revision is now accepted and ready to land.Aug 28 2017, 1:08 AM
This revision was automatically updated to reflect the committed changes.