Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Array.domain in an otherwise GPU eligible loop causes it to fail to be eligible #22433

Open
stonea opened this issue May 31, 2023 · 2 comments
Open

Comments

@stonea
Copy link
Contributor

stonea commented May 31, 2023

This issue is somewhat of a generalization of https://github.com/Cray/chapel-private/issues/4761, which uses c_ptrTo on array. Internally that calls out to Array.domain, which is what this issue is more directly about.

Anyway, if I have the following program that calls out to Array.domain from within a GPU eligible loop:

use GPU;

on here.gpus[0] {
  var A : [0..10] int;
  foreach i in 0..10 {
    assertOnGpu();
    A.domain;
  }
}

This errors out with:

foo.chpl:1: In module 'foo':
foo.chpl:5: error: Loop containing assertOnGpu() is not eligible for execution on a GPU
$CHPL_HOME/modules/internal/MemConsistency.chpl:112: note: function calls out to extern function (chpl_rmem_consist_release), which is not marked as GPU eligible

What appears to be happening (as also described in comments on #22151) is that _array._dom calls _getDomain which create a new instance of _domain (a record wrapping a pointer to the domain class) and when that gets destructed can (based off of a runtime condition) potentially call chpl_rmem_consist_release.

A couple quick things i tried that didn't work:

  • Having array._dom return by const ref (doesn't work because we create a new temporary record every time it's called)
  • Creating an empty GPU stub function (we'll get an error $CHPL_HOME/modules/internal/ChapelDomain.chpl:1068: note: call has outer var access)

Once we fix this issue we should also check to see that #22151 is fixed, as I imagine as fixing the Array.domain issue will fix that as well.

@mppf
Copy link
Member

mppf commented May 31, 2023

Having array._dom return by const ref (doesn't work because we create a new temporary record every time it's called)

I suppose this has to do with the current record-wrapping stuff. I also suppose we might be able to get some cases to work by telling the compiler it doesn't ever need to destroy a domain created by one of these functions. But, that's not likely to work in more general cases (with intermediate functions and calls etc).

Separately, I can think of a few more general approaches:

  • add runtime checks to the GPU-ization; so in this case we would GPU-ize but only run the GPU-ized versions if the _domain has _unowned=true.
  • have some kind of aggresive constant propagation to, in common cases, get the compiler to know that the _unowned=false will not be taken
  • use a different type for these "borrowed domains"

@e-kayrakli
Copy link
Contributor

add runtime checks to the GPU-ization; so in this case we would GPU-ize but only run the GPU-ized versions if the _domain has _unowned=true.

This could work today. But with Andy's AST specialization effort we'll move towards a world where we don't do a per-loop dynamic check. This would add a dynamic check back into the mix. I don't find this option particularly future-proof.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants