> I do not claim to follow the "C vs. Chapel-level" paragraph above.
The point was important and is this: Just because a Chapel argument does
not have a 'ref' intent or type does not mean that an argument cannot/will
not be passed by ref/pointer in the generated C. In particular, all
records in the generated code are passed by pointer by default (that I'm
aware of). This is why I don't think it's important to make the type into
a ref type for a constant record argument in flattenFunctions.cpp. The
case to watch out for would be if I were blitheley changing record
arguments to 'in' or 'const in' intent since this is the one thing (that
I'm aware of) that would introduce an unnecessary record copy.
> In any case, the example is this:
>
> record R { var x: int; }
> outer();
> proc outer() {
> const r: R;
> inner();
> proc inner() {
> writeln(r);
> }
> }
>
> If R is large (which the compiler currently does not discern), inner() in the
> generated code should be getting a pointer argument, rather than a struct
> argument.
Here's the generated code for my patch:
/* testvass.chpl:3 */
static void outer(void) {
R this8;
R wrap_call_tmp;
(&this8)->x = INT64(0);
(&this8)->x = INT64(0);
wrap_call_tmp = _construct_R(INT64(0), &this8);
inner(&wrap_call_tmp);
return;
}
/* testvass.chpl:6 */
static void inner(R* const r) {
writeln3(r);
return;
}
Looks OK to me -- you?
For curiosity, here's the generated code before my patch:
/* testvass.chpl:3 */
static void outer(void) {
R this8;
R wrap_call_tmp;
R r;
_ref_R T = NULL;
(&this8)->x = INT64(0);
(&this8)->x = INT64(0);
wrap_call_tmp = _construct_R(INT64(0), &this8);
r = wrap_call_tmp;
T = &r;
inner(T);
return;
}
/* testvass.chpl:6 */
static void inner(_ref_R r) {
R T;
T = *(r);
writeln3(&T);
return;
}
(personally, I find the new code easier to read).
> BTW in the following example RVF on the trunk fails to forward 'r' even
> though it should. Maybe your patch fixes that.
>
> record R { var x: int; }
> proc test() {
> const r: R;
> var i$: sync int;
> on Locales[numLocales-1] {
> i$ = 1;
> writeln(r);
> }
> }
> test();
My patch doesn't help with this case because (a) the compiler (correctly)
won't pass the record by 'const in' intent to the outlined on-clause
function; and (b) I didn't add any callsite analysis to see whether
actuals are constant in cases where the formal isn't a local constnt
(i.e., 'const in'). Note that expanding my test to include 'const ref'
wouldn't help either because a 'var r: R;' could also be passed to a
'const ref' formal in the current language definition. This gets back to
the "intents describe callee formals, not caller actuals" point.
> (a) we should distinguish, in the intent bitmask, the bit that says "do not
> modify this formal" from the bit that says "this formal does not change", and
If by the second bit you mean "this formal's actual will not change",
there is no such bit today because the intent can't say this. Which is to
say, we've defined our intents to only describe callee semantics, not
what's happening simultaneously outside the callee. What you're really
looking for is callchain analysis with some sort of summary information,
and that's a reasonable thing to want, but IMO it doesn't belong in the
intents field (because I view that as representing the user notion of
intents, not information gleaned by compile-time analysis).
Moreover, note that a single function with multiple callsites may yield
different information along different paths. For example:
var r: R;
const r2: R;
begin foo(r);
begin foo(r2);
proc foo(const ref x: R) { ... }
The query "can the actual to foo() be simultaneously modified?" is
different for the different callsites -- i.e., it's not a function of the
procedure itself.
> (b) in addition to 'const ref', we should provide an intent that (1) has the
> run-time efficiency of 'ref' and (2) assures the callee that the formal does
> not change. The actuals for such intent must be 'const' variables or formals
> of guaranteed-unchanged ref intents.
Mmmaybe. Or we should just implement the callchain analysis described
above and use that summary information to make such decisions rather than
putting it on the user.
-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
