(Closes #3251) Implementation for the PSyDirective. #3255

LonelyCat124 · 2025-12-09T15:20:13Z

Example and doc TODO

Basic implementation of PSyDirective including reading and writing.

At the moment I implemented it as a Standalone Directive (as it doesn't make sense to me to be a RegionDirective) but that could be more difficult to use in some ways, but I'll put together an example and see.

codecov · 2025-12-09T15:32:44Z

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 99.91%. Comparing base (63d4c22) to head (6cb1b14).
⚠️ Report is 11 commits behind head on master.

Additional details and impacted files

@@           Coverage Diff           @@
##           master    #3255   +/-   ##
=======================================
  Coverage   99.91%   99.91%           
=======================================
  Files         376      377    +1     
  Lines       53512    53547   +35     
=======================================
+ Hits        53467    53502   +35     
  Misses         45       45

☔ 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.

sergisiso · 2025-12-09T15:48:26Z

I still don't think we need psyclone (!$psy) specific directives, I thought the plan was to add a generic Directive node, so that we could do psyir.walk(Directive) as an slight improvement to the current psyir.walk(Codeblock) (which tbh is almost the same as this PR implements, just with a slighly different _directive_handler)

LonelyCat124 · 2025-12-09T16:00:45Z

We could do that instead, but it would mean more false positives when people are looking for something specific to their script, e.g. after psyir.walk(???Directive) (name is not straightforward as we can't make it Directive since thats a necessary abstract class, along with StandaloneDirective and RegionDirective nodes. The other pain point is taking things out of CodeBlocks is it could be misinterpreted as support for them, and having non-RegionDirective nodes having to be StandaloneDirective subclasses.

I would maybe with ok with doing some GenericDirective and excluding all directives that start omp, ompx, omx, acc if you think that would be better?

sergisiso · 2025-12-09T16:19:18Z

name is not straightforward as we can't make it Directive since thats a necessary abstract class, ...
The other pain point is taking things out of CodeBlocks is it could be misinterpreted as support for them
I would maybe with ok with doing some GenericDirective

Good points, what about [Unrecognised/Unsupported/Unknown]Directive (being sibling of Standalone/RegionDirective)?

So it is clear that we don't support them and if we ever try to parse OpenMP, OpenACC into their own Directive the name will still make sense for those that we have not done yet. (my preferd is UnknownDirective, but I am unsure)

but it would mean more false positives when people are looking for something specific to their script

I don't understand this. It will be better than the current Codeblock, and it would allow searching for "ivdep" or other know compiler directive that may exist in the code.

excluding all directives that start omp, ompx, omx, acc

Why excluding those?

LonelyCat124 · 2025-12-10T10:22:58Z

Yeah I agree with most of that, so I'll modify this PR towards that.

My reasoning for ignoring omp/acc/etc. directives is party that keeping them as CodeBlocks means they'd keep any current properties that CodeBlocks have (i.e. disabling transformations etc.) and also make it clear that they're not meant to be used as part of any tree manipulation programming, but are just for keeping elements of the source code that PSyclone doesn't or can't support.

…tives supported

…yclone scripts

LonelyCat124 · 2025-12-15T10:33:46Z

@sergisiso this is ready for review, but no rush to do so.

sergisiso

@LonelyCat124 I like how this is implemented now, and especially having an example to showcase it. There are just minor comments now.

sergisiso · 2025-12-18T15:03:07Z

src/psyclone/tests/psyir/frontend/fparser2_directive_test.py

-    """Test that the FortranReader stored a directive before an allocate as a
-    CodeBlock."""
+    """Test that the FortranReader stored a directive before an allocate as an
+    UnknownDirective."""


Revert (this one is still a Codeblock)

sergisiso · 2025-12-18T15:05:31Z

src/psyclone/psyir/nodes/unknown_directive.py

+from psyclone.psyir.nodes.directive import StandaloneDirective
+
+
+class UnknownDirective(StandaloneDirective):


Maybe StandaloneDirective is the right choice, but why not just Directive?

sergisiso · 2025-12-18T15:06:29Z

src/psyclone/psyir/nodes/unknown_directive.py

+    Directive representing PSyclone-specific directives in the tree.
+
+    :param directive_string: The content after the !$psy part of this
+                             node in the tree.


param kwargs is missing

sergisiso · 2025-12-18T15:07:30Z

src/psyclone/psyir/nodes/unknown_directive.py

+    :param directive_string: The content after the !$psy part of this
+                             node in the tree.
+    '''
+    _children_valid_format = ""


we typically write "<LeafNode>" as format for leafs

sergisiso · 2025-12-18T15:13:46Z

examples/psyir/unknown_directives/README.md

+Authors: A. B. G. Chalk, STFC Daresbury Lab
+-->
+
+# PSyclone PSyIR directives.


Maybe:

# Use custom directives This directory contains an example of how write PSyclone scripts to transform code by taking advantage of information provided by the use of custom Fortran code directives.

I would also call the directory "custom_directives" (although the node name is Unknown, the use case is probably what people will look for in the examples)

sergisiso · 2025-12-18T15:13:56Z

examples/psyir/unknown_directives/README.md

+
+The example can be compiled using the Makefile, but this is a simple operation:
+``psyclone --keep-directive -s omp.py directive_filtering.F90``. The
+ ``-keep-directives`` option is essential to enabling the directive of interest


double dash?

sergisiso · 2025-12-18T15:16:36Z

examples/psyir/unknown_directives/omp.py

+from psyclone.psyir.nodes import Loop, Routine, UnknownDirective
+
+
+# Find the PSyDirectives, check what the directive is for and then find the


Maybe "Find the custom directive and add the associated loop to the list of loops to skip. Parallelise the rest."

And better as the method docstring instead of a comment, to make pylint happy.

sergisiso · 2025-12-18T15:16:53Z

examples/psyir/unknown_directives/Makefile

+# Author: M. Naylor, University of Cambridge, UK
+# Modified: A. B. G. Chalk, STFC Daresbury Lab


Only you as author

sergisiso · 2025-12-18T15:18:46Z

examples/psyir/unknown_directives/directive_filtering.F90

+    end do
+
+    ! Don't parallelise this next loop due to the string comparison
+    !$psy no_par


I would prefer a different string than "psy" for the example, maybe "mydirective" or "custom_directive" to not confuse anybody just looking at this file that this is a psyclone directive (change the readme with the same).

sergisiso · 2025-12-18T15:23:12Z

examples/psyir/unknown_directives/Makefile

+# -----------------------------------------------------------------------------
+# Author: M. Naylor, University of Cambridge, UK
+# Modified: A. B. G. Chalk, STFC Daresbury Lab
+


We typically add "transform, compile and run" standardised target names to all examples, and you will need to make ../Makefile recurse to it (at least for the transform target) so that this is tested in the CI.

Also, it will be good to mention the example in the parent folder README

Implementation for the PSyDirective. Example and doc TODO

91aef63

LonelyCat124 added the in progress label Dec 10, 2025

LonelyCat124 added 8 commits December 10, 2025 15:16

Chance PSyDirective to UnknownDirective and change the types of direc…

d2dbb24

…tives supported

Added a simple example for using directives to filter behaviour of PS…

9219cf3

…yclone scripts

Merge branch 'master' into 3251_psy_directive

b5322a0

Merge branch 'master' into 3251_psy_directive

60bc725

linting

e994474

Missed conflict

bde8e38

Fixed failing test for omp directives still being kept as clodeblocks

73a14ac

Linting updatE

6cb1b14

LonelyCat124 added ready for review and removed in progress labels Dec 15, 2025

LonelyCat124 requested a review from sergisiso December 15, 2025 10:33

LonelyCat124 marked this pull request as ready for review December 15, 2025 10:33

sergisiso added under review and removed ready for review labels Dec 18, 2025

sergisiso requested changes Dec 18, 2025

View reviewed changes

sergisiso added reviewed with actions and removed under review labels Dec 18, 2025

		from psyclone.psyir.nodes.directive import StandaloneDirective


		class UnknownDirective(StandaloneDirective):

		from psyclone.psyir.nodes import Loop, Routine, UnknownDirective


		# Find the PSyDirectives, check what the directive is for and then find the

		# Author: M. Naylor, University of Cambridge, UK
		# Modified: A. B. G. Chalk, STFC Daresbury Lab

(Closes #3251) Implementation for the PSyDirective. #3255

Are you sure you want to change the base?

(Closes #3251) Implementation for the PSyDirective. #3255

Uh oh!

Conversation

LonelyCat124 commented Dec 9, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

codecov bot commented Dec 9, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

sergisiso commented Dec 9, 2025

Uh oh!

LonelyCat124 commented Dec 9, 2025

Uh oh!

sergisiso commented Dec 9, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

LonelyCat124 commented Dec 10, 2025

Uh oh!

LonelyCat124 commented Dec 15, 2025

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!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

LonelyCat124 commented Dec 9, 2025 •

edited

Loading

codecov bot commented Dec 9, 2025 •

edited

Loading

sergisiso commented Dec 9, 2025 •

edited

Loading