[Bug]: `&&=` and `reduce=` produce different results

[jabraham17]

When writing a forall with a reduce intent, it can be useful to use reduce= inside the loop to avoid having to specify the reduction operator in multiple places. However, I find that &&= and reduce= differ in behavior.

The following code behaves differently based on the value of useReduceEq

config const useReduceEq = false;
var numCalls: atomic int;
proc getBool(i) {
  numCalls.add(1);
  if i == 7 then return false; else return true;
}

var sum = true;
forall i in 1..1000 with (&& reduce sum) {
  if useReduceEq then
    sum reduce= getBool(i);
  else
    sum &&= getBool(i);
}
writeln(sum);
writeln("numCalls: ", numCalls.read());

> ./bug --useReduceEq=false
false
numCalls: 882
> ./a.out --useReduceEq=true
false
numCalls: 1000

[bradcray]

Is the implication that reduce= is smart about short-circuiting a task's local work when it is storing a false, but that &&= isn't (and perhaps can't be) because it doesn't know that it's part of a reduction computation—it's just a context-less &&= operator?

[jabraham17]

I am not sure if a && reduction is defined to be short circuiting or not. A quick grep of the docs does not say anything

But this program also prints 1000.

writeln(&& reduce [i in 1..1000] getBool(i));
writeln("numCalls: ", numCalls.read());

[bradcray]

I am not sure if a && reduction is defined to be short circuiting or not.

I think it'd be most natural if it were to be, but I'm surprised it's not.

I think short-circuiting is definitely the explanation for the behavior you're seeing in any case, as if I run it with --dataParTasksPerLocale=1 I get 7 out, and if I get use 2, I get 507 out.

Since we rewrite op reduce expressions into forall loops, I suspect that means we rewrite this case to use &&= rather than reduce= (?). Which seems surprising since someone seems to have put in some effort to make reduce= short-circuit, so why not map down to it?

This would be interesting to lock in as a test with a fixed value of dataParTasksPerLocale to track this going forward (e.g., maybe && reductions did originally lower to reduce= but then changed at some point without us catching this differences?)

[jabraham17]

I'm a bit confused what you mean here. reduce= is not doing any form of short-circuiting? &&= is.

I rewrote the code to be more clear

var numCalls: atomic int;
proc getBool(i) {
  numCalls.add(1);
  if i == 7 then return false; else return true;
}

var sum = true;
forall i in 1..1000 with (&& reduce sum) {
  sum reduce= getBool(i);
}
writeln(sum);
writeln("numCalls (reduce=): ", numCalls.read());

numCalls.write(0);
sum = true;
forall i in 1..1000 with (&& reduce sum) {
  sum &&= getBool(i);
}
writeln(sum);
writeln("numCalls (&&=): ", numCalls.read());

numCalls.write(0);
writeln(&& reduce [i in 1..1000] getBool(i));
writeln("numCalls (&& reduce ...): ", numCalls.read());

false
numCalls (reduce=): 1000
false
numCalls (&&=): 882
false
numCalls (&& reduce ...): 1000

[bradcray]

reduce= is not doing any form of short-circuiting? &&= is.

Oh, you're right, I was reading your conditional backwards, assuming that the default/false case was first since false < true (which, I realize, is ridiculous). My bad.

But at least that makes things make more sense:

• reductions are always rewritten in terms of reduce= (as imagined)
• nobody has put in effort to make reduce= short-circuit for && reductions (which is not surprising)
  • I think we should though… why waste that computation
  • As an optimization, once we hit the short-circuit, we could also break; out of the task's chunk of the forall loop, saving additional wasted time, in the event that it was the only thing in the loop / the rest of the loop was side-effect free.