F2C: DeReferenceTrafo

[MichaelSt98]

DeReferenceTrafo for and within FortranCTransformation to insert apply/insert Dereference = * and Reference/address-of = & operators.

[github-actions]

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

[reuterbal]

Thanks, this looks good. I've made some suggestions how to improve the readability of the implementation, but fundamentally no objections here.

[reuterbal]

Tthe creation of the symbol map is then simplified to

symbol_map = {
    symbol: Dereference(symbol.clone()) for symbol in self.retriever.retrieve()
    if symbol.name.lower() in self.vars2dereference
}

[reuterbal]

The criterion for a symbol that needs (de)referencing is the same throughout, so maybe it would make sense to capture this as a static method in the transformer:

@staticmethod
def is_reference(symbol):
    return (
        isinstance(symbol, Array) 
        and not (symbol.dimensions is None or all(dim == ':' for dim in symbol.dimensions))
    )

[reuterbal]

With the above method, you could use this directly in the retriever:

self.retriever = ExpressionRetriever(self.is_reference)
self.vars2dereference = vars2dereference

[reuterbal]

This conditional would then also become more readable as

if self.is_reference(arg) and (isinstance(call_arg_map[arg], Array) or call_arg_map[arg].type.intent.lower() != 'in'):

[reuterbal]

Readability of the method might benefit from putting the reference/dereference handling into a utility method maybe, and then simply calling self.convert_args_to_references(kernel)

[reuterbal]

For my education: What is the intended outcome if we had (another) argument arr(:,:) here? Would this pre-empt the F2C transformation until we figure out the actual shape, or would it simply not be passed as a reference?

[MichaelSt98]

Since we need the actual shape information for flattening (see flatten_arrays() - TypeError: Resolve shapes being of type RangeIndex, e.g., ":" before flattening!, raised if isinstance(shape[-2], sym.RangeIndex))arrays having more than one dimension, the DeReferenceTrafo wouldn't be executed.

However,

subroutine transpile_call_driver(a)
  use transpile_call_kernel_mod, only: transpile_call_kernel
    integer, parameter :: len = 5
    integer, intent(inout) :: arr1(len)
    integer, intent(inout) :: arr2(len)
    call transpile_call_kernel(arr1, arr2, len)
end subroutine transpile_call_driver

  subroutine transpile_call_kernel(arr1, arr2, len)
    integer, intent(in) :: len
    integer, intent(inout) :: arr1(len)
    integer, intent(inout) :: arr2(:)

    arr1(1) = 1
    arr2(1) = 1
  end subroutine transpile_call_kernel

is transformed/transpiled to:

int transpile_call_driver_c() {
  int len = 5;
  transpile_call_kernel(arr1, arr2, len);
  return 0;
}

int transpile_call_kernel_c(int * restrict arr1, int * restrict arr2, int len) {

  int arr1[len];
  int arr2[len];
  arr1[1 - 1] = 1;
  arr2[1 - 1] = 1;
  return 0;
}

if that answers your question?!

[reuterbal]

It does, many thanks!

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 92.87%. Comparing base (beb848c) to head (92dcaf5).
Report is 3 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #273      +/-   ##
==========================================
+ Coverage   92.81%   92.87%   +0.05%     
==========================================
  Files         102      102              
  Lines       18216    18243      +27     
==========================================
+ Hits        16908    16943      +35     
+ Misses       1308     1300       -8     

Flag	Coverage Δ
lint_rules	96.39% <ø> (ø)
loki	92.84% <100.00%> (+0.06%)	⬆️
transformations	92.20% <ø> (ø)

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]

Many thanks, nothing further from me!