clang-20.1 starts to zero padding in global initializers

[karolherbst]

I think this is something that needs to be worked around in the translator properly.

Starting with llvm/llvm-project@7a086e1 clang zero initializes in C dialects besides C++ all padding in static initializers, which do include Program scope variables in the global address space.

Assume code like this (OpenCL CTS test_basic progvar_prog_scope_misc):

input1.cl:

typedef struct { uchar c; uint i; } my_struct_t;

my_struct_t var = { 'a', 42 };

kernel void writer( uchar c, uint i ) {
  var.c = c;
  var.i = i;
}

and input2.cl:

typedef struct { uchar c; uint i; } my_struct_t;

extern my_struct_t var;

kernel void reader( global uchar* C, global uint* I ) {
  *C = var.c;
  *I = var.i;
}

The types of the global variable var ends up being different in the translation units:

@var = addrspace(1) global { i8, [3 x i8], i32 } { i8 97, [3 x i8] zeroinitializer, i32 42 }, align 4

vs

%struct.my_struct_t = type { i8, i32 }
@var = external addrspace(1) global %struct.my_struct_t, align 4

This also leads to different types for those variables on the SPIR-V level and therefore makes spirv-link fail with this: Type mismatch on symbol "var" between imported variable/function %47 and exported variable/function %13.

One could convince spirv-link to link variables together despite their different type, however this will cause access chains to point to the wrong members in some translation units.

I don't really know on which level this could be handled the best. I see a couple of options:

• make clang not do it for OpenCL C
• make LLVM add the padding fields also in kernels without initializers
• make the translator treat imported global variables as opaque (give it some random type, like being a single byte) and cast pointers to that variable on use
• add some lowering to spirv-link to pick one type and fix up all derefs (can be difficult, because how would the linker know what's padding and what's real).

[svenvh]

Thanks for raising this. I suppose it should be easy to exclude OpenCL in the clang patch to work around the issue for the time being, but we should think of a longer term solution too. My first thought is that we should try to stop clang from emitting inconsistent types.

Would this also be an issue for llvm's SPIR-V target? (@VyacheslavLevytskyy)

[MrSidims]

@svenvh do double check, does (OpenCL) clang still follow SPIR2.0 spec (and do we want to follow it)? It states:

2.1.5 Structs
The alignment of structures data members is the alignment of the SPIR data type. Extra padding
is disallowed. The alignment of the structure is the alignment of the member which requires the
largest alignment.

add some lowering to spirv-link to pick one type and fix up all derefs

@karolherbst my immediate question was: "what llvm-link does in this case?". And the answer is: it successfully links 2 modules resolving GV as
@var = dso_local local_unnamed_addr addrspace(1) global { i8, [3 x i8], i32 } { i8 97, [3 x i8] zeroinitializer, i32 42 }
buy llvm-link jobs is much simpler, it doesn't need to deal with AccessChain, I have hard times imagining how to implement this. The only idea I have is replacing AccessChains with PtrAccessChains as for OpenCL addressing model must be Physical.

(can be difficult, because how would the linker know what's padding and what's real).

Do we really need to know that if we decide to have strategy: "always pick the 'biggest' type"?

Overall it seems like the easiest way is to adjust clang for spir/spirv targets at this moment. Fixing it in spirv-link might be indeed quite challenging. I haven't though how difficult it would be to fix in SPIR-V producers yet.

[karolherbst]

@svenvh do double check, does (OpenCL) clang still follow SPIR2.0 spec (and do we want to follow it)? It states:

2.1.5 Structs
The alignment of structures data members is the alignment of the SPIR data type. Extra padding
is disallowed. The alignment of the structure is the alignment of the member which requires the
largest alignment.

Isn't this about runtimes adding extra padding when consuming the SPIR, which is disallowed?

add some lowering to spirv-link to pick one type and fix up all derefs

@karolherbst my immediate question was: "what llvm-link does in this case?". And the answer is: it successfully links 2 modules resolving GV as @var = dso_local local_unnamed_addr addrspace(1) global { i8, [3 x i8], i32 } { i8 97, [3 x i8] zeroinitializer, i32 42 } buy llvm-link jobs is much simpler, it doesn't need to deal with AccessChain, I have hard times imagining how to implement this. The only idea I have is replacing AccessChains with PtrAccessChains as for OpenCL addressing model must be Physical.

(can be difficult, because how would the linker know what's padding and what's real).

Do we really need to know that if we decide to have strategy: "always pick the 'biggest' type"?

Overall it seems like the easiest way is to adjust clang for spir/spirv targets at this moment. Fixing it in spirv-link might be indeed quite challenging. I haven't though how difficult it would be to fix in SPIR-V producers yet.

I meant any access chains. The issue with access chains is that they use logical addressing. So if they refer to member 2 of a struct, that's what they refer to.

So in order for spirv-link to deal with this it would have to figure out where to add the padding fields and which offset accesses to fix, by replacing the constants or adding add instructions for indirect accesses to fix it all up.

It's probably not impossible, but I can see this being a lot of work to get right.

[MrSidims]

I meant any access chains. The issue with access chains is that they use logical addressing. So if they refer to member 2 of a struct, that's what they refer to.

Clang deals quite elegantly with it, see https://godbolt.org/z/q8ExKzTWK and https://godbolt.org/z/jE8YcWKx3 - in both cases it get an address with 4-bytes offset. And it seems like it's not tight to uchar from your example - same for ushort.

In SPIR-V it already looks like this:
%29 = OpInBoundsPtrAccessChain %_ptr_CrossWorkgroup_uchar %28 %uint_4
and I don't see, how (InBounds)PreAccessChain can be broken, when we offset bytes and not elements. So unless I'm missing something, rewriting AccessChains into PtrAccessChain could help. And I agree - it's quite a lot of work to do, at least in SPIR-V linker and translator. In SPIR-V backend it should be easier to implement as global isel gives us enough flexibility to select the correct instruction with correct operands.

[karolherbst]

I meant any access chains. The issue with access chains is that they use logical addressing. So if they refer to member 2 of a struct, that's what they refer to.

Clang deals quite elegantly with it, see https://godbolt.org/z/q8ExKzTWK and https://godbolt.org/z/jE8YcWKx3 - in both cases it get an address with 4-bytes offset. And it seems like it's not tight to uchar from your example - same for ushort.

In SPIR-V it already looks like this: %29 = OpInBoundsPtrAccessChain %_ptr_CrossWorkgroup_uchar %28 %uint_4 and I don't see, how (InBounds)PreAccessChain can be broken, when we offset bytes and not elements. So unless I'm missing something, rewriting AccessChains into PtrAccessChain could help. And I agree - it's quite a lot of work to do, at least in SPIR-V linker and translator. In SPIR-V backend it should be easier to implement as global isel gives us enough flexibility to select the correct instruction with correct operands.

With the right compilation flags you can get logical addressing (as in.. not byte addressing by casting all the things to byte arrays):

• https://godbolt.org/z/szjx55zd4
• https://godbolt.org/z/MaoMYP4ae

[karolherbst]

I meant any access chains. The issue with access chains is that they use logical addressing. So if they refer to member 2 of a struct, that's what they refer to.

Clang deals quite elegantly with it, see https://godbolt.org/z/q8ExKzTWK and https://godbolt.org/z/jE8YcWKx3 - in both cases it get an address with 4-bytes offset. And it seems like it's not tight to uchar from your example - same for ushort.
In SPIR-V it already looks like this: %29 = OpInBoundsPtrAccessChain %_ptr_CrossWorkgroup_uchar %28 %uint_4 and I don't see, how (InBounds)PreAccessChain can be broken, when we offset bytes and not elements. So unless I'm missing something, rewriting AccessChains into PtrAccessChain could help. And I agree - it's quite a lot of work to do, at least in SPIR-V linker and translator. In SPIR-V backend it should be easier to implement as global isel gives us enough flexibility to select the correct instruction with correct operands.

With the right compilation flags you can get logical addressing (as in.. not byte addressing by casting all the things to byte arrays):

• https://godbolt.org/z/szjx55zd4
• https://godbolt.org/z/MaoMYP4ae

I just noticed, that even in the program with the initializer, the kernel actually using the global variable does cast it to a struct type without the explicit padding members. So maybe this can be hacked up in the spirv linker, just need to use the correct type.