-
Notifications
You must be signed in to change notification settings - Fork 75
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Remove restriction that no loop unrolling is done when breaks are in the loop #1518
base: master
Are you sure you want to change the base?
Conversation
9cf2531
to
e656cc1
Compare
@@ -31,34 +31,25 @@ class checkNoBreakVisitor = object | |||
|
|||
end | |||
|
|||
let checkNoBreakStmt stmt = | |||
let visitor = new checkNoBreakVisitor in |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
checkNoBreakVisitor
itself is also unused now.
let checkNoBreakBlock block = | ||
let visitor = new checkNoBreakVisitor in | ||
ignore @@ visitCilBlock visitor block | ||
|
||
class findBreakVisitor(compOption: exp option ref) = object |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looking at this visitor now, I understand why it didn't want multiple break
s. It's main goal is to actually find the comparison for the loop, which will be an if
with a break
inside.
By only allowing one such break
it guaranteed that the result is unambiguous: there couldn't be multiple such comparisons found.
Now, if there are multiple matching if
s with break
s inside, this will still find the first one because compOption
is only assigned when it's None
, so that can happen at most once.
Is the loop condition always in the first matching one though? If so, then why did the old version have a problem at all? Or maybe there are loops where the condition isn't actually the first one?
In SV-COMP no-overflow tasks there are cases similar to the following example:
Where we do not detect loops with fixed iterations due to the restriction:
This PR removes this restriction, as reaching these breaks should make the loop iterations smaller anyway.
However, all these tasks also have the incrementcounter++
in the loop condition, which we do not handle and which is very difficult to handle using only the syntax, as these statements are picked into pieces by CIL:Thus, I opted for another assumption that when we find a fixed loop and its start but cannot detect the increment within the loop, we assume the increment is 1.This solved the tasks with the bounds being 5 and 10, but for bounds 20 and 50, I also had to apply the hack introduced in #1516 to just use the fixed loop size that was found by the heuristics if there are less than 10 loops in the program (the only stat I could find/access to estimate the size of the program (being small)).