Skip to content

Commit

Permalink
Add unstable warning for serial statements (chapel-lang#24387)
Browse files Browse the repository at this point in the history
Resolves Cray/chapel-private#5810

This PR adds a `--warn-unstable` warning for the `serial` statement. We
are making this construct unstable due to the concerns brought up in
issue chapel-lang#16387 which I summarize in a section below.

It also updates the specto clarify that `cobegin` etc can create
non-serializable tasks & to note that `serial` is unstable and will be
deprecated.

Reviewed by @DanilaFe - thanks!

- [x] full comm=none testing

### The Concerns: Why make `serial` unstable?

First, tasks can be written in a way that assumes that they are not
serialized. For example:

``` chapel
proc f() {
  var s: sync int;
  cobegin {
    s.readFE();   // task 1: wait for task 2 -- will block forever if serialized
    s.writeEF(1); // task 2: write 1 to ‘s’
  }
}
```

As noted in the comment above, this function will block forever if the
implementation does not create separate tasks for the statements with
the `cobegin`. In particular, it will block forever if run in a `serial`
block.

This presents a programming problem, because in order to use the
`serial` statement, one has to be certain that any function called
within its dynamic scope only uses parallel tasks in a way that can be
serialized. That might imply that each function in a library needs to
document whether or not it can be used from a `serial` block. Or, it
might imply that programmers should defensively always use Chapel's task
parallel features in a way that is serializeable.

An additional concern with `serial` is that the use case for it is more
as a performance hint, but it can prevent programs from running
correctly in some cases.

### Next Steps

 * Develop a suitable replacement for the `serial` statement chapel-lang#24388
 * Consider ways to conditionally limit cobegin/begin chapel-lang#13518
  • Loading branch information
mppf authored Feb 12, 2024
2 parents 13e1488 + c184588 commit 0325125
Show file tree
Hide file tree
Showing 4 changed files with 237 additions and 1 deletion.
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,10 @@ Tasks are considered to be created when execution reaches the start of a
actually executed depends on the Chapel implementation and run-time
execution state.

A task is represented as a call to a *task function*, whose body
Tasks created by ``begin``, ``cobegin``, and ``coforall`` can depend upon
each other, even if that leads to the program not being serializable.

A task is implemented as a call to a *task function*, whose body
contains the Chapel code for the task. Variables defined in outer scopes
are considered to be passed into a task function by default intent,
unless a different *task intent* is specified explicitly by a
Expand Down Expand Up @@ -712,6 +715,10 @@ continue statements may not be used to exit a sync statement block.
The Serial Statement
--------------------

.. note::

The ``serial`` statement is unstable and likely to be deprecated.

The ``serial`` statement can be used to dynamically disable parallelism.
The syntax is:

Expand Down
13 changes: 13 additions & 0 deletions frontend/lib/uast/post-parse-checks.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -147,6 +147,7 @@ struct Visitor {
void checkCStringLiteral(const CStringLiteral* node);
void checkAllowedImplementsTypeIdent(const Implements* impl, const Identifier* node);
void checkOtherwiseAfterWhens(const Select* sel);
void checkUnstableSerial(const Serial* ser);
/*
TODO
void checkProcedureFormalsAgainstRetType(const Function* node);
Expand Down Expand Up @@ -191,6 +192,7 @@ struct Visitor {
void visit(const PrimCall* node);
void visit(const Return* node);
void visit(const Select* node);
void visit(const Serial* node);
void visit(const TypeQuery* node);
void visit(const Union* node);
void visit(const Use* node);
Expand Down Expand Up @@ -1556,6 +1558,17 @@ void Visitor::checkOtherwiseAfterWhens(const Select* sel) {
}
}

void Visitor::visit(const Serial* node) {
checkUnstableSerial(node);
}

void Visitor::checkUnstableSerial(const Serial* ser) {
if (shouldEmitUnstableWarning(ser)) {
warn(ser, "'serial' statements are unstable "
"and likely to be deprecated in a future release");
}
}

void Visitor::visit(const Yield* node) {
const AstNode* blockingNode;
const AstNode* allowingNode;
Expand Down
13 changes: 13 additions & 0 deletions test/unstable/serial.chpl
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
serial {
coforall i in 1..100 {
writeln("hello ", i);
}
}

proc main() {
serial {
forall i in 1..100 {
writeln("goodbye ", i);
}
}
}
203 changes: 203 additions & 0 deletions test/unstable/serial.good
Original file line number Diff line number Diff line change
@@ -0,0 +1,203 @@
serial.chpl:1: warning: 'serial' statements are unstable and likely to be deprecated in a future release
serial.chpl:7: In function 'main':
serial.chpl:8: warning: 'serial' statements are unstable and likely to be deprecated in a future release
hello 1
hello 2
hello 3
hello 4
hello 5
hello 6
hello 7
hello 8
hello 9
hello 10
hello 11
hello 12
hello 13
hello 14
hello 15
hello 16
hello 17
hello 18
hello 19
hello 20
hello 21
hello 22
hello 23
hello 24
hello 25
hello 26
hello 27
hello 28
hello 29
hello 30
hello 31
hello 32
hello 33
hello 34
hello 35
hello 36
hello 37
hello 38
hello 39
hello 40
hello 41
hello 42
hello 43
hello 44
hello 45
hello 46
hello 47
hello 48
hello 49
hello 50
hello 51
hello 52
hello 53
hello 54
hello 55
hello 56
hello 57
hello 58
hello 59
hello 60
hello 61
hello 62
hello 63
hello 64
hello 65
hello 66
hello 67
hello 68
hello 69
hello 70
hello 71
hello 72
hello 73
hello 74
hello 75
hello 76
hello 77
hello 78
hello 79
hello 80
hello 81
hello 82
hello 83
hello 84
hello 85
hello 86
hello 87
hello 88
hello 89
hello 90
hello 91
hello 92
hello 93
hello 94
hello 95
hello 96
hello 97
hello 98
hello 99
hello 100
goodbye 1
goodbye 2
goodbye 3
goodbye 4
goodbye 5
goodbye 6
goodbye 7
goodbye 8
goodbye 9
goodbye 10
goodbye 11
goodbye 12
goodbye 13
goodbye 14
goodbye 15
goodbye 16
goodbye 17
goodbye 18
goodbye 19
goodbye 20
goodbye 21
goodbye 22
goodbye 23
goodbye 24
goodbye 25
goodbye 26
goodbye 27
goodbye 28
goodbye 29
goodbye 30
goodbye 31
goodbye 32
goodbye 33
goodbye 34
goodbye 35
goodbye 36
goodbye 37
goodbye 38
goodbye 39
goodbye 40
goodbye 41
goodbye 42
goodbye 43
goodbye 44
goodbye 45
goodbye 46
goodbye 47
goodbye 48
goodbye 49
goodbye 50
goodbye 51
goodbye 52
goodbye 53
goodbye 54
goodbye 55
goodbye 56
goodbye 57
goodbye 58
goodbye 59
goodbye 60
goodbye 61
goodbye 62
goodbye 63
goodbye 64
goodbye 65
goodbye 66
goodbye 67
goodbye 68
goodbye 69
goodbye 70
goodbye 71
goodbye 72
goodbye 73
goodbye 74
goodbye 75
goodbye 76
goodbye 77
goodbye 78
goodbye 79
goodbye 80
goodbye 81
goodbye 82
goodbye 83
goodbye 84
goodbye 85
goodbye 86
goodbye 87
goodbye 88
goodbye 89
goodbye 90
goodbye 91
goodbye 92
goodbye 93
goodbye 94
goodbye 95
goodbye 96
goodbye 97
goodbye 98
goodbye 99
goodbye 100

0 comments on commit 0325125

Please sign in to comment.