compiler: Misc code generation improvements #2282

FabioLuporini · 2023-12-13T16:15:27Z

No description provided.

FabioLuporini · 2023-12-13T16:20:02Z

devito/passes/iet/langbase.py

@@ -215,6 +217,169 @@ def Prodder(self):
        return self.lang.Prodder


+class ShmTransformer(LangTransformer):


essentially just lifted from parpragma

codecov · 2023-12-13T16:20:21Z

Codecov Report

Attention: 83 lines in your changes are missing coverage. Please review.

Comparison is base (3126fb0) 86.85% compared to head (5f63560) 86.76%.

Files	Patch %	Lines
devito/arch/compiler.py	20.00%	39 Missing and 1 partial ⚠️
devito/ir/iet/visitors.py	70.42%	12 Missing and 9 partials ⚠️
devito/symbolics/extended_sympy.py	91.25%	2 Missing and 5 partials ⚠️
devito/ir/iet/nodes.py	84.61%	5 Missing and 1 partial ⚠️
devito/passes/iet/orchestration.py	0.00%	5 Missing ⚠️
devito/passes/iet/langbase.py	97.18%	1 Missing and 1 partial ⚠️
devito/symbolics/printer.py	66.66%	1 Missing and 1 partial ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #2282      +/-   ##
==========================================
- Coverage   86.85%   86.76%   -0.10%     
==========================================
  Files         229      229              
  Lines       42584    42884     +300     
  Branches     7900     7951      +51     
==========================================
+ Hits        36986    37207     +221     
- Misses       4942     5002      +60     
- Partials      656      675      +19

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

mloubout

Quick look and comments, will have another pass.

Overall looks fairly straighforward.

devito/symbolics/extended_sympy.py

mloubout · 2023-12-13T16:44:30Z

.github/workflows/docker-bases.yml

+          context: .
+          file: './docker/Dockerfile.cpu'
+          push: true
+          target: 'cpu-icx-sycl'


would drop the icx

I put it there because SYCL is an open standard and there are multiple implementations... though yeah, the only that really matters at this point is Intel's, I think...

mloubout · 2023-12-13T16:44:35Z

.github/workflows/docker-bases.yml

+          context: .
+          file: './docker/Dockerfile.cpu'
+          push: true
+          target: 'gpu-icx-sycl'


mloubout · 2023-12-13T16:45:20Z

devito/finite_differences/differentiable.py

@@ -682,11 +682,12 @@ def __init_finalize__(self, *args, **kwargs):
        assert isinstance(weights, (list, tuple, np.ndarray))

        # Normalize `weights`
-        weights = tuple(sympy.sympify(i) for i in weights)
+        from devito.symbolics import pow_to_mul  # noqa, sigh
+        weights = tuple(pow_to_mul(sympy.sympify(i)) for i in weights)


otherwise you get pow(h_x, 2) upon reconstructions

mloubout

Few minor comments left nearly GTG, Compiler init_finalize needs revert to CustomCompiler compatible.

Launched docker build will let you know when ready

mloubout · 2023-12-18T13:41:52Z

devito/arch/archinfo.py

@@ -29,7 +29,7 @@
           # Generic GPUs
           'AMDGPUX', 'NVIDIAX', 'INTELGPUX',
           # Intel GPUs
-           'PVC']
+           'PVC', 'MAX1100', 'MAX1550']


maybe a generic "MAX" by itself too

Adding INTELGPUMAX

mloubout · 2023-12-18T13:42:57Z

devito/arch/compiler.py

@@ -774,7 +782,7 @@ def __lookup_cmds__(self):
 class IntelKNLCompiler(IntelCompiler):

    def __init_finalize__(self, **kwargs):
-        IntelCompiler.__init_finalize__(self, **kwargs)
+        super().__init_finalize__(**kwargs)


No this needs to be IntelCompiler.__init_finalize__(self, **kwargs) or it breaks CustomCompiler

mloubout · 2023-12-18T13:43:10Z

devito/arch/compiler.py

@@ -787,41 +795,85 @@ def __init_finalize__(self, **kwargs):
 class OneapiCompiler(IntelCompiler):

    def __init_finalize__(self, **kwargs):
-        IntelCompiler.__init_finalize__(self, **kwargs)
+        super().__init_finalize__(**kwargs)


mloubout · 2023-12-18T13:44:52Z

devito/arch/compiler.py

+        # The Intel toolchain requires the I_MPI_OFFLOAD env var to be set
+        # to enable GPU-aware MPI (that is, passing device pointers to MPI calls)
+        if isinstance(platform, IntelDevice):
+            environ['I_MPI_OFFLOAD'] = '1'


there is no compiler flag for it? Seems "dangerous" to change environ like that

Unfortunately no compiler flag...

it is "dangerous" but devito would break anyway without this env var. It's what enables device pointer support in MPI calls ("GPU-aware MPI"), which is what we rely on (only sane choice today)

mloubout · 2023-12-18T13:49:49Z

devito/ir/iet/visitors.py

+                if body:
+                    body.append(c.Line())
+                body.extend(as_tuple(v))
+        body = flatten(body)


mloubout · 2023-12-18T13:56:40Z

devito/types/object.py

+    _C_modifier = None
+    """
+    A modifier added to the LocalObject's C declaration when the object appears
+    in a function signature. For example, a subclass my define `_C_modifier = '&'`


might define

mloubout · 2023-12-18T13:58:53Z

docker/Dockerfile.cpu

+
+# NOTE: the name of this file ends with ".cpu" but this is a GPU image.
+# It then feels a bit akward, so some restructuring might be needed


should probably make a Dockerfile.intel the same way we have an AMD and NVIDIA one and then rename this one as something like "Dockerfile.basic" or something with just the GCC in it

mloubout · 2023-12-18T14:00:50Z

tests/test_iet.py

+
+    # A LocalObject using both a template and a modifier
+    class SpecialObject(LocalObject):
+        dtype = CustomDtype('bar', ('int', 'float'), '&')


template=('int', 'float'), modifier='&' since this is the "example" a bit better to be explicit for new users/...

mloubout · 2023-12-18T14:02:02Z

tests/test_iet.py

+    lo4 = MyObject('obj4', cargs=(1, 2), initvalue=Macro('meh'))
+
+    # A LocalObject with generic sympy exprs used as constructor args
+    expr = sympy.Function('ceil')(FLOAT(Symbol(name='s'))**-1)


Note: we might wanna create place holder for all those ceil(type, floor(type .... for cleaner use

mloubout · 2023-12-18T14:03:54Z

tests/test_symbolics.py

@@ -287,6 +288,52 @@ def test_intdiv():
    assert ccode(v) == 'b*((a + b) / 2) + 3'


+def test_def_function():
+    foo0 = DefFunction('foo', arguments=['a', 'b'], template=['int'])


Should this allow template='int' or is list mandatory

FabioLuporini added the compiler label Dec 13, 2023

FabioLuporini requested a review from mloubout December 13, 2023 16:15

FabioLuporini commented Dec 13, 2023

View reviewed changes

mloubout reviewed Dec 13, 2023

View reviewed changes

FabioLuporini force-pushed the sycl-init branch from b857931 to b2387c4 Compare December 15, 2023 09:17

mloubout reviewed Dec 18, 2023

View reviewed changes

FabioLuporini force-pushed the sycl-init branch 3 times, most recently from 213d3a6 to 2ebdc2d Compare December 19, 2023 14:33

FabioLuporini added 20 commits December 20, 2023 09:07

api: Accept language=sycl (still no-op though)

bbe3f5d

arch: Tweak OneapiCompiler for SYCL

a0af11f

compiler: Separate out common code in PragmaShmTransformer

96f145d

compiler: Add Lambda handler to Uxreplace

465fd99

compiler: Fix Definition.expr_symbols

5e9e8a2

compiler: Improve Call-Lambda interaction

89c8514

compiler: Patch LocalObject.free_symbols

48368e5

compiler: Patch Definition.functions

7664589

compiler: Fix Call.defines

ecaa2aa

compiler: Add Lambda.{functions,defines}

b073fb0

compiler: Support namespaces in the generated code

bfd961b

arch: Introduce SyclCompiler

5113b3d

compiler: Fix FindSections for Lambda

1ef8f70

misc: Add docker images for sycl backend

6b0c1c5

compiler: Add ListMajor

be4b1bf

compiler: Enhance Lambda support

e7bf2a1

compiler: Enhance C++ codegen capabilities

6cbdbe9

compiler: Pump C++ codegen capabilities

3bc43ae

compiler: Fix LocalObject codegen

5842722

arch: Fix OneapiCompiler MPI commands

b31976c

FabioLuporini added 26 commits December 20, 2023 09:07

compiler: Distinguish between standalones and objs

00f8693

compiler: Fix LocalObject codegen

d322cdc

compiler: Add LocalObject.modifier

025bac4

compiler: Improve support for Call via FieldFrom* objs

c4e1d24

compiler: Allocate Weights array on the stack by default

34b16cb

compiler: Optimize weights coefficients at startup

dd349ab

arch: Fix OneapiCompiler.MPICC

7546ac0

arch: Improve SyclCompiler

2037fb4

compiler: Patch IntDiv construction

05bd196

compiler: Add Call.templates

5d7d0df

compiler: Patch Lambda visitors

ff7e480

arch: Set I_MPI_OFFLOAD w/ Intel MPI on GPUs

568cf3e

compiler: Fix factorization involving Objects

e748bc0

compiler: Enhance LocalObject

ac7c854

compiler: Generalize _alloc_mapped_array_on_high_bw_mem

35f78b7

compiler: Tweak orchestration

0e12b6f

misc: Rename sycl docker images

4988492

compiler: Refactor Rvalue

a72f64a

arch: Fix OneapiCompiler and SyclCompiler initialization

b4102d9

arch: Add INTELGPUMAX

9930c4e

arch: Fix CustomCompiler via Intel toolchain

e7d5d34

compiler: Remove unnecessary code

632319c

misc: Fix docstring

762ea3d

tests: Use kwargs to make API explicit

796b05f

misc: Split Dockerfile.cpu into .cpu and .intel

b815b03

misc: Fix pep8

dfd7968

FabioLuporini force-pushed the sycl-init branch from 2ebdc2d to dfd7968 Compare December 20, 2023 09:10

docker: update oneapi base drivers

5f63560

mloubout merged commit c888cee into master Dec 21, 2023
35 checks passed

mloubout deleted the sycl-init branch December 21, 2023 20:38

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

compiler: Misc code generation improvements #2282

compiler: Misc code generation improvements #2282

FabioLuporini commented Dec 13, 2023

FabioLuporini Dec 13, 2023

codecov bot commented Dec 13, 2023 •

edited

Loading

mloubout left a comment

mloubout Dec 13, 2023

FabioLuporini Dec 15, 2023

mloubout Dec 13, 2023

FabioLuporini Dec 15, 2023

mloubout Dec 13, 2023

FabioLuporini Dec 15, 2023

mloubout left a comment

mloubout Dec 18, 2023

FabioLuporini Dec 19, 2023

mloubout Dec 18, 2023

FabioLuporini Dec 19, 2023

mloubout Dec 18, 2023

FabioLuporini Dec 19, 2023

mloubout Dec 18, 2023

FabioLuporini Dec 19, 2023

mloubout Dec 18, 2023

FabioLuporini Dec 19, 2023

mloubout Dec 18, 2023

FabioLuporini Dec 19, 2023

mloubout Dec 18, 2023

FabioLuporini Dec 19, 2023

mloubout Dec 18, 2023

FabioLuporini Dec 19, 2023

mloubout Dec 18, 2023

mloubout Dec 18, 2023

		@@ -215,6 +217,169 @@ def Prodder(self):
		return self.lang.Prodder


		class ShmTransformer(LangTransformer):


		# NOTE: the name of this file ends with ".cpu" but this is a GPU image.
		# It then feels a bit akward, so some restructuring might be needed

compiler: Misc code generation improvements #2282

compiler: Misc code generation improvements #2282

Conversation

FabioLuporini commented Dec 13, 2023

Choose a reason for hiding this comment

codecov bot commented Dec 13, 2023 • edited Loading

Codecov Report

mloubout left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mloubout left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

codecov bot commented Dec 13, 2023 •

edited

Loading