Surprising behavior with task intents and global variables

[benharsh]

Consider the following program:

module M {
  var start = 0;

  proc setup() {
    start = 100;
  }

  proc helper() {
    writeln(start); // '100'
  }

  proc main() {
    //coforall loc in Locales with (ref start) {
    coforall loc in Locales {
      setup();
      writeln(start); // '0'
      helper();
    }
  }
}

As of 1.30, and running with -nl 1, this program prints:

0
100

By adding in the with-clause you can get this program to print 100 in both cases.

---

As far as I know this program's behavior is justified by the definition of task-intents in the spec, which I believe is to use const in for start. Even so, this is surprising behavior to me, and it could be difficult for users to recognize what was happening.

Do we want to change the behavior of task-intents specifically for global variables (e.g., use const ref in this case)?

Is there a way that we can detect this kind of problem and warn about it?

[bradcray]

I thought that, at one point at least, module-scope variables were not subject to task intents / shadowing behavior. Looking to @vasslitvinov who I believe wrestled with that question for reminders of where we ended up.

Do you get different behavior if start is declared within module Loop? (i.e., are "my" module scope variables treated differently than someone else's?) If so, this seems like a bug, or at least a strange inconsistency. [checking, it seems like "no". For that matter, it looks like putting all the code into one module is the same]

Could we at least issue a warning about using a global variable directly inside of a coforall's loop body?

It seems as though referring to module-scope variables within a parallel construct is a pretty common and reasonable thing to do in most cases, isn't it? (e.g., the config const alpha = ...; in Stream Triad comes quickly to mind).

To me, the gut-level red flag in this code feels more due to setup()'s non-functional modification of the module-scope variable / the fact that the module variable isn't set up in its initializer more than the coforall itself. I'm assuming there was a real-world example of this that caught you off-guard that might feel more compelling, though, maybe?

Also, note that this behavior isn't particularly specific to module-scope variables.

[benharsh]

@sdbachman encountered this problem in a real program earlier, which involved a config var and computing its value if a checkpointing file was available. The coforall came about from a straightforward translation from a single-node code to something more SPMD-like (per-locale rather than per-task). Some writelns had been inserted to see what the value of the global was at certain points, and the differing output was strange. At first I wondered about some possible RVF bug (as the original case was a coforall-on), but it turns out to be just the way that task-intents work in this case.

It seems as though referring to module-scope variables within a parallel construct is a pretty common and reasonable thing to do in most cases, isn't it? (e.g., the config const alpha = ...; in Stream Triad comes quickly to mind).

facepalm — yes, of course, not sure what I was thinking. It would still be nice if we had a way of helping users with this problem, somehow.

I've also updated the issue to use your improved single-module example.

[bradcray]

It would still be nice if we had a way of helping users with this problem, somehow.

It seems to me that the warning would need to be something like

• "[outer|module]-scope variables should not be assigned from within routines directly, but should be passed into the routine that's modifying it as an argument"

though that feels a bit stuffy to me. More palatable would be:

• "routines directly assigning [outer|module]-scope variables should generally only be called from the scope containing the declaration itself"
• or potentially "...from outside of any parallel sections"

Specifically, if the code had been written as:

proc setup(ref start) {
  start = 100;
}

... setup(start)...;

it would not have compiled (since a const would have been sent to a ref).

Or, if the call had been at the module scope or within main / outside of the coforall, it would've been fine.

Stylistically, if I were reviewing the code, I'd suggest:

• moving the setup() outside of the loop anyway, because having a bunch of parallel tasks simultaneously assigning a module-scope variable the same value is redundant and expensive, even if it did happen to give the desired result.
• trying to refactor the code to compute setup() as part of the module-scope variable's initializer so that it could be made into a const and be broadcast to all the locales at program startup.

[vasslitvinov]

I do not recall a lot of wrestling with the module-variable issue at the time. The spec makes it simple: "We decided to apply task intents to module level variables, in addition to local variables. Again, this is for consistency. One could view module level variables differently than local variables (e.g. a module level variable is “always available”), but we favored consistency over such an approach."

One case to see "consistency" is this:

var i: int;
coforall ... { ... do something about 'i' ... }

We may want the behavior to be the same whether this code is at the module scope vs. within a function.

[bradcray]

Thanks for confirming that the current behavior is as-intended, Vass. I'm glad we're treating module-scope variables consistently... I just had a memory of not doing so (or considering not doing so) at some point in the evolution of task-private vars.

[benharsh]

I've opened a PR here to add a test that exhibits this behavior explicitly, just to lock it in: https://github.com/chapel-lang/chapel/pull/22078

Is it worth keeping this issue open to track an interest in generating some kind of warning in this case?