(Closes #3334 #1013) Permit only module-inlined Kernels to be transformed by arporter · Pull Request #3294 · stfc/PSyclone

arporter · 2026-01-21T17:11:42Z

Require that all kernel transformations act only on module-inlined Kernels.
Do away with the 'rename-and-write' functionality currently used for modified kernels.
We have to keep the -okern command-line flag as that is used when creating OpenCL versions of Kernels.

arporter · 2026-01-21T19:26:52Z

Possibly I need to store the RoutineSymbol associated with a Kernel within the Kernel class (c.f. Call.reference). That way, I can tell whether it has been module-inlined or not and can get rid of the special flag for that.

arporter · 2026-01-23T15:39:02Z

Note to self: I will need to remove the kernel_outputdir test fixture.

EDIT: can't do this because we do still output OpenCL versions of kernels in some tests.

codecov · 2026-01-27T16:54:00Z

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 99.95%. Comparing base (97543bc) to head (0949296).
⚠️ Report is 15 commits behind head on master.

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #3294      +/-   ##
==========================================
- Coverage   99.95%   99.95%   -0.01%     
==========================================
  Files         384      385       +1     
  Lines       54225    54087     -138     
==========================================
- Hits        54203    54065     -138     
  Misses         22       22

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

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

arporter · 2026-02-04T13:35:42Z

Coverage is now all good but the LFRic extraction integration test failed :-(

arporter · 2026-02-04T15:55:05Z

In lfric/eg17/full_example_extract:

$ diff main_psy.f90 on_main/main_psy.f90 
4d3
<   use testkern_w0_kernel_mod, only : testkern_w0_code
74a74
>     use testkern_w0_kernel_mod, only : some_other_var, testkern_w0_code

so it appears the some_other_var import is missing on my branch.

…n outer scope

arporter · 2026-02-04T17:16:47Z

This turned out to be due to the fact that I've moved the import of Kernel routines up into the Container and then the ExtractNode was falling foul of #1734

arporter · 2026-02-20T10:05:39Z

Doc failure was a timeout on the NVIDIA web site. It works OK for me though.

sergisiso

@arporter It's good to see this (even bigger) clean up.

I just have a some questions about why some lines were changed and it may be good to have a plan for when the KernelModuleInlineTrans is made generic (see inline).

sergisiso · 2026-02-23T12:55:03Z

src/psyclone/psyir/transformations/omp_declare_target_trans.py

-            # Flag that the kernel has been modified
-            node.modified = True


There is one more use in:

tutorial/practicals/LFRic/single_node/3_sequential/matvec_opt.py 67: kernel.modified = True

Which made me realise this example will also need the ModuleInlining (and the kernel_schedule.walk(IntrinsicCall) is also broken because kernel.get_callees() returns a list now)
(also the Makefile commands have the --psykal-dsl lfric missing)

Grepping Matmul2CodeTrans, also examples/lfric/eg15/matvec_opt.py may be missing ModuleInling

I see from #1015 that this has been broken since 2020. Perhaps we should just remove this material in favour of @hiker's recent training material?

Oh, eg15 shows up a bit of a problem: there's nothing stopping a user getting the kernel schedule and then applying transformations to it themselves. However, those transformations will now have no effect as the modified PSyIR will never be written to file :-(

Anyway, I've now fixed-up the example and the tutorial.

sergisiso · 2026-02-23T13:01:04Z

src/psyclone/tests/domain/common/transformations/kernel_module_inline_trans_test.py

    use quadrature_xyoz_mod, only : quadrature_xyoz_proxy_type, \
-quadrature_xyoz_type
-    use testkern_qr_mod, only : testkern_qr_code""" in output)
+quadrature_xyoz_type""" in output)


pylint says "superfluous-parens"

sergisiso · 2026-02-23T13:19:25Z

src/psyclone/domain/common/transformations/kernel_transformation_mixin.py

+
+        """
+        if not isinstance(node, CodedKern):
+            return


When I first saw that I though this is to let Builtins pass, but then I found somewhere that this is provided with a call_node as returns None. But then I realised there is nothing here specific to kernels. If this a transfomration is provided a call and its callee is being modified, the same check must be done. Maybe it's worth it doing this generic: class CalleeTransformationMixin and _check_callee_implementation_is_local ?

Then I would do this more strict and not return None for everything else, if somewhere a builtin is valid I would do:

if not isinstance(node, Builtin): node.check_callee_implementation_is_local(node)

What do you think?

(I have not look everywhere this is used, it could be good to check is applicable everywhere before doing the change)

Oh I also see that the KernelModuleInlineTrans that the error message suggest is only applicable to kernels. So maybe this can be left with a TODO together with the plans to make KernelModuleInlineTrans generic which we have discussed in the past (maybe we could repurpose #2683 to include this?)

(I would still rename the method to check_callee_implementation_is_local and make it stricter now)

Good points. I have generalised it, renamed it and moved it into psyir.transformations. In some places a Routine is valid so I've added checks for those.

sergisiso · 2026-02-23T14:48:00Z

src/psyclone/domain/gocean/transformations/gocean_opencl_trans.py

+                global_variables = set(sym.name for sym in
+                                       ksched.symbol_table.imported_symbols)
+                prec_sym_names = set(sym.name for sym in
+                                     ksched.symbol_table.precision_datasymbols)
+                non_prec_vars = global_variables.difference(prec_sym_names)
+                if non_prec_vars:
+                    names = sorted(non_prec_vars)


Why was this change needed?

SymbolTable.precision_datasymbols collects the Symbols referred to by the precision property (if any) of all Symbols in the table. This problem must have arisen because we have added new symbols that refer to a precision symbol from another table. I will investigate as it would be better if those references were updated.

Probably we're missing a call to update_symbols_using or similar.

I have 'fixed' this within KernelModuleInlineTrans but I'm still not happy - I suspect the problem is originating earlier so I will do some further investigation.

sergisiso · 2026-02-23T14:50:34Z

src/psyclone/tests/domain/gocean/transformations/globalstoargs_test.py

-@pytest.mark.usefixtures("kernel_outputdir")
-def test_kernelimportstoargumentstrans(monkeypatch):
+def test_kernelimportstoargumentstrans(monkeypatch, fortran_writer):
    ''' Check the KernelImportsToArguments transformation with a single kernel


The monkeypatch argument is unused now

sergisiso · 2026-02-23T15:04:34Z

...s/domain/gocean/transformations/gocean_move_iteration_boundaries_inside_kernel_trans_test.py

    assert isinstance(kschedule.symbol_table.lookup("xstart").interface,
                      ArgumentInterface)
-    assert isinstance(kschedule.symbol_table.lookup("xstop").interface,
+    assert isinstance(kschedule.symbol_table.lookup("xstop_1").interface,


Why has this changed? I am a bit worried about this one because if the call had argument names, this would not match anymore.

sergisiso · 2026-02-23T15:05:46Z

src/psyclone/tests/domain/gocean/transformations/gocean_opencl_trans_test.py

+@pytest.fixture(name="mod_inline_trans")
+def make_mod_inline_trans() -> KernelModuleInlineTrans:
+    '''Creates and returns a KernelModuleInlineTrans transformation.'''
+    return KernelModuleInlineTrans()


This seems unnecessary

sergisiso · 2026-02-23T15:12:09Z

doc/user_guide/psyclone_command.rst

                    [-l {off,all,output}] [-p {invokes,routines,kernels}]
                    [-o OUTPUT_FILE] [-api DSL] [-oalg OUTPUT_ALGORITHM_FILE] [-opsy OUTPUT_PSY_FILE]
                    [-okern OUTPUT_KERNEL_PATH] [-dm] [-nodm]
-                    [--kernel-renaming {multiple,single}]


Also clean these please:

doc/user_guide/transformations.rst 582:two different kernel-renaming schemes: "multiple" and "single" 583:(specified via the ``--kernel-renaming`` command-line flag). In the doc/user_guide/getting_going.rst 179: [--kernel-renaming {multiple,single}] 223: --kernel-renaming {multiple,single}

sergisiso · 2026-02-23T15:17:50Z

src/psyclone/psyGen.py

-        '''
-        :returns: whether or not this kernel is being module-inlined.
-        :rtype: bool
+    def module_inline(self) -> bool:


Maybe another TODO for 2054 saying that it subclasses Call, this probably should live in the super class

sergisiso · 2026-02-23T15:23:35Z

src/psyclone/transformations.py

                    kernel.symbol_table.swap(imported_var, updated_sym)
-
-            if updated_sym in kernel.symbol_table.precision_datasymbols:
+            if updated_sym.name.lower() in precision_sym_names:


Why was this changed?

…ans instead

…mbols

arporter · 2026-02-26T09:28:32Z

Unfortunately, my change to ScopingNode.replace_symbols_using means that any local symbol that shadows an outer one gets replaced by the outer one. The problem I wanted to fix was where a local symbol depends on a symbol in an outer scope during a tree-copy operation. Child nodes are only attached into the new (copied) tree at the end of the copy operation - prior to that they can't see the parent scope. Therefore, only at this point can we update any references to symbols in the outer scope.

arporter added 2 commits January 21, 2026 13:21

For #1823. Add Kernel symbols when Kernel is created.

6cbe966

#1823 first sketch of mixin [skip ci]

ba8f599

arporter self-assigned this Jan 21, 2026

arporter marked this pull request as draft January 21, 2026 17:11

#1823 WIP fixing module-inlining for multiple kernel calls [skip ci]

5b4b68b

arporter added 2 commits January 22, 2026 14:00

#1823 WIP removing .module_inline setter and fixing tests [skip ci]

95b5710

#1823 fix module-inline tests

1bb59eb

arporter added 10 commits January 23, 2026 16:39

#1823 WIP fixing tests [skip ci]

3811893

#1823 WIP fixing more tests [skip ci]

3082aa8

Merge branch 'master' into 1823_transform_inlined_kerns_only

2ae13c8

#1823 more test fixes [skip ci]

564e46d

#1823 fix lint [skip ci]

134cd05

#1823 WIP fixing tests [skip ci]

06341f7

#1823 more test fixing

0230c2f

Merge branch 'master' into 1823_transform_inlined_kerns_only

8fec073

#1823 WIP fixing examples [skip ci]

240f86d

#1823 add docstrings to new mixin

b269f7a

arporter and others added 5 commits February 2, 2026 16:36

Merge branch 'master' into 1823_transform_inlined_kerns_only

5a88b29

#1823 begin adding new tests [skip ci]

9d8ac4a

Merge branch 'master' into 1823_transform_inlined_kerns_only

1c07cb1

#1823 get coverage of new mixin

2e0dbc1

#1823 remove unused/unreachable code and reinstate some tests

f73cd68

arporter temporarily deployed to integration February 4, 2026 13:12 — with GitHub Actions Inactive

#1823 allow for ContainerSymbol from which kernel is imported to be i…

2eaf62c

…n outer scope

arporter added reviewed with actions and removed in progress labels Feb 20, 2026

arporter added ready for review and removed reviewed with actions labels Feb 20, 2026

sergisiso added under review and removed ready for review labels Feb 23, 2026

sergisiso temporarily deployed to integration February 23, 2026 11:00 — with GitHub Actions Inactive

sergisiso requested changes Feb 23, 2026

View reviewed changes

sergisiso added reviewed with actions and removed under review labels Feb 23, 2026

arporter added 9 commits February 24, 2026 10:21

Merge branch 'master' into 1823_transform_inlined_kerns_only

914b67d

#1823 update lfric practical

20401f1

#1823 fix lfric/eg15

80a1b91

#1823 tighten-up checks and make general purpose

4970ecf

#1823 rename and move mixin

26c1a18

#1823 complete renaming of class and method

6bec686

#1823 fix all tests after generalisation

a50cd28

#2381 revert change to GOceanOpenCLTrans and fix KernelModuleInlineTr…

93413b1

…ans instead

#2381 linting

0949296

arporter added in progress and removed reviewed with actions labels Feb 25, 2026

#1823 add test and fix issue with symbols referred to in copies of sy…

eba9f80

…mbols

arporter mentioned this pull request Feb 26, 2026

Copying a tree containing shadowed symbols may change semantics #2666

Open

arporter added 5 commits February 26, 2026 15:28

#1823 WIP fixing imports from symbols not in table [skip ci]

6f9b1e0

#1823 more work on respecting symbol object identity [skip ci]

42e80bb

#1823 WIP fixing tests [skip ci]

2dc3d80

#1823 get all LFRic tests passing [skip ci]

3742d08

#1823 WIP fixing non-lfric tests [skip ci]

e866f1d

		# Flag that the kernel has been modified
		node.modified = True

Conversation

arporter commented Jan 21, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

arporter commented Jan 21, 2026

Uh oh!

arporter commented Jan 23, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

codecov bot commented Jan 27, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

arporter commented Feb 4, 2026

Uh oh!

arporter commented Feb 4, 2026

Uh oh!

arporter commented Feb 4, 2026

Uh oh!

arporter commented Feb 20, 2026

Uh oh!

sergisiso left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

arporter commented Feb 26, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

arporter commented Jan 21, 2026 •

edited

Loading

arporter commented Jan 23, 2026 •

edited

Loading

codecov bot commented Jan 27, 2026 •

edited

Loading