-
Notifications
You must be signed in to change notification settings - Fork 33
Changes to OpenMP scripts to extract arguments from iom_put #3373
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 7 commits
52b962a
dae4e86
34aeaac
a95ece0
d79fae1
46d6dfa
6339e55
6ce8a38
7c98fd4
dd3e62a
e06176b
ed69361
af9ea3d
a912e88
78127da
c3804eb
0420aa4
470ed98
cf4947e
c6c718c
03e0ffa
93d814c
f147e94
339f8b4
d9cced4
51f5e71
4fb5d7b
65fa992
19d77ef
fa35a58
78b4345
93c0b20
51c250a
5ad2f3e
18dae4e
b89289f
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 |
|---|---|---|
|
|
@@ -41,14 +41,15 @@ | |
| from psyclone.domain.common.transformations import KernelModuleInlineTrans | ||
| from psyclone.psyir.nodes import ( | ||
| Assignment, Loop, Directive, Node, Reference, CodeBlock, Call, | ||
| Routine, Schedule, IntrinsicCall, StructureReference, IfBlock) | ||
| from psyclone.psyir.symbols import DataSymbol | ||
| Routine, Schedule, IntrinsicCall, StructureReference, IfBlock, | ||
| Operation) | ||
| from psyclone.psyir.symbols import DataSymbol, ArrayType | ||
| from psyclone.psyir.transformations import ( | ||
| ArrayAssignment2LoopsTrans, HoistLoopBoundExprTrans, HoistLocalArraysTrans, | ||
| HoistTrans, InlineTrans, Maxval2LoopTrans, ProfileTrans, | ||
| OMPMinimiseSyncTrans, Reference2ArrayRangeTrans, | ||
| ScalarisationTrans, IncreaseRankLoopArraysTrans, MaximalRegionTrans) | ||
| from psyclone.transformations import TransformationError | ||
| ScalarisationTrans, IncreaseRankLoopArraysTrans, MaximalRegionTrans, | ||
| DataNodeToTempTrans, TransformationError) | ||
|
|
||
| # USE statements to chase to gather additional symbol information. | ||
| NEMO_MODULES_TO_IMPORT = [ | ||
|
|
@@ -526,3 +527,17 @@ def _satisfies_minimum_region_rules(self, region: list[Node]) -> bool: | |
| routine_name = parent_routine.name if parent_routine else "" | ||
| if routine_name not in PROFILING_IGNORE: | ||
| MaximalProfilingOutsideDirectivesTrans().apply(children) | ||
|
|
||
|
|
||
| def iom_put_argument_to_temporary(calls: list[Call]): | ||
|
Collaborator
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. "param:" missing
Collaborator
Author
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. Added. I also have slightly changed the behaviour so I will rerun integration. |
||
| '''Extracts the second argument of all iom_put calls and puts them | ||
| in a temporary if they are an Operation with an array datatype.''' | ||
| for call in calls: | ||
| if call.symbol.name == "iom_put": | ||
| arg = call.arguments[1] | ||
| dtype = arg.datatype | ||
| if isinstance(dtype, ArrayType) and isinstance(arg, Operation): | ||
| try: | ||
| DataNodeToTempTrans().apply(arg) | ||
| except TransformationError: | ||
| pass | ||
|
Collaborator
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. The integration tests don't show any performance advantage which is not what we expected (we are changing from multiple gpu->cpu reads to one, and to maybe preventing the data touched from the gpu to be brought back):
Collaborator
Author
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. I've added a preceding comment now. I'll try generalising it as well.
Collaborator
Author
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. This currently causes stuff to fail, I'll try to see if I can get my VPN to start working again and see if I can try manually building NEMO5 to find the cause.
Collaborator
Author
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. This has shown a few issues with the DataNodeToTempTrans (partly because some things are
Collaborator
Author
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. Fixed the bugs now |
||
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.
Delete this now?
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.
Yeah.