The compiler produce invalid spirv with enabled PhysicalPointerArgsPass.

[AlexDemydenko]

The tool spirv-val return errors for some spirv binaries.

The test list with OpenCL-CTS:

vector_swizzle

Kernel example:
vector-swizzle.txt

[lpavank]

@alan-baker and @rjodinchr, do you any inputs for @AlexDemydenko on this issue/PR?

[alan-baker]

What kinds of errors? When I run:

clspv vector-swizzle.cl -o vector-swizzle.spv -int8=1 -physical-storage-buffers -arch=spir64

The resulting binary passes spirv-val. What command line are you using?

[rjodinchr]

This is at least missing a test I would say.

[AlexDemydenko]

Command line from the clvk driver:

clspv -cl-std=CL3.0 -no-8bit-storage=pushconstant -no-16bit-storage=pushconstant -spv-version=1.6 -arch=spir64 -physical-storage-buffers vector-swizzle.cl

Error from spirv-val:

error: line 99: OpPtrAccessChain result type (OpTypeInt) does not match the type that results from indexing into the base <id> (OpTypeVector).
  %56 = OpPtrAccessChain %_ptr_PhysicalStorageBuffer_uint %37 %uint_4

[AlexDemydenko]

ping

[rjodinchr]

Could you also add another test using clspv-opt targeting the replace-pointer-bitcast pass only.
Thank you

[lpavank]

@rjodinchr and @alan-baker any further inputs for @AlexDemydenko here?

[rjodinchr]

@rjodinchr and @alan-baker any further inputs for @AlexDemydenko here?

Maybe you have missed my comment from last month: #1418 (review)
"Could you also add another test using clspv-opt targeting the replace-pointer-bitcast pass only."

[lpavank]

@rjodinchr and @alan-baker any further inputs for @AlexDemydenko here?

Maybe you have missed my comment from last month: #1418 (review) "Could you also add another test using clspv-opt targeting the replace-pointer-bitcast pass only."

@rjodinchr believe @AlexDemydenko did answer (#1418 (comment)). Please confirm if thats what you wanted or no.
PS: Just trying to facilitate and/or get this change checked since its blocking one test on our end.

[rjodinchr]

@rjodinchr and @alan-baker any further inputs for @AlexDemydenko here?

Maybe you have missed my comment from last month: #1418 (review) "Could you also add another test using clspv-opt targeting the replace-pointer-bitcast pass only."

@rjodinchr believe @AlexDemydenko did answer (#1418 (comment)). Please confirm if thats what you wanted or no. PS: Just trying to facilitate and/or get this change checked since its blocking one test on our end.

At the moment, both added tests are not passing (on every bots). Please make sure they are passing using ToT.
Maybe you can also reduce the .ll test to only the necessary part so that it's easier to maintain (here is an example of a reduced test: https://github.com/google/clspv/blob/main/test/PointerCasts/opaque_implicit_gep.ll)

[rjodinchr]

That test is passing even without your fix, so it is not testing what you expect.

[AlexDemydenko]

A new version physical_pointers_vector-swizzle.ll has been uploaded, which only passes with a fix.

[rjodinchr]

That test is still not passing (with or without the fix)

[AlexDemydenko]

A new version physical_pointers_vector-swizzle.cl has been uploaded

[rjodinchr]

Please run the tests locally and make sure they fail without the fix and pass with it

[rjodinchr]

I had a closer look at the PR. I don't think this is the right way to deal with that issue.
Here is what I propose: #1434

[AlexDemydenko]

Yes, you propose works for me.