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

[SPIR-V] Creation of UniformConstant kind of variables emits invalid SPIR-V code #96513

Closed
VyacheslavLevytskyy opened this issue Jun 24, 2024 · 0 comments · Fixed by #96514
Closed
Assignees

Comments

@VyacheslavLevytskyy
Copy link
Contributor

Creation of UniformConstant kind of variables may emit invalid SPIR-V code by reusing composite constants of array type with a wrong number of components. The key to understanding the issue is the following minimal reproducer that basically contains the biggest part of addressing the issue:

%Vec3 = type { <3 x i8> }
%Vec16 = type { <16 x i8> }

define spir_kernel void @foo(ptr addrspace(1) noundef align 16 %arg) {
  %a1 = getelementptr inbounds %Vec3, ptr addrspace(1) %arg, i64 1
  call void @llvm.memset.p1.i64(ptr addrspace(1) align 4 %a1, i8 0, i64 3, i1 false)
  %a2 = getelementptr inbounds %Vec3, ptr addrspace(1) %arg, i64 1
  call void @llvm.memset.p1.i64(ptr addrspace(1) align 4 %a2, i8 1, i64 3, i1 false)
  ret void
}

define spir_kernel void @bar(ptr addrspace(1) noundef align 16 %arg) {
  %a1 = getelementptr inbounds %Vec16, ptr addrspace(1) %arg, i64 1
  call void @llvm.memset.p1.i64(ptr addrspace(1) align 4 %a1, i8 0, i64 16, i1 false)
  %a2 = getelementptr inbounds %Vec16, ptr addrspace(1) %arg, i64 1
  call void @llvm.memset.p1.i64(ptr addrspace(1) align 4 %a2, i8 1, i64 16, i1 false)
  ret void
}

declare void @llvm.memset.p1.i64(ptr addrspace(1) nocapture writeonly, i8, i64, i1 immarg)

Here vectors of different length are set with @llvm.memset that is implemented via variables of UniformConstant kinds initialized by an array of the corresponding size. Wrong way of creating constants of array type makes it impossible for SPIRV Backend to distinguish between [1, 1, 1] and [1, 1, 1, 1, 1, ...] (repeated 16 times).

spirv-val catches that as

error: line 14146: Initializer type must match the type pointed to by the Result Type
  %126 = OpVariable %_ptr_UniformConstant__arr_uchar_uint_16 UniformConstant %80
@VyacheslavLevytskyy VyacheslavLevytskyy self-assigned this Jun 24, 2024
VyacheslavLevytskyy added a commit that referenced this issue Jun 25, 2024
…96514)

This PR fixes #96513.

The way of creation of array type constant was incorrect: instead of
creating [1, 1, 1] or [1, 1, 1, 1, 1, ....] constants, the same [1]
constant was always created, substituting original composite constants.
This in its turn led to a situation when only one of constants might
exist in the code without emitting invalid code, the second constant
would be eventually rewritten to the first constant, because a key to
address both was an array of a single element (like [1]).

This PR fixes the issue and purges from the code unneeded copy/pasted
clone of the function that creates an array constant.
AlexisPerry pushed a commit to llvm-project-tlp/llvm-project that referenced this issue Jul 9, 2024
…lvm#96514)

This PR fixes llvm#96513.

The way of creation of array type constant was incorrect: instead of
creating [1, 1, 1] or [1, 1, 1, 1, 1, ....] constants, the same [1]
constant was always created, substituting original composite constants.
This in its turn led to a situation when only one of constants might
exist in the code without emitting invalid code, the second constant
would be eventually rewritten to the first constant, because a key to
address both was an array of a single element (like [1]).

This PR fixes the issue and purges from the code unneeded copy/pasted
clone of the function that creates an array constant.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants