chapel-lang / chapel

a Productive Parallel Programming Language
https://chapel-lang.org
Other
1.78k stars 418 forks source link

Passing ArrayViews to extern procs should protected #18135

Open ShreyasKhandekar opened 3 years ago

ShreyasKhandekar commented 3 years ago

This issue is prompted by #18118 and its subsequent fix as described in #18134.

Currently we are able to pass ArrayViews to extern C functions which causes unexpected behaviors as what we think is being passed actually differs from the Actual contents of the Array.

A potential resolution to this, as suggested by @e-kayrakli, is to not allow passing ArrayViews to any extern proc by raising a compilerError() from our C Interoperability suite. This would leave the onus of figuring out when to copy arrays to whichever function is trying to pass a Chapel array the extern proc.

bradcray commented 3 years ago

My stance on this is similar, but slightly different I think: I believe that we shouldn't support passing Chapel arrays whose memory is not C-like (non-contiguous, non-1D?) to a routine like extern proc foo(X: [] real); Unfortunately, this ends up being a dynamic check in most cases, though, so would result in a dynamic halt, rather than a static check. That said, it strikes me as the kind of error you'd get when you were writing and debugging your program for the first time rather than something that would only show up on the 100th run or 100th hour of a run (so reasonable to halt on).

What I imagine (and thought we had, at least for some cases) is that the routine that the compiler calls to convert a Chapel _array record into a C pointer for passing to the extern would check that the array's elements were contiguous in memory (and possibly other semantic checks like "logically 1D in Chapel" before passing the array off). Thus, for a default array, A[3, ..] could be passed since the elements are stored in RMO, but A[.., 3] could not be (but for a CMO array, the reverse would be true; and for a tiled array, probably neither would be legal). Similarly, most Block-distributed 1D arrays could not be passed to such a routine, though a localSlice() of such an array could be (or maybe a Block-distributed array—or slice of one?—that happened to be mapped completely to a single locale?).

I try to think about whether any cases could be a compile-time error. Cyclically distributed arrays come to mind: its logical elements are never contiguous in memory unless it's running on a single locale. But I'd probably choose the compile-time error in this case rather than an execution-time error that would apply in most practical contexts.

e-kayrakli commented 3 years ago

I guess you can statically error for rank change views where the picked rank is not the contiguous dimension. But then you might be over-limiting things. Like var A: [1..0, 3..3]; .... A[..., 3] ....; would be an error statically, although the array view's data is actually contiguous because of the weird nature of the parent array.