Add specific handling for inline spirv pointer types

[s-perron]

Add specific handling for inline spirv pointer types

We currently blindly create a new type for all vk::SpirvType* types.
This can cause problems when the type is suppose to match another type.
In this case, we want a spirv pointer type to match the pointer type of the
variable. The OpTypePointer for the variable is implicitly created when
declaring the variable, but, if we want to explicitly declare the
pointer as a SpirvType, we get two different OpTypePointer instructions
in the module. So technically the types do not match.

To fix this, I add special handling in the SPIR-V backend to be able to
merge the implicit pointer type created and the SpirvType when they
match.

This implements the SPIR-V Pointers in HLSL spec proposal 0021

[Keenuts]

Are they places where we want to generate 2 distinct types for the same thing in DXC?
If not, why isn't this the default path for pointer type creation?

[devshgraphicsprogramming]

Does this fix any of the #6541 , #6578 or related issues with Access Chains crashing the compiler with a disallowed cast ?

[s-perron]

You should be able to do pointers for storage classes that do not require a layout. So function, private, and workgroup.

[s-perron]

Some other issues might be small bugs that still need to be fixed separately from this one. If you have problems let me know. Do we still have an issue opened for the access chain? I may have lost that issue.

[devshgraphicsprogramming]

Some other issues might be small bugs that still need to be fixed separately from this one. If you have problems let me know. Do we still have an issue opened for the access chain? I may have lost that issue.

AFAIK you just closed it, it was wrapped up with #6541

[devshgraphicsprogramming]

You should be able to do pointers for storage classes that do not require a layout. So function, private, and workgroup.

Later in #6578 I did some tests with attempting to use function and private pointers, they won't work.

Above optimization level O0 (so O1 and above) stuff gets aggressively inlined so the variable never exists in the first place to take a pointer of.

P.S. When one uses -fvk-use-scalar-layout DXC's SPIR-V codegen makes all SSBO and UBOs use a scalar layout, so making a pointer with a Storage Class of PhysicalStorageBuffer, StorageBuffer and Uniform shouldn't need any deduction.

[s-perron]

I have opened #6891 to follow up on the access chain issue. There was already an existing issues for the decoration attribute on fields.

[s-perron]

Are they places where we want to generate 2 distinct types for the same thing in DXC?

For pointers, I don't think we ever want to generate two distinct types. We might want to generate different types for struts that are the "same" if the user gave them different names:

struct A { int i; };
struct B { int i; };

If not, why isn't this the default path for pointer type creation?

I believe it is. The code that creates pointers calls SpirvContext::getPointerType. We cannot use the exact same function because how you extract the storage class and pointee type is different. That is different for every place that requires a pointer type. See below for one example:

DirectXShaderCompiler/tools/clang/lib/SPIRV/LowerTypeVisitor.cpp

Lines 225 to 231 in bb3a0c3

	case spv::Op::OpAccessChain: {
	const auto *pointerType = spvContext.getPointerType(
	resultType,
	cast<SpirvAccessChain>(instr)->getBase()->getStorageClass());
	instr->setResultType(pointerType);
	break;
	}

[Keenuts]

I need to check something.

[cassiebeckley]

LGTM

[devshgraphicsprogramming]

am I not understanding something or was this PR supposed to fix issues like needing the -Vd flag in https://godbolt.org/z/3vq3xWY7v ?

[s-perron]

am I not understanding something or was this PR supposed to fix issues like needing the -Vd flag in https://godbolt.org/z/3vq3xWY7v ?

It should fix the issue with using access chains.

It only for storage classes that do not require a layout (Function, Private, workgroup). I don't think we will be able to fix the layout problem in DXC.