Hi Brad (and Vass): > But your mention of 'const ref' and semantic grounds has got me > thinking... 'const' only means that the callee won't modify the argument, > not that the caller won't do so, so I'm not convinced that we can safely > r.v.f. 'const ref' arguments. In particular, the caller may still be > modifying its copy, and a sync var access may be used to force the callee > to see those updates.
Right. Sometimes I slip back into thinking serially. (Fortunately this was not the case when I was reimplementing copy propagation. There, I assumed that any argument passed in by ref can be changed by the caller at any time. SFSG.) That approach is pessimistic. We might be able to do a better job (of both copyPropagation and RVF) by performing static analysis to discover which variables are effectively constant at the call site. I think it is good to keep separate concepts separate, so in terms of the meaning of flags I like Brad's approach better than Vass's. 'ref' controls the mechanism used for argument passing, while 'const' controls how the callee will use it. THH ________________________________________ From: Brad Chamberlain [[email protected]] Sent: Friday, March 14, 2014 11:32 AM To: Tom Hildebrandt Cc: Chapel Sourceforge Developers List Subject: RE: [Chapel-developers] [patch] Improved const args and remote value forwarding > 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. Unfortunately, it is necessary for the reason that I tried to explain in the comments above the check. Because flattenFunctions.cpp currently inserts things that ought to be passed by ref using a "const in" argument with a ref type (incorrectly, IMO), I have to check the type of the argument to avoid incorrectly remote value forwarding such cases. I'll modify the comment to point to a test that fails without this additional check. In any case, the intention was not to filter against CONST_REF (though it probably does have that effect as well), but rather to guard against args that are created with CONST_IN intent and ref type. I'll be looking at cleaning that up in a separate commit since it's a pre-existing condition and distinct from this effort. But your mention of 'const ref' and semantic grounds has got me thinking... 'const' only means that the callee won't modify the argument, not that the caller won't do so, so I'm not convinced that we can safely r.v.f. 'const ref' arguments. In particular, the caller may still be modifying its copy, and a sync var access may be used to force the callee to see those updates. I'm going to check whether tightening this up to CONST_IN only (still with the ref type check) passes testing, update the comments to reflect the above, and commit it that way if there are no objections. -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
