(Towards #3325) Improving .datatype to return size intrinsics instead of DEFERRED by LonelyCat124 · Pull Request #3336 · stfc/PSyclone

LonelyCat124 · 2026-02-17T15:22:03Z

Still in progress, this so far changes the arrayreference datatype to never give shapes with ArrayType.Extent in the result if we can resolve the shape.

I want to look at binaryoperation and how it chooses what result to use next, as I think we can do better than the current resul.

…taining status quo for more unknown types

LonelyCat124 · 2026-02-18T11:54:41Z

Waiting on coverage to come back, but if its ok this is ready for a first look.

I'm not totally convinced by the choice I make in BinaryOperation, so if the reviewer has a better strategy I'm happy to change it.

StructureReference already did output sizes better than ArrayReference, so from my experiment I didn't think that needed to be updated.

sergisiso · 2026-02-18T12:09:54Z

I am happy to review this, as I have been thinking what I need to redo WHEREs and it will be something similar. (I will wait for test/coverage to be reported before starting)

codecov · 2026-02-18T12:21:04Z

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 99.95%. Comparing base (b703387) to head (e4fb575).

❌ Your project check has failed because you have indirect coverage changes. Learn more about Unexpected Coverage Changes and reasons for indirect coverage changes.

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #3336      +/-   ##
==========================================
- Coverage   99.95%   99.95%   -0.01%     
==========================================
  Files         384      384              
  Lines       54225    54232       +7     
==========================================
+ Hits        54203    54208       +5     
- Misses         22       24       +2

☔ 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

Looks good @LonelyCat124 (although I am not convinced about the new special case - see inline)

I see this is a towards, is the goal of that issue to make "the datatype method should not propagate the DEFERRED attribute when constructing the datatype of an expression". Where are we on this?

src/psyclone/psyir/nodes/array_reference.py

src/psyclone/psyir/nodes/operation.py

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

src/psyclone/psyir/nodes/operation.py

sergisiso · 2026-02-19T11:07:38Z

Oh and I forgot about the Coverage indirect changes. It seems they are related to WHEREs, so I would address that comment first and see if they are still there.

LonelyCat124 · 2026-02-24T14:18:34Z

@sergisiso Ready for another look now I've updated the array_mixin stuff.

sergisiso

Thanks for improving the array_mixin._extent behaviour @LonelyCat124 . I still have some questions about the behaviour of the datatype method that probably means we are missing some tests.

sergisiso · 2026-02-25T14:36:58Z

src/psyclone/psyir/nodes/array_mixin.py

+        if (isinstance(start, Literal) and start.value_as_python == 1 and
+                isinstance(step, Literal) and step.value_as_python == 1):


Just for me to understand, what is the difference here? Is that now the precision doesn't matter in the comparison?

Also, I find the "value_as_python" slightly odd, for me it should be the other way around: "as_python_value" or even excluding the python and just being "as_value"

sergisiso · 2026-02-25T14:57:36Z

src/psyclone/psyir/nodes/operation.py

-        :param argtypes: the types of the two operands.
-        :type argtypes: list[:py:class:`psyclone.psyir.symbols.DataType`,
+        :param optypes: the types of the two operands.
+        :type optypes: list[:py:class:`psyclone.psyir.symbols.DataType`,


typehint, maybe Iterable[DataType]?

sergisiso · 2026-02-25T15:01:24Z

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

      b(widx1) = a(widx1)
    end if
  enddo'''
+    print(out)


sergisiso · 2026-02-25T15:02:41Z

src/psyclone/tests/psyir/nodes/operation_test.py

+            IntrinsicCall.Intrinsic.SIZE,
+            [Reference(sym2)])
+        )])
+    print("------------")


sergisiso · 2026-02-25T15:06:02Z

src/psyclone/tests/psyir/nodes/operation_test.py

+            [Reference(sym2)])
+        )])
+    print("------------")
+    binop7 = BinaryOperation.create(oper, ref7.copy(), ref6.copy())


Can you also do another BinOp with the operands switched? I think you are trying to check that the "datatype" prefers the type without the SIZE intrinsic (add a comment saying that this is the purpose), but in this case its the first operand which would also be chosen by default.

sergisiso · 2026-02-25T15:36:34Z

src/psyclone/psyir/nodes/array_reference.py

            # TODO #1857 - passing base_type as an instance of ArrayType
            # only works because the ArrayType constructor just pulls out
            # the intrinsic and precision properties of the type.


Is this the reason the method have the desirable property that "the datatype method should not propagate the DEFERRED attribute when constructing the datatype of an expression". In this case this should not be a TODO, just a comment. And it will be good to add tests with Extent.ATTRIBUTE and Extent.DEFERRED in src/psyclone/tests/psyir/nodes/array_reference_test.py which are the moment there is none.

Also if the above is true can you update the docstring to say:

def datatype(self) -> Datatype: ''' Note that if the resulting datatype is an ArrayType, this will convert Extent attributes to the appropriate inquiry intrinsic, which is convenient to directly use the resulting expression as executable code. Use `node.symbol.datatype` to obtain the type declaration of the array symbol (with its Extent attributes). :returns: the resulting datatype of the array access expression. '''

(We may want the first sentence to be in each node.datatype docstring, as it is true by extension. E.g. operation.datatype must also have this property)

By the way, if you extend the src/psyclone/tests/psyir/nodes/array_reference_test.py::test_array_datatype I would prefer to have it with a fortran_writer and a bunch of fortran declaration and array_references. I find it much easier to quickly verify it than reading the constructed PSyIR.

sergisiso · 2026-02-25T15:42:01Z

src/psyclone/psyir/nodes/array_reference.py

+                all(self.is_full_range(idx) for idx in range(len(shape)))
+                and all(isinstance(idx, ArrayType.ArrayBounds) for idx in
+                        orig_shape)):
                return self.symbol.datatype


I am still usure about this:

Are not all shapes ArrayType.ArrayBounds?, what is the new condition checking?

What happens if there is a DEFERRED array bounds e.g. real, dimension(3:) :: a. There is no tests for this.

Wouldn't we want the same behaviour if it is not the full range, for instance in . real, dimension(100,100) :: a and a(:,1), the resulting shape should preferable be [100] and not SIZE(a, 1).

sergisiso · 2026-02-25T15:47:21Z

src/psyclone/tests/psyir/nodes/operation_test.py

+    binop7 = BinaryOperation.create(oper, ref7.copy(), ref6.copy())
+    dtype6 = binop7.datatype
+    assert dtype6.shape[0].lower.value == "1"
+    assert dtype6.shape[0].upper.debug_string() == "5"


Another case we could implement/test is UnresolvedType/UnknwonType + ArrayType

In fortran the operand of an ArrayType can only be an array of the same rank or an scalar. But both will always results as an array of the exact same shape than the ArrayType of the known operand.

LonelyCat124 added 3 commits February 17, 2026 15:17

Update to arrayreference datatype to improve functionality while main…

75723e1

…taining status quo for more unknown types

Improvement(ish) to BinaryOperation's datatype

391a92f

Fix remaining test

bfa94d8

LonelyCat124 requested review from arporter and sergisiso February 18, 2026 11:49

LonelyCat124 added the ready for review label Feb 18, 2026

LonelyCat124 marked this pull request as ready for review February 18, 2026 11:49

Missing space from test output

87ab1dc

Merge branch 'master' into 3325_datatype_enhancement

255d8d7

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

sergisiso requested changes Feb 19, 2026

View reviewed changes

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

LonelyCat124 added 2 commits February 20, 2026 10:11

Changes for review

37e4a80

linting

75aaa78

LonelyCat124 mentioned this pull request Feb 20, 2026

(Closes #3339) Added a python representation to the Literal class #3342

Merged

LonelyCat124 added 2 commits February 24, 2026 13:47

Merge branch 'master' into 3325_datatype_enhancement

39d7c62

Updated array_mixin to be precision independent on extent sizes

e4fb575

LonelyCat124 added ready for review and removed reviewed with actions labels Feb 24, 2026

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

sergisiso requested changes Feb 25, 2026

View reviewed changes

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

		if (isinstance(start, Literal) and start.value_as_python == 1 and
		isinstance(step, Literal) and step.value_as_python == 1):

Conversation

LonelyCat124 commented Feb 17, 2026

Uh oh!

LonelyCat124 commented Feb 18, 2026

Uh oh!

sergisiso commented Feb 18, 2026

Uh oh!

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

Uh oh!

Codecov Report

Uh oh!

sergisiso left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

sergisiso commented Feb 19, 2026

Uh oh!

LonelyCat124 commented Feb 24, 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!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

codecov bot commented Feb 18, 2026 •

edited

Loading