Constant Propagation Transform by Andrew-Beggs-ECMWF · Pull Request #515 · ecmwf-ifs/loki

Andrew-Beggs-ECMWF · 2025-03-18T16:55:03Z

Putting here for visibility and getting the CI to run. Still WIP (including this description).

Adds:

Constant Propagation Analysis

ConstantPropagationAnalysis

Constant Propagation Transformation

ConstantPropagationTransformer

Changes:

Analysers now inherit from the abstract AbstractDataflowAnalysis

This is instead of there only being analyse_dataflow, which is now under DataFlowAnalysis
Introduces a standardise format for analysers

Todo:

Write todos

This commit adds AbstractDFA and the concrete implementation of LiveVariableAnalysis. LiveVariableAnalysis replaces DataFlowAnalysis, and is simply a rename from the user's perspective. Internally, everything has been brought under one class (with the use of inner classes) and makes use of AbstractDFA. The goal is that other forms of DFA will follow and use this pattern as a common interface.

This is to better reflect existing patterns & functionality.

# Conflicts: # loki/transformations/extract/outline.py

ConstantPropagationAnalysis.py -> constant_propagation_analysis AbstractDFA.py -> abstract_dfa.py DataFlowAnalysis.py -> data_flow_analysis.py

The following loop may never be taken. However previously `a` would have been propagated, resulting in an erroneous print of 5 - instead of the correct 0. `a = 0 do i = 1, randomInteger a = 5 end do print *, a`

`_pop_array_accesses` will now be able to cope with arrays that have non-literal indices and shapes. This is done by partially matching the access and pop any candidates from the consts map. E.g. foo(1,bar,3) matches foo(1,2,3) and foo(1,1,3) in the map [foo(1,2,3), foo(1,1,3), foo(1,1,1)]

Loki can now better tell if a loop is taken at least once, might be taken, or is never taken and can update the const map accordingly.

This does two things: 1. Fix statically known never taken bodies not being propagated 2. Hoist loop independent variables

Sorry for the mega commit. I wanted to get the tests stable before clogging up my git log.

reuterbal

Thanks, at first glance this looks promising! I like the refactoring ideas around the dataflow analysis.

I've left a first few comments on stylistic choices, but I haven't looked in detail at the implementation, yet. Since this PR mixes refactoring the dataflow analysis with a new transformation, could we maybe split the contributions and have first a separate PR with the DFA refactoring that only applies the plumbing without any new features? That would help with the review because right now it's hard to see what's new and what's just moved around.

reuterbal · 2025-03-28T07:46:07Z

loki/analyse/abstract_dfa.py

+            self.get_detacher().visit(module_or_routine.body)
+
+    @contextmanager
+    def dataflow_analysis_attached(self, module_or_routine):


I understand the rationale for embedding this inside the class to directly relate to the implementation of the specific DFA class. However, I'd prefer to also keep the current API with a class-free contextmanager. This could instead receive an optional argument dfa=None or so, which by default will use the previous DFA implementation:

@contextmanager def dataflow_analysis_attached(self, module_or_routine, dfa=None): if dfa is None: from loki.analyse.dataflow_analysis import DataflowAnalysis # pylint: disable=no-toplevel-import dfa = DataFlowAnalysis() dfa.attach_dataflow_analysis(module_or_routine) yield dfa.detach_dataflow_analysis(module_or_routine)

reuterbal · 2025-03-28T07:50:21Z

loki/analyse/__init__.py

 """

-from loki.analyse.analyse_dataflow import *  # noqa
+from loki.analyse.data_flow_analysis import *  # noqa


Missing import:

Suggested change

from loki.analyse.data_flow_analysis import * # noqa

from loki.analyse.constant_propagation_analysis import * # noqa

from loki.analyse.data_flow_analysis import * # noqa

reuterbal · 2025-03-28T07:51:18Z

loki/analyse/abstract_dfa.py

+
+__all__ = ['AbstractDataflowAnalysis']
+
+class AbstractDataflowAnalysis(Transformer, ABC):


A few docstrings would be useful here.

reuterbal · 2025-03-28T07:57:48Z

loki/analyse/data_flow_analysis.py

Let's call this dataflow_analysis, to be consistent with the naming elsewhere.

reuterbal · 2025-03-28T07:58:18Z

loki/analyse/data_flow_analysis.py

+    'loop_carried_dependencies'
+]
+
+class DataFlowAnalysis(AbstractDataflowAnalysis):


Also, please use DataflowAnalysis to be consistent with baseclass and use elsewhere.

reuterbal · 2025-03-28T07:59:05Z

loki/analyse/abstract_dfa.py

+
+__all__ = ['AbstractDataflowAnalysis']
+
+class AbstractDataflowAnalysis(Transformer, ABC):


I'm not following why this needs to inherit from Transformer, given that the actual transformers are _Attacher and _Detacher and inheriting themselves.

Andrew-Beggs-ECMWF added 28 commits November 7, 2024 14:29

Add Constant Propagation transformation

b98b3c6

Add tests for Constant Propagation transformation

926dcfe

Fix imports

b12e2ce

Fix casting issues with Power expressions

7c710e9

Change NestedTransformer -> Transformer

9f92c85

Add optimised DFS path for loop unroller

14913ad

Change ConstantPropagator to use ConstPropMapper for expressions

fb0d395

Add no_unroll tests for Constant Propagation

586fbf9

Change namespacing of internal functions within ConstProp

42ba0ae

Change rebuilding of nodes to inplace update for ConstProp

01eb414

Refactor DataFlowAnalysis to LiveVariableAnalysis

30fd0a7

Rename LiveVariableAnalysis to DataFlowAnalysis

56acf41

This is to better reflect existing patterns & functionality.

Refactor Constant propagation into analysis and transformer

b8bb823

Merge branch 'main' into naab-const-prop-transform

44b0c7f

# Conflicts: # loki/transformations/extract/outline.py

Rename test_live_variable_analysis.py -> test_data_flow_analysis.py

04fac04

Fix uninitialised DataFlowAnalysis objects

65e6335

Rename files to match convention

9e8a243

ConstantPropagationAnalysis.py -> constant_propagation_analysis AbstractDFA.py -> abstract_dfa.py DataFlowAnalysis.py -> data_flow_analysis.py

Fix const prop'ing a loop possibly never taken

5be52e7

The following loop may never be taken. However previously `a` would have been propagated, resulting in an erroneous print of 5 - instead of the correct 0. `a = 0 do i = 1, randomInteger a = 5 end do print *, a`

Allow ConstantPropagationTransformer.visit() to take a constants_map

b91f6d9

Simplify logic in binary_num_op_helper()

2cfffb9

Add short-circuiting for logicals

eeb073d

Improve ability to const prop loops

2e2eb9a

Loki can now better tell if a loop is taken at least once, might be taken, or is never taken and can update the const map accordingly.

Remove redundant code

da86b4f

Refactor fallback Loop analysis

5b112f4

This does two things: 1. Fix statically known never taken bodies not being propagated 2. Hoist loop independent variables

Fix propagating consts out of a loop body that's statically never taken

c08310a

Add tests

53ce0a9

Sorry for the mega commit. I wanted to get the tests stable before clogging up my git log.

reuterbal reviewed Mar 28, 2025

View reviewed changes

Andrew-Beggs-ECMWF mentioned this pull request Apr 1, 2025

Dataflow Analysis Refactor #521

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Constant Propagation Transform#515

Constant Propagation Transform#515
Andrew-Beggs-ECMWF wants to merge 28 commits intoecmwf-ifs:mainfrom
Andrew-Beggs-ECMWF:naab-const-prop-transform

Andrew-Beggs-ECMWF commented Mar 18, 2025 •

edited

Loading

Uh oh!

reuterbal left a comment

Uh oh!

reuterbal Mar 28, 2025

Uh oh!

reuterbal Mar 28, 2025

Uh oh!

reuterbal Mar 28, 2025

Uh oh!

reuterbal Mar 28, 2025

Uh oh!

reuterbal Mar 28, 2025

Uh oh!

reuterbal Mar 28, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

	from loki.analyse.data_flow_analysis import * # noqa
	from loki.analyse.constant_propagation_analysis import * # noqa
	from loki.analyse.data_flow_analysis import * # noqa


		__all__ = ['AbstractDataflowAnalysis']

		class AbstractDataflowAnalysis(Transformer, ABC):

Conversation

Andrew-Beggs-ECMWF commented Mar 18, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

reuterbal left a comment

Choose a reason for hiding this comment

Uh oh!

reuterbal Mar 28, 2025

Choose a reason for hiding this comment

Uh oh!

reuterbal Mar 28, 2025

Choose a reason for hiding this comment

Uh oh!

reuterbal Mar 28, 2025

Choose a reason for hiding this comment

Uh oh!

reuterbal Mar 28, 2025

Choose a reason for hiding this comment

Uh oh!

reuterbal Mar 28, 2025

Choose a reason for hiding this comment

Uh oh!

reuterbal Mar 28, 2025

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Andrew-Beggs-ECMWF commented Mar 18, 2025 •

edited

Loading