google/clspv

Segmentation fault with conditional pointer assignment from different address spaces

bmanga opened this issue · 4 comments

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

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.

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.

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?

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.