chapel-lang/chapel — pull request #420 by vasslitvinov (2014-09-14 21:56:02+00:00)
Title: Fix passing sync/single variables by ref/blank intent.

I fixed the bug where sync/single variables were not passed by reference
to generic blank/ref-intent formals. Instead, readFE/readFF were applied
so the formals were instantiated to those variables' base types.

The fix itself is trivial - see

compiler/resolution/generics.cpp

However, it exposed a couple of implementation and conceptual bugs.

* Putting a single variable in a tuple results in an extra deallocation
of that single variable, e.g. at close-brace here:

{
var x$: single int = 1;
writeln((x$, 1));
}

This is new because _build_tuple now takes that single variable by
reference. I wonder if it's easy to fix by throwing some flag
"do not deallocate" on the single var?

My workaround is to add readFF() in the above. See

spec/Task_Parallelism_and_Synchronization.tex

* To make write(x$) write the base variable, the most natural place
is to add

// modules/internal/ChapelSyncvar.chpl
proc _syncvar.writeThis(x: Writer) { x.write(this.readFE()); }
proc _singlevar.writeThis(x: Writer) { x.write(this.readFF()); }

However, the effect of this is that the sync var's lock is sought
*after* locking the channel that's being written to.

Indeed, the call write(x$) progresses as follows:

channel.write(args ...?k):bool
channel.write(args ...?k, out error:syserr):bool <--- IO.chpl:1991
_write_one_internal(_channel_internal:qio_channel_ptr_t, param kind:iokind, x\
:?t)
Writer.write(args ...?k)
Writer.writeIt(x:?t)
_syncvar.writeThis(x: Writer)

The channel gets locked in channel.write/IO.chpl:1991.
The sync var readFE() only happens in writeThis, when the channel
is already locked.

Therefore this code is a deadlock, which it wasn't before:

var x$: sync int;

cobegin {
{ writeln(x$); }
{ sleep(1);
writeln("about to assign x$");
x$ = 1; }
}

The first writeln() is stuck on x$; the second writeln() is stuck
on the channel lock that the first one is holding.

My workaround is, again, to add readFE().

test/multilocale/deitz/needMultiLocales/test_on_begin1.chpl
test/parallel/single/waynew/class0.chpl
test/parallel/single/waynew/class1.chpl

* min/max do not behave as expected when passed sync vars:

proc min(x, y) return if x < y then x else y;

If the 'then' branch is taken and x's actual is a sync variable,
it gets read twice, which is not intuitive.
I added an additional definition of min/max to address this.

modules/internal/ChapelBase.chpl

although there may be alternative solutions.