-
Notifications
You must be signed in to change notification settings - Fork 0
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
FuncPtrCall #4
FuncPtrCall #4
Conversation
In PETSc, I need to be able generate the following command:
where |
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.
bunch of minor comments but otherwise GTG
devito/ir/iet/nodes.py
Outdated
@@ -337,6 +337,19 @@ def writes(self): | |||
return self._writes | |||
|
|||
|
|||
class FuncPtrCall(Call): |
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 would call it "CallablePtr" or simply "Callback" ?
Also, I don't think it should inherit from Call. Why should it?
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.
Can you also (nitpicking) move this class further down in this file, ideally somewhere around the "second level IET nodes" (https://github.com/devitocodes/devito/blob/master/devito/ir/iet/nodes.py#L832)
perhaps after Lambda?
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.
It seemed to be the simplest/easiest way to bind the callback to an actual call
devito/ir/iet/visitors.py
Outdated
name = o.name | ||
return_type = o.return_type | ||
parameter_type = o.parameter_type | ||
return FuncPtrArg(name, return_type, parameter_type) |
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.
simply return FuncPtrArg(o.name, o.rettype, ...)
devito/ir/iet/nodes.py
Outdated
|
||
super().__init__(name=name) | ||
|
||
self.return_type = return_type |
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.
since we have retobj
in a lot of places in this module, how about rettype
?
we also use the word "rettype" here https://github.com/devitocodes/devito/blob/master/devito/ir/iet/visitors.py#L268
so for homogeneity...
(clearly still nitpicking here)
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.
actually we call it retval
here, sigh:
https://github.com/devitocodes/devito/blob/master/devito/ir/iet/nodes.py#L687
so up to u I guess
devito/ir/iet/nodes.py
Outdated
super().__init__(name=name) | ||
|
||
self.return_type = return_type | ||
self.parameter_type = parameter_type |
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.
parameterS ?
or simply "signature" or "param_types"?
use as_tuple
for flexibility
bf6b263
to
83c2b4f
Compare
Adding a comment for clarity: The reason Callback is an IET type rather than a SymPy type is due to the fact that, when represented at the SymPy level, the IET engine fails to bind the callback to a specific Call. Consequently, errors occur during the creation of the call graph because |
tests/test_iet.py
Outdated
code0 = CGen().visit(foo0_arg) | ||
assert str(code0) == '(void (*)(int))foo0' | ||
|
||
# test nested calls with a Callback as an argument. |
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.
Nitpick: start comments with a captial
devito/ir/iet/nodes.py
Outdated
@@ -1097,6 +1097,36 @@ def defines(self): | |||
return tuple(self.parameters) | |||
|
|||
|
|||
class Callback(Call): | |||
|
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.
nitpicking: no blank line here
devito/ir/iet/nodes.py
Outdated
""" | ||
|
||
def __init__(self, name, retval, param_types): | ||
|
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.
nitpicking: no blank line here
74520aa
to
54262db
Compare
No description provided.