Extract: Improved region-outlining for complex procedures

[mlange05]

This change brings various improvements to enable the use of outine_pragma_regions for more complex code regions. This is intended to eventually allow us to combine pragma-based extraction with auto-parallelisation to re-rewrite complex control-flow sections.

In particular, this PR refactors the utility structure to allow invocations per-region, deals with derived type symbols and arguments, and imposes an ordering on extracted argument signatures. There are also other small improvements added, as well as a utility to remove unused imports and import symbols.

In more detail:

• Use of Transformer, instead of the more complex NestedMaskedTransformer to replace PragmaRegion nodes
• Separation of the outer loop over subroutines from the specific single call-subroutine extraction for a single region
• A new utility to remove unused imports and trim unused symbols from import statements within a routine
• Ability to handle C-style imports when pragma-outlining sburoutines
• Resolving explicitly requested argument symbols (via the "intent" notation) from the parent routine. This allows explicitly requesting arguments that might not be used in the body, but are necessary for shape info in the spec of the extracted routine.
• Handling of derived-type symbols in extracted body/routine, in particular ensuring that only the parent is added to the extracted routine signature
• Additional tests for derived-type arguments and associated symbols in extracted bodies/routines. The latter is not strictly needed, but useful to have.
• Explicit ordering of arguments for reproducibility - this will use lexicographic base ordering, but put derived-type arguments, before array arguments, before scalar arguments.

[github-actions]

Documentation for this branch can be viewed at https://sites.ecmwf.int/docs/loki/412/index.html

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 93.09%. Comparing base (13aba08) to head (66995cf).
Report is 25 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #412      +/-   ##
==========================================
+ Coverage   93.07%   93.09%   +0.01%     
==========================================
  Files         198      198              
  Lines       39449    39547      +98     
==========================================
+ Hits        36719    36817      +98     
  Misses       2730     2730              

Flag	Coverage Δ
lint_rules	96.39% <ø> (ø)
loki	93.04% <100.00%> (+0.01%)	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

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

[reuterbal]

This looks great and is a valuable expansion of functionality over the original stubs!

I've left a few minor comments that would be good to address but no major showstoppers. However, I think we have a duplication between sanitise_imports and the new utility to remove unused imports.

[reuterbal]

Is this not redundant to or at least overlapping in functionality with the sanitise_imports utility? I would prefer if we combined the two somehow.

loki/loki/transformations/utilities.py

Lines 281 to 295 in 83c9ebe

	def sanitise_imports(module_or_routine):
	"""
	Sanitise imports by removing unused symbols and eliminating imports
	with empty symbol lists.
	Note that this is currently limited to imports that are identified to be :class:`Scalar`,
	:class:`Array`, or :class:`ProcedureSymbol`.
	"""
	if isinstance(module_or_routine, Subroutine):
	find_and_eliminate_unused_imports(module_or_routine)
	elif isinstance(module_or_routine, Module):
	used_symbols = set()
	for routine in module_or_routine.subroutines:
	used_symbols |= find_and_eliminate_unused_imports(routine)
	eliminate_unused_imports(module_or_routine, used_symbols)

[mlange05]

Good spot had forgotten about the overlap with this. However, the existing utility doesn't work for my use case (and I'd argue Transformer-based is better), but this certainly looks like something that deserves its own PR. If you don't mind, I'll drop this in a local rebase?

[mlange05]

@reuterbal Ok, I've rebased out the import sanitisation utility for now and addressed all other comments in the top two commits, as we as a fresh rebase over main. Please have another look.

[reuterbal]

Excellent, looks good.

I agree on the better implementation of the import sanitisation and happy to sort out the overlap separately!