Unroll negative loop bounds and retain pragmas inside unrolled loop body #443
Conversation
|
Documentation for this branch can be viewed at https://sites.ecmwf.int/docs/loki/443/index.html |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #443 +/- ##
=======================================
Coverage 93.32% 93.33%
=======================================
Files 212 212
Lines 40769 40810 +41
=======================================
+ Hits 38047 38089 +42
+ Misses 2722 2721 -1
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚨 Try these New Features:
|
|
Can I please push an update here? I've thought of an edge case that would break the updates herein. |
awnawab
force-pushed
the
naan-loop-unroll-fixes
branch
from
bac536f to
6fae2b4
Compare
| _pragma = o.pragma if o.pragma and not 'unroll' in o.pragma[0].content.lower() else None | ||
| _pragma_post = o.pragma_post if o.pragma_post and not 'unroll' in o.pragma_post[0].content.lower() else None |
There was a problem hiding this comment.
Do we always have a tuple for o.pragma/o.pragma_post? (if not, we should for consistency). Are we otherwise certain it will always be the first entry in the pragma list? Also, the plain check for unroll in the text is guaranteed to cause aliasing issues - e.g. in an !$omp parallel do private (funroller)
I would suggest something like this:
| _pragma = o.pragma if o.pragma and not 'unroll' in o.pragma[0].content.lower() else None | |
| _pragma_post = o.pragma_post if o.pragma_post and not 'unroll' in o.pragma_post[0].content.lower() else None | |
| _pragma = tuple( | |
| p for p in as_tuple(o.pragma) | |
| if not (p.keyword.lower() == 'loki' and p.content.lower().startswith('loop-urnoll') | |
| ) or None | |
| _pragma_post = tuple( | |
| p for p in as_tuple(o.pragma_post) | |
| if not (p.keyword.lower() == 'loki' and p.content.lower().startswith('loop-urnoll') | |
| ) or None |
There was a problem hiding this comment.
Thanks, that is indeed much safer! And yup, pragma/pragma_post is always a tuple.
awnawab
force-pushed
the
naan-loop-unroll-fixes
branch
from
6fae2b4 to
f723554
Compare
There was a problem hiding this comment.
Many thanks for taking care of the comment - and I like that you have even further cleaned-up my proposal via the use of pragma utilities!
Also, I should have applauded earlier on how super-clean the implementation: The use of is_constant and get_pyrange is really neat!
reuterbal
added
the
ready to merge
This PR provides the following two fixes to the loop unrolling functionality: