Hi Brad:
[I sent this just to you, rather than copying to the whole external developers
list. You can publish it if you like. Probably you are already aware of these
issues, but I'm not sure the're written down anywhere....]
Regarding the removal of formal temps --
Formal temps are a necessary element of UMM, where objects of record type are
passed by value. Currently, I believe they are overused. But when we
undertake to remove the excess, this must be done bearing in mind that some
cases must still be preserved.
Somewhat counterintuitively, fundamental types (except for string) are special
cases, since their constructors are trivial. When a record is passed by value,
the implementation *must* provide a formal temp and call a copy-constructor to
initialize it.
The touchstone is to consider a record type R that is the handle for a
reference-counted data structure. Given
proc foo(r:R) { ... }
the implementation must at some point produce the rewriting
proc foo(ref r:R) { var _formal_tmp_r; move(_formal_tmp_r, autoCopy(r)); ...
; autoDestroy(_formal_tmp_r); }
Except for efficiency considerations, it makes no difference whether the
original r is passed in by reference or by value (in this rewriting). The
important point is that the copy-constructor (here called "autoCopy" for
historical reasons) and destructor (ditto) must be called. Otherwise, the
reference count is not right within the body of the function.
Of course, there are optimizations that can be applied to remove the
autoCopy/autoDestroy pair. If it can be proven that the value of r does not
"escape" the routine, and that the reference-counted data structure willl not
be reclaimed (on a parallel thread) while foo is executing, then the pair can
be removed (as long as the spec allows the side-effects associated with the
construction and destruction of the formal temp in that routine to be elided).
Regarding the representation of ref intent in the AST --
I agree with the comment, that the representation of arguments passed by ref
using reference types is inserted too early. But some caution is warranted
here as well. The refPropagation, copyPropagation and LICM passes currently
rely on checking whether the type of an operand is a reference type in at least
some of their predicates. Unless they are reworked to use only intents to
decide how an argument is being passed, the conversion of ref arguments to use
reference types must precede these passes. (Additional caution: inlineFunctions
calll copyPropagation directly)
Regarding the patch itself --
I'm not sure that the test (for ref type) on line 153 of
remoteValueForwarding.cpp is necessary for the sake of RVF itself. If it is
declared to be 'const' and we respect that, then it seems reasonable to be able
to forward the value. The machinery for implementing RVF may have to be more
sophisticated to handle this case (and that may very be why the test is there),
but I think that forwarding the value of an argument declared to be 'const ref'
should not be ruled out on semantic grounds.
All that said, the patch is fine to go in as-is. We can leave as a future work
item the idea of making RVF more effective by allowing it to forward 'const
ref' arguments as well.
THH
________________________________________
From: Brad Chamberlain [[email protected]]
Sent: Thursday, March 13, 2014 8:32 PM
To: Chapel Sourceforge Developers List
Subject: [Chapel-developers] [patch] Improved const args and remote value
forwarding
Hi Chapel Developers --
This is a request for review.
Size of patch = 2 out of 7.
Complexity = 4 out of 7.
The attached patch makes the insertion of formals to represent variables
when de-nesting functions a little less conservative by strengthening the
check for const-ness and not making the argument type a ref type if it is
const. This reduces the number of unnecessary references we use for
nested functions (user or compiler-introduced).
This in turn improves remote value forwarding because it permits us to
remote value forward constants more aggressively in the presence of sync
variable accesses. For the 'reduce*' tests in the distribution robustness
performance suite, it resulted in one fewer get per locale.
I added an editorial note in the comments pointing out that making a
formal have a 'ref' type but a 'blank' intent, as we currently do in
flattenFunctions.cpp, is arguably inconsistent but left that as future
work (perhaps near-future work, as we've talked about pushing the
insertion of 'ref' types very close to codegen in recent discussions). See
the comment itself for more detail.
I also added a test that presents a simplified version of the case that
caused me to note the lack of remote value forwarding (co-written by
Vass). This test at present works with or without r.v.f., but once formal
temps are removed (coming soon to a trunk near you), lack of r.v.f. breaks
this test (and SSCA2, from which it was derived).
-Brad
------------------------------------------------------------------------------
Learn Graph Databases - Download FREE O'Reilly Book
"Graph Databases" is the definitive new guide to graph databases and their
applications. Written by three acclaimed leaders in the field,
this first edition is now available. Download your free book today!
http://p.sf.net/sfu/13534_NeoTech
_______________________________________________
Chapel-developers mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/chapel-developers
