-
Notifications
You must be signed in to change notification settings - Fork 88
Split octagon autotuner to separate octagonAnalysis and octagonVars autotuners #1952
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
base: master
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,4 +1,4 @@ | ||
| // SKIP PARAM: --enable ana.sv-comp.functions --enable ana.autotune.enabled --set ana.autotune.activated[+] octagon | ||
| // SKIP PARAM: --enable ana.sv-comp.functions --enable ana.autotune.enabled --set ana.autotune.activated[+] octagonVars | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why just And perhaps |
||
| // Extracted from: nla-digbench-scaling/sqrt1-ll_unwindbound5.c | ||
| #include <goblint.h> | ||
| extern int __VERIFIER_nondet_int(void); | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,10 +1,17 @@ | ||
| Should not annotate functions for octagon. | ||
|
|
||
| $ goblint --enable ana.autotune.enabled --set ana.autotune.activated[*] octagon 38-autotune-octagon-fun.c | ||
| [Info] Enabled octagon domain ONLY for: | ||
| [Info] i, count, tmp, count, i, j, i___0, j___0, k, size, r | ||
| $ goblint --enable ana.autotune.enabled --set ana.autotune.activated[*] octagonAnalysis 38-autotune-octagon-fun.c | ||
| [Info] Enabled octagon domain. | ||
| [Info][Deadcode] Logical lines of code (LLoC) summary: | ||
| live: 5 | ||
| dead: 0 | ||
| total lines: 5 | ||
|
|
||
| $ goblint --enable ana.autotune.enabled --set ana.autotune.activated[*] octagonVars 38-autotune-octagon-fun.c | ||
| [Info] Enabled octagon domain. | ||
|
Comment on lines
+10
to
+11
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. So I think for #1449 they should be independent. |
||
| [Info] Restricted octagon analysis to following tracked variables: | ||
| [Info] i, count, tmp, count, i, j, i___0, j___0, k, size, r | ||
| [Info][Deadcode] Logical lines of code (LLoC) summary: | ||
| live: 5 | ||
| dead: 0 | ||
| total lines: 5 | ||
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.
This doesn't seem right without the costs thing. It will always enable octagons if
octagonAnalysisautotuner is activated.That's not much of an autotuner if it just unconditionally activates things which could be activated directly.
I think I wasn't aware of this dependency when I opened #1449. Maybe it's actually impossible to separate them then?
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.
So, what I did was to just implement the requirements from the issue:
where the main problem was that the option
octagon:Due to the separation, the confusing behavior is gone as the option names now better reflect what they actually do.
However, it is true that I did not really consider that the autotuner should actually tune something with each option, which is not the case for
octagonAnalysisnow.The reason I separated it the way I did is that the cost calculation depends on the number of selected variables. At first glance, this made it seem like the cost belonged purely to the variable selection step. Because of that, I overlooked the fact that the cost value is later used.
Looking at this again from the perspective of autotuning, I see two possible approaches:
octagonAnalysiscould consider all variables present in the program, but I am not sure if we can count these easily with some visitor. I am also not sure if the cost will then ever stay under some threshold that the analysis would even be considered (I haven't looked into how the cost is used).octagonAnalysismeaningfully Instead of simply enabling it through a portfolio configuration, the tuner could decide whether activating it makes sense based not only on the number of variables but also on their structure, e.g., the presence of patterns that would require relational analyses likey = x + 1;.It definitely is not impossible to separate them cleanly. The question is whether this change is already worthwhile because it makes the options less confusing, even though one of them does not actually tune anything yet, or whether we should wait and introduce that separation only once we also improve the autotuning of enabling
octagonin some way.