Minor bugfixes for GlobalVarOffloadTransformation #204
Conversation
… error over nonetype and prepend new imports to avoid imports after implicit none statement
| @@ -370,7 +369,7 @@ def process_driver(self, routine, successors): | |||
| 'enter_data_create': 'enter data create', | |||
| } | |||
| for key, directive in key_directive_map.items(): | |||
| variables = set.union(*[s.trafo_data.get(self._key, {}).get(key) for s in successors], set()) | |||
| variables = set.union(*[s.trafo_data.get(self._key, {}).get(key, {}) for s in successors], set()) | |||
There was a problem hiding this comment.
This change avoids an iteration error over Nones, since in the original version, the get(key) can return a None
| import_pos = 0 | ||
| if (old_imports := FindNodes(Import).visit(routine.spec)): | ||
| import_pos = routine.spec.body.index(old_imports[-1]) + 1 | ||
| if new_imports: | ||
| routine.spec.insert(import_pos, Comment(text= | ||
| routine.spec.prepend(new_imports) | ||
| routine.spec.prepend(Comment(text= | ||
| '![Loki::GlobalVarOffload].....Adding global variables to driver symbol table for offload instructions')) | ||
| import_pos += 1 | ||
| routine.spec.insert(import_pos, new_imports) |
There was a problem hiding this comment.
This change modifies the way the module variable imports are added to the driver routine. The original inserts them to the specification part. The problem with this is that it may add the imports after IMPLICIT NONE, which results in errors when compiling the transformed code. The suggested code avoids this by prepending the imports.
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #204 +/- ##
=======================================
Coverage 92.26% 92.26%
=======================================
Files 96 96
Lines 17092 17087 -5
=======================================
- Hits 15770 15766 -4
+ Misses 1322 1321 -1
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
|
Hi Santeri, many thanks, these changes look good. Unfortunately, I had to undertake a refactoring of the entire transformation, which has been submitted in #207. Would you mind confirming that the refactored transformation works as expected on ACRANEB2? |
|
@reuterbal, great, thanks! I will check what happens with your PR and let you know the results (EDIT: I encountered no issues with your PR!). |
|
Closing this, issue has been resolved as part of the refactoring in #207 |
Hello.
This PR includes two minor bugfixes for
GlobalVarOffloadTransformationthat came up when I used it with the ACRANEB2 dwarf. I discuss the changes in comments below.