chapel-lang / chapel

a Productive Parallel Programming Language
https://chapel-lang.org
Other
1.79k stars 420 forks source link

General warnings for race conditions #23976

Open jabraham17 opened 11 months ago

jabraham17 commented 11 months ago

In a recent discussion for default array intents (#23819), there was a desire expressed to push for more warnings for race conditions.

Two examples of cases that we would want to warn for

This is something that will be off by default and controlled by a flag (straw proposal of --warn-race-conditions). This would also be paired with a mechanism to selectively tell the compiler that what we are doing is ok.

For example:

A[B] = 1; // warn with --warn-race-conditions

@noRace // straw proposal, tells the compiler that the user accepts responsibility for a race condition
A[B] = 1;

forall i in A.domain do A[i] = A[i-1]; // warn with --warn-race-conditions

// the user has explicitly asked for `ref A`, the compiler won't warn
forall i in A.domain with (ref A) do A[i] = A[i-1];

@noRace  // the user accepts responsibility for a race condition
forall i in A.domain do A[i] = A[i-1];

Note that this is pretty strongly related to #23609 and I am envisioning they would fall under the same flag.

bradcray commented 11 months ago

Anyone picking up this issue may want to read my comments starting from https://github.com/chapel-lang/chapel/issues/23819#issuecomment-1828629394 which wrestled with trying to find the line between "obviously non-race-y" and "potentially race-y." I think this is a tricky topic because while:

forall i in A.domain do A[i] = A[i-1]; // warn with --warn-race-conditions

is race-y,

forall i in A.domain by 2 do A[i] = A[i-1]; // warn with --warn-race-conditions

is not. The compiler can also be made endlessly smarter and smarter about races (yet without ever being able to be perfect at distinguishing between the potential and the actual). So nobody should enter into this thinking that it's super-simple.

(I also don't consider it super-high priority... for me it's more like the reason that I'm comfortable with our conclusion to https://github.com/chapel-lang/chapel/issues/23819. See https://github.com/chapel-lang/chapel/issues/23819#issuecomment-1834438747 for more details.

jabraham17 commented 9 months ago

Note that #24240 implements some support for this under the flag name --warn-potential-races. The linked PR implements checking for modifying the result of a promoted indexing expression like A[B] += 1. This flag is off by default.

To make the warning go away, you have to rewrite the expression using an explicit loop like [b in B] A[b] += 1. This doesn't actually resolve the potential race condition (although the warning will go away), that requires using an explicit reduce intent like [b in B with (+ reduce A)] A[b] += 1