Skip to content
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

compiler: Infer type generation from _C_ctype #1971

Merged
merged 13 commits into from Jul 28, 2022
Merged

Conversation

FabioLuporini
Copy link
Contributor

@FabioLuporini FabioLuporini commented Jul 23, 2022

CodeSymbol subclasses were required to override a subset of _C_typedata, _C_typename, _C_typedecl, _C_ctype. This was messy, difficult to understand, challenging to maintain, and also a bit hacky.

Now CodeSymbol subclasses are only requested to specialize _C_ctype. All other information are derived from _C_ctype through suitable utility functions (e.g., a str representation of the C type for code generation will be generated via the utility ctypes_to_cstr).

This is in my opinion a remarkable improvement to the codegen infrastracture, that was long due.

The ArrayMapped type is also introduced.

@review-notebook-app
Copy link

Check out this pull request on  ReviewNB

See visual diffs & provide feedback on Jupyter Notebooks.


Powered by ReviewNB

@FabioLuporini FabioLuporini changed the title compiler: Use C structs to generate ArrayMapped compiler: Infer type generation from _C_ctype Jul 23, 2022
@codecov
Copy link

codecov bot commented Jul 23, 2022

Codecov Report

Merging #1971 (9a5e60d) into master (1d98eb1) will increase coverage by 0.00%.
The diff coverage is 83.20%.

@@           Coverage Diff           @@
##           master    #1971   +/-   ##
=======================================
  Coverage   87.88%   87.88%           
=======================================
  Files         213      213           
  Lines       36196    36175   -21     
  Branches     5449     5459   +10     
=======================================
- Hits        31810    31793   -17     
+ Misses       3877     3867   -10     
- Partials      509      515    +6     
Impacted Files Coverage Δ
tests/test_gpu_common.py 1.50% <0.00%> (ø)
tests/test_operator.py 97.69% <ø> (-0.14%) ⬇️
devito/ir/iet/visitors.py 81.64% <80.85%> (+0.01%) ⬆️
devito/tools/utils.py 82.79% <82.92%> (+1.21%) ⬆️
devito/types/parallel.py 83.95% <83.33%> (+1.11%) ⬆️
devito/passes/iet/definitions.py 83.87% <84.74%> (-0.10%) ⬇️
devito/types/basic.py 94.27% <88.57%> (+<0.01%) ⬆️
devito/types/array.py 96.00% <95.00%> (+4.19%) ⬆️
devito/ir/iet/efunc.py 91.07% <100.00%> (+0.50%) ⬆️
devito/ir/iet/nodes.py 92.56% <100.00%> (-0.33%) ⬇️
... and 13 more

Help us with your feedback. Take ten seconds to tell us how you rate us.

_C_field_nbytes = 'nbytes'
_C_field_dmap = 'dmap'

_C_ctype = POINTER(type(_C_structname, (Structure,),
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you see? a ney symbolic type, ArrayMapped, but the only thing that needs to be defined is the _C_ctype. This is enough to create a ctype, a str representing its C type, etc etc.

@@ -1269,120 +1269,6 @@ def test_loose_kwargs(self):
@skipif('device')
class TestDeclarator(object):

def test_heap_1D(self):
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

all these tests were obsolete, a massive PITA to maintain, and basically unnecessary. Hence, dropping


def ctypes_to_cgen(ctype, fields=None):
"""
Create a cgen.Structure off a ctypes.Struct, or (possibly nested) ctypes.Pointer
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

gap after period

Copy link
Contributor

@georgebisbas georgebisbas left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Overall not any comment to make.

@FabioLuporini FabioLuporini merged commit 41b303a into master Jul 28, 2022
@FabioLuporini FabioLuporini deleted the array-struct branch July 28, 2022 15:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants