Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Should we support with intent clauses for on clauses? #26752

Open
bradcray opened this issue Feb 21, 2025 · 3 comments
Open

Should we support with intent clauses for on clauses? #26752

bradcray opened this issue Feb 21, 2025 · 3 comments

Comments

@bradcray
Copy link
Member

In recent years, particularly as GPU support has come on-line, we've increasingly taken it as a given that on clauses ought to support with clauses to specify the intents for how an outer-scope variable is made available on a (potentially) distinct locale. Today, the default is arguably [const] ref for everything (based on the outer variable's constness)—or potentially const in in cases where remote value forwarding (RVF) kicks in (?).

Here are some examples of how this might look/work:

var x = 42;

on Locales.last with (in x) do
  x += 1;  // modifies local copy of `x`, leaving original as-is

on Locales.last with (const in x) do
  computeWith(x);  // computes with a local copy of `x` rather than referring to the remote one all the time

on Locales.last with (const ref x) do
  computeWith(x);  // computes with the remote copy of `x`, but prevents local modifications to it

on Locales.last with (inout x) do
  x += 1;  // modifies local copy of `x`, but copies back to the original when done

on Locales.last with (out x) do
  x = 33;  // creates a local (uninitialized?) variable `x`, modifies it, and copies the result back to the original

on Locales.last with (const x) do
  computeWith(x);  // prevents modification of `x` but leaves `const in` vs. `const ref` unspecified, similar to other `const` intents

Mostly this has been discussed in live meetings, and showed up prominently in #18585, but I was surprised today to not be able to find an issue for it, so am opening this one.

@bradcray
Copy link
Member Author

Historically, I believe I've been the person most opposed to this, arguing that it isn't really necessary / doesn't add anything that you couldn't express in other ways; but I've come around over time, in part because I think the GPU cases are pretty compelling, partially from an orthogonality standpoint, and partially because so many Chapel programmers have seemed to expect it.

The only thing that gives me slight hesitation is that—without taking on breaking language changes—the default intents for on-clauses would be different from other contexts (argument passing, task intents), and I believe we've been criticized for having a too-complex default intent story before. I also think there's a productivity / least surprise argument that could be made for the status quo, but would not be surprised if the addition of with clauses led to concerns about intent inconsistency. I don't think this should hold up the idea, though.

@bradcray bradcray changed the title Should we add with intent clauses to on clauses? Should we support with intent clauses for on clauses? Feb 21, 2025
@mppf
Copy link
Member

mppf commented Feb 21, 2025

It's an interesting question how this proposal could interact with coforall.

It might be that, if you want to get a local copy of a variable on a per-locale basis, it would be this:

  coforall loc in Locales with (in x) {
    on loc with (in x) {
      ... do something with x ...
    }
  }

It's interesting to think about if copy elision has a role here. We could say, we don't have to copy the in x for the on statement, we can move it, because we already have a unique copy. For that to work, we'd either need to have a way to provide a cross-locale "move" to create a local copy of the fields (e.g. the array field in #26749) or we would need to provide some other function to localize a record once it is moved.

I suppose that on loc with (in x) is normally doing a cross-locale copy, so maybe we would want to think about it in terms of serialization / deserialization. See also #15675.

Which gets me wondering if this version would suffice:

  coforall loc in Locales {
    on loc with (in x) {
      ... do something with x ...
    }
  }

@e-kayrakli
Copy link
Contributor

I am a strong supporter of exploring this direction. Intent inconsistency worry that @bradcray brought up is an important concern, but I have no immediate reaction to it.

For @mppf's musings about coforall interactions, I believe that the second variant should be the best practice for most cases. As a user, I want a per on statement copy of x. How it gets wired through the task that fires up the on statement feels like an implementation detail to me. IOW, if x had a init= with a writeln in it, I would prefer the number of outputs from that writeln to be undefined in the second case.

A user could very well write the code as in the first variant. I see that as a more prescriptive version of doing the same thing, in which case, I'd expect to see two outputs from that writeln.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants