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

Segmentation fault with conditional pointer assignment from different address spaces #1288

Open
bmanga opened this issue Jan 19, 2024 · 4 comments
Assignees

Comments

@bmanga
Copy link
Contributor

bmanga commented Jan 19, 2024

kernel void exec(global float *in, global float *buf) {
    float tmp[32];
    float tmp2[32];

    float *in_ptr = in;
    float *out_ptr = tmp;
    for (int j = 0; j < 4; ++j) {
        for (int k = 0; k < 32; ++k) {
            out_ptr[k] = in_ptr[k] * buf[k];
        }
        in_ptr = out_ptr;
        out_ptr = out_ptr == tmp ? tmp2 : tmp;
    }
}

Results in:

  %6 = load float, ptr addrspace(4) %arrayidx, align 4
 : 
OpPtrAccessChain is not supported for this storage class
UNREACHABLE executed at ../clspv/lib/SPIRVProducerPass.cpp:4946!
Aborted (core dumped)

Godbolt link: https://godbolt.org/z/vx91zr9Po

@rjodinchr
Copy link
Collaborator

When I compile this example with c9d2022 I get:

error: initializing '__private float *__private' with an expression of type '__global float *__private' changes address space of pointer
   10 |     float *in_ptr = in;
      |            ^        ~~

which seems like a real issue in the test. Not sure why clspv is not returning that in godbolt.

@bmanga
Copy link
Contributor Author

bmanga commented Mar 13, 2024

When I compile this example with c9d2022 I get:

error: initializing '__private float *__private' with an expression of type '__global float *__private' changes address space of pointer
   10 |     float *in_ptr = in;
      |            ^        ~~

which seems like a real issue in the test.

Do you mean that the sample code that I provided is wrong or that it exposes an issue in clspv? AFAIK the code should be valid, and in_ptr should be in the generic address space.

@rjodinchr
Copy link
Collaborator

rjodinchr commented Mar 14, 2024

alright, I used the compilation options put in the godbolt link (-inline-entry-points -O3). With those, the generic addrspace does not exist.
Adding -cl-std=CLC++ allows to pass the frontend issue and now prints:

# | Instruction not handled:   %out_ptr.0 = phi ptr addrspace(4) [ %arraydecay.ascast, %entry ], [ %arraydecay8.ascast, %for.inc9 ]
# | Missing support for instruction
# | UNREACHABLE executed at clspv/lib/LowerAddrSpaceCastPass.cpp:346!

This is very similar to #1259, faaiz can you also take care of this one please?

@rjodinchr
Copy link
Collaborator

We have been working on that, and it is not simple. I'm not sure how to support this pattern.

At the moment with generic addrspace we try to figure out the real addrspace and get rid of all generic addrspace. Which means that for each variable we need to figure out one addrspace.

But in this pattern in_ptr starts with the addrspace of in which is global. And then it is updated with out_ptr wich is coming from tmp (private addrspace). So we end up with a PHI node that is not legal, as the type of its incoming values are different. I don't know how to support that pattern at the moment.

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

No branches or pull requests

3 participants