External Issue: Array type deduction results in a function being called twice

Notifications Issues
1

19674, "twesterhout", "Array type deduction results in a function being called twice ", "2022-04-20T19:23:45Z"

Summary of Problem

In the following example, the function enumerateStatesFixedHamming_v2 is called twice, but it should only be called once. Removing return type annotation in localEnumerateRepresentatives_v2 fixes the problem. Storing return value in a variable first looks like another workaround.

Steps to Reproduce

Source Code:

var global_count : atomic int = 0;

proc enumerateStatesFixedHamming_v2() {
  writeln("Calling enumerateStatesFixedHamming ...");
  global_count.add(1);
  var rs : [0 ..# 10] uint(64);
  return rs;
}

proc localEnumerateRepresentatives_v2() : [] uint(64) {
  writeln("inside localEnumerateRepresentatives ...");
  return enumerateStatesFixedHamming_v2();
}

proc main() {
  writeln(global_count);
  const r = localEnumerateRepresentatives_v2();
  writeln(global_count);
}

Compile command:

chpl -o bin/TestStatesEnumeration test/TestStatesEnumeration.chpl

Execution command:

./bin/TestStatesEnumeration

Additional info

The following comments from Gitter seem relevant:

Thomas Rolinger:

If you look at the compiler AST of this (at normalize), you see something like:

function localEnumerateRepresentatives_v2[204156]() : _unknown[54]
  {
    unknown ret[585178] "RVV" "temp"
    (204170 call writeln "inside localEnumerateRepresentatives ...")
    (536878 'end of statement')
    unknown call_tmp[787945] "expr temp" "maybe param" "maybe type" "temp"
    (787948 'move' call_tmp[787945](585183 call enumerateStatesFixedHamming_v2))
    unknown call_tmp[787950] "expr temp" "maybe param" "maybe type" "temp"
    (787953 'move' call_tmp[787950](585185 call uint(64)[118] 64))
    (585187 call chpl__checkRetEltTypeMatch call_tmp[787945] call_tmp[787950])
    (585189 'move' ret[585178](204175 call enumerateStatesFixedHamming_v2))
   (585180 return ret[585178])
  }

Maybe that'll provide some clues to others who know what is happening underneath the hood.

Thomas Rolinger:

From what I can tell, the compiler sees that you have a partially generic array return, so it inserts a call to chpl__checkRetEltTypeMatch, which will try to validate the return type by calling the function. Not sure if that is the intended behavior there

Elliot Ronaghan:

Yeah, there is some code to determine the shape of arrays, but I would hope we'd eliminate that code and not call a function twice at execution time.

Thomas Rolinger:

Digging a bit deeper since this is kinda of interesting: when it builds up the call to chpl__checkRetEltTypeMatch, it passes in the declared return type, which is the uint(64) stuff and it passes in the actual return type, which is (according to the compiler) the call to enumerateStatesFixedHamming_v2. So when chpl__checkRetEltTypeMatch is resolved/called, its first argument is a call itself, which ends up calling the function. I imagine this would go away if you stored the return to a variable and then returned that variable instead. EDIT: yes, that does seem to work and will only call the function once.

Also pinging @thomasrolinger and @ronawho who participated in the discussion on Gitter.