-
Notifications
You must be signed in to change notification settings - Fork 33
#1312 ScalarArray code generation #3101
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
bf523ba
dc1ec4a
862f368
1a07148
decf6c1
6d74811
71c7d0c
fa6a7e6
ae9ba03
4961046
bc8260f
8e2b3de
5b8c42d
ea0e8f7
3b6ad7d
33caeac
605d9ab
c9a5598
bbb37b7
9f3cfb5
8e311c2
ea6ae8d
136d89e
fd0d546
50a2c09
f4c685b
cc53390
f3ee59e
140b022
8527d01
39f96fb
3360be3
02a42b2
6cf25bc
a9dc367
75692cc
6edd5f3
12b4a74
c455482
184a624
20ecf84
35bc15a
3a6ddc6
e92ca50
a21d57e
1d6b212
1301f11
a73d33b
39b7cb9
9556c2b
4052702
86d3231
03c600a
b8c358f
2216993
0d6874b
06fbf55
aa7e39b
0dcd904
13bcee0
0cc2463
531e181
40aaf0e
18be0df
388d45e
f8d1a8a
502b618
da0a8d1
013463c
8249ade
4d503fa
42ff5b7
662ac2e
bfc3e47
463fcff
9828122
05b2725
abbed54
ec15259
f78a894
dc1bce9
5133a3c
7338d2a
6b265e2
0d1b97a
01f507c
afaebb6
cd61f83
eca5ad1
0566cce
6269aab
5e48f74
53111a9
279f882
b4d2647
7be0fa6
72b3026
699aeac
d10b34f
cee1d48
cc61781
c58d7cf
c2f572a
8bdf4d1
efca80b
553a610
8cd4dbb
03ea2f8
d546841
8b46182
a1e5111
5ccffee
bd82d29
3b167fc
889fefa
58ba124
e9c7b77
3febeb7
6af9a55
a7f3e98
9c8499d
49d89e5
5d92498
2741a4b
444d99a
b2d4553
adcdb37
d2a7ef9
f187466
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 |
|---|---|---|
|
|
@@ -261,7 +261,8 @@ def fs_intergrid(self, function_space, var_accesses=None): | |
| def scalar(self, scalar_arg, var_accesses=None): | ||
| ''' | ||
| Override the default implementation as there's no need to specify | ||
| scalars for an OpenACC data region. | ||
| scalar values for an OpenACC data region. But there is a need for | ||
| ScalarArrays. | ||
|
|
||
| :param scalar_arg: the kernel argument. | ||
| :type scalar_arg: :py:class:`psyclone.lfric.LFRicKernelArgument` | ||
|
|
@@ -271,6 +272,13 @@ def scalar(self, scalar_arg, var_accesses=None): | |
| :py:class:`psyclone.core.VariablesAccessMap`] | ||
|
|
||
| ''' | ||
| # TODO: Add implementation of OpenACC data region for ScalarArrays | ||
|
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. I suggest adding a check here and if we have a ScalarArray, raise a NotImplementedError. You'll need an associated test too I'm afraid.
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. Done
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. Please remove the TODO as otherwise our rule is that you need an associated Issue and I don't think we want another Issue :-)
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. Also, please document the NotImplementedError as a ":raises ...." in the docstring. |
||
| # If the argument is a simple scalar value then doesn't need values | ||
| # added to OpenACC region | ||
| if scalar_arg.is_scalar_array: | ||
| raise NotImplementedError( | ||
| "OpenACC data regions are not currently supported for arrays" | ||
| " of scalars.") | ||
|
|
||
|
|
||
| # ============================================================================ | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -252,6 +252,11 @@ def scalar(self, scalar_arg, | |
| if scalar_arg.is_literal: | ||
| self.psyir_append(scalar_arg.psyir_expression()) | ||
| else: | ||
| if scalar_arg.is_scalar_array: | ||
| # If it's a ScalarArray we need to add the dimensions | ||
| # array to the call | ||
| dims_sym = self._symtab.lookup("dims_" + scalar_arg.name) | ||
|
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. Probably safer to use "dims_"+scalar_arg.name as a tag and then do |
||
| self.psyir_append(Reference(dims_sym)) | ||
| sym = self._symtab.lookup(scalar_arg.name) | ||
| self.psyir_append(Reference(sym)) | ||
|
|
||
|
|
||
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.
We're going to need to add this to https://github.com/stfc/lfric_core and/or https://github.com/MetOffice/lfric_core.
Uh oh!
There was an error while loading. Please reload this page.
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.
I have a draft PR ready in
MetOffice/lfric_core- MetOffice/lfric_core#223. It'll need to go through review, but it's currently waiting for the PSyclone PR to be implemented.I'm not entirely sure the process you take to get it into
external/so let me know if you need me to make a PR instfc/lfric_coretoo