Support `foreach` intents

[e-kayrakli]

This issue is to capture the general desire towards supporting intents on foreach loops. The foreach loop was added in https://github.com/chapel-lang/chapel/pull/18046, and it was added in a way that they are just a specially-marked for loops. This implies lack of intents similar to for. We want to support intents on foreach loops to enable vector-lane-private variables and reductions.

Note that in https://github.com/chapel-lang/chapel/issues/17857, it looks like we may end up adding for intents, as well. And because foreaches are actually fors in compiler, the implementation for both will have significant overlap.

[bradcray]

Highly related to this: I think that foreach loops probably also need to introduce shadow variables like forall loops to avoid inadvertent race conditions. While trying to apply a foreach loop this week, I realized that the compiler permitted me to write a loop like:

var sum: int;
foreach i in 1..n do
  sum += i;

which would not be permitted for a forall loop. In a discussion on slack, it seemed like there was a general consensus that this should not be permitted and that it would need to be written with a task intent. For example, I wrote:

But it makes me wonder whether I could write increasingly complex cases where suddenly they wouldn’t be able to, such that we shouldn’t rely on them to in this case either. For example, A[f(i)] += would have to be treated differently in a vectorizer for f(i) = i vs. f(i) = 0 vs. f(i) = something arbitrary and if the compiler can’t reason about f(), it seems it couldn’t distinguish between those cases.

[bradcray]

I put a 2.0 label on here for reasons explained on #19153—I think of these two issues as being strongly linked (though I suppose we could introduce the foreach shadow variables without supporting intents for a period of time which would likely avoid future breakages while just limiting what a user could write.

[lydia-duncan]

Are foreach loops considered stable otherwise? I get the sense that we don't have time to resolve this, but it seems like they're a new enough feature that that's okay?

[e-kayrakli]

When discussing foreach recently, I called them an unstable feature but surprised to see that they are not marked as such. I think we should make them unstable for 2.0 because you can write code today that you will not be able to once we have intents implemented (in the works, but I don't think we can have an implementation that we are confident enough to call things stable). @bradcray what do you think?

[bradcray]

I agree that they should be considered unstable in their current state. My strong preference would still be to have shadow variables and intents for them in the 2.0 release (which is arguably different from the fall release). Rationale: I think they're a pretty core/crucial feature, and even moreso in a vectorization + GPU world, so I think it'd be a shame for our core parallel features to have non-orthogonalities like this in them in a 2.0 release. (basically, if the 'Random' module is unstable, no big deal, that's not our reason for being; but parallelism is so if the first thing a person tries is some GPU code and finds out it's unstable... their first impression of the project will not be great).

[this is not the only thing in that category... some other non-orthogonalities in task intents are in the same bucket for me]

[e-kayrakli]

My strong preference would still be to have shadow variables and intents for them in the 2.0 release

I agree and feel optimistic about it.

[e-kayrakli]

I wanted to link this issue here more prominently as [default] intents, could help part of the issue: https://github.com/chapel-lang/chapel/issues/22343

[lydia-duncan]

I'm going to go ahead and remove the 2.0/Stabilization tag from this - the GPU team is making progress on it and I don't think there's much the stabilization subteam needs to be involved about?

[bradcray]

I'd be inclined to keep the label in the sense that it is still a stabilization issue even if the stabilization team doesn't own it (similar to how user issues may be owned by the GPU team rather than the user issue team but still deserve the "user issue" label).

[e-kayrakli]

I assigned @stonea who's been leading this push, in case it helps with Stabilization team's priorities/finding owners etc.

[lydia-duncan]

I'd be inclined to keep the label in the sense that it is still a stabilization issue even if the stabilization team doesn't own it (similar to how user issues may be owned by the GPU team rather than the user issue team but still deserve the "user issue" label).

I can see the comparison, but considering:

1. foreach is close to being finished
2. I wouldn't want to label every single new feature we make as a stabilization issue

I think I'd still prefer to not have it labeled as 2.0/Stabilization. My hope is that new features will get developed with stabilization in mind, while the stabilization tag would be most useful for existing features that are marked unstable. Put another way, I view "developing with stabilization in mind" as everyone's responsibility, rather than having to have a separate pass by the stabilization team for everything that gets developed after we've already done most of the feature development (and marking something with the tag indicates to me that a pass over it is necessary, since it'll be used to scoop issues into the Github Project).

But that's more philosophical and off-topic for this particular issue. I can move this to a process discussion if you'd like to talk about it further?

[e-kayrakli]

foreach is close to being finished

Just noting that "being finished" (what this issue asks) and "being stabilized" are different things in this context. reduce intents are a big part of "being finished", but somewhat orthogonal to stabilization as they don't even exist today and what we do there to support reduce intents should be strictly additive.

[stonea]

There's more work to be done with foreach but we've made significant progress towards having intents for foreach and the remaining work I think we could spin off into targetted subissues. Please reopen if you feel having this issue as an overarching "umbrella issue" would be useful.

As I see it the current status of foreach intents is that:

• We do not currently support reduce or var intents but produce a nice error message indicating such
• For CPU: support ref, in, const ref, and const in intents
• For GPU: support in, and const in intents
• For GPU: do not always do the right thing for ref intents but I have a followup issue I'm working on to address that: https://github.com/Cray/chapel-private/issues/6442

[bradcray]

Thanks for the summary here, Andy, which is great to have. If we don't have another issue that could serve as an umbrella issue for these cases, and haven't yet spawned off sub-issues, I'd be inclined to leave this one open.