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

Constant initialized global variable rewrites produce invalid IR #1303

Open
alan-baker opened this issue Feb 7, 2024 · 0 comments
Open

Constant initialized global variable rewrites produce invalid IR #1303

alan-baker opened this issue Feb 7, 2024 · 0 comments

Comments

@alan-baker
Copy link
Collaborator

In the SPIRVProducer, constant-initialized global variables are rewritten to be private variables. This seems to work out in practice, but actually leads to invalid IR.

Run the following IR as test.ll:

target datalayout = "e-p:32:32-i64:64-v16:16-v24:32-v32:32-v48:64-v96:128-v192:256-v256:256-v512:512-v1024:1024"
target triple = "spir-unknown-unknown"

%struct.data_type = type { i32, [12 x i8] }

@c_var = local_unnamed_addr addrspace(2) constant [2 x %struct.data_type] [%struct.data_type { i32 0, [12 x i8] undef }, %struct.data_type { i32 1, [12 x i8] undef }], align 16
@__spirv_WorkgroupSize = local_unnamed_addr addrspace(8) global <3 x i32> zeroinitializer

define spir_kernel void @foo(ptr addrspace(1) nocapture writeonly align 16 %data, ptr addrspace(2) nocapture readonly align 16 %c_arg, { i32 } %podargs) !clspv.pod_args_impl !8 !kernel_arg_map !14 {
entry:
  %0 = call ptr addrspace(1) @_Z14clspv.resource.0(i32 0, i32 0, i32 0, i32 0, i32 0, i32 0, { [0 x %struct.data_type] } zeroinitializer)
  %1 = call ptr addrspace(2) @_Z14clspv.resource.1(i32 0, i32 1, i32 1, i32 1, i32 1, i32 0, { [4096 x %struct.data_type] } zeroinitializer)
  %2 = call ptr addrspace(6) @_Z14clspv.resource.2(i32 0, i32 2, i32 4, i32 2, i32 2, i32 0, { { i32 } } zeroinitializer)
  %3 = getelementptr { { i32 } }, ptr addrspace(6) %2, i32 0, i32 0
  %4 = load { i32 }, ptr addrspace(6) %3, align 4
  %n = extractvalue { i32 } %4, 0
  %5 = getelementptr { [4096 x %struct.data_type] }, ptr addrspace(2) %1, i32 0, i32 0, i32 %n, i32 0
  %6 = load i32, ptr addrspace(2) %5, align 16
  %7 = getelementptr [2 x %struct.data_type], ptr addrspace(2) @c_var, i32 0, i32 %n, i32 0
  %8 = load i32, ptr addrspace(2) %7, align 16
  %add.i = add nsw i32 %8, %6
  %9 = getelementptr { [0 x %struct.data_type] }, ptr addrspace(1) %0, i32 0, i32 0, i32 %n, i32 0
  store i32 %add.i, ptr addrspace(1) %9, align 16
  ret void
}

declare ptr addrspace(1) @_Z14clspv.resource.0(i32, i32, i32, i32, i32, i32, { [0 x %struct.data_type] })

declare ptr addrspace(2) @_Z14clspv.resource.1(i32, i32, i32, i32, i32, i32, { [4096 x %struct.data_type] })

declare ptr addrspace(6) @_Z14clspv.resource.2(i32, i32, i32, i32, i32, i32, { { i32 } })

!8 = !{i32 1}
!14 = !{!15, !16, !17}
!15 = !{!"data", i32 0, i32 0, i32 0, i32 0, !"buffer"}
!16 = !{!"c_arg", i32 1, i32 1, i32 0, i32 0, !"buffer_ubo"}
!17 = !{!"n", i32 2, i32 2, i32 0, i32 4, !"pod_ubo"}

With the command:

clspv-opt test.ll -o a.ll -producer-out-file test.spv -constant-args-ubo -pod-ubo --passes=ubo-type-transform,spirv-producer

LLVM will complain about a broken module:

# | GEP address space doesn't match type
# |   %7 = getelementptr [2 x %0], ptr addrspace(8) @c_var, i32 0, i32 %n, i32 0
# | LLVM ERROR: Broken module found, compilation aborted!
# | PLEASE submit a bug report to https://github.com/llvm/llvm-project/issues/ and include the crash backtrace.
# | Stack dump:
# | 0.	Program arguments: clspv-opt -constant-args-ubo -pod-ubo test.ll -o a.ll -producer-out-file test.spv --passes=ubo-type-transform,spirv-producer
# | Stack dump without symbol names (ensure you have llvm-symbolizer in your PATH or set the environment var `LLVM_SYMBOLIZER_PATH` to point to it):
# | 0  clspv-opt                0x0000000105c1193c llvm::sys::PrintStackTrace(llvm::raw_ostream&, int) + 56
# | 1  clspv-opt                0x0000000105c0fc74 llvm::sys::RunSignalHandlers() + 112
# | 2  clspv-opt                0x0000000105c12138 SignalHandler(int) + 344
# | 3  libsystem_platform.dylib 0x000000018c2c5a24 _sigtramp + 56
# | 4  libsystem_pthread.dylib  0x000000018c295cc0 pthread_kill + 288
# | 5  libsystem_c.dylib        0x000000018c1a1a40 abort + 180
# | 6  clspv-opt                0x0000000105b80794 llvm::report_fatal_error(llvm::Twine const&, bool) + 468
# | 7  clspv-opt                0x0000000105b805c0 llvm::report_fatal_error(llvm::Twine const&, bool) + 0
# | 8  clspv-opt                0x00000001056a78b0 llvm::VerifierPass::run(llvm::Function&, llvm::AnalysisManager<llvm::Function>&) + 0
# | 9  clspv-opt                0x000000010567c92c llvm::PassManager<llvm::Module, llvm::AnalysisManager<llvm::Module>>::run(llvm::Module&, llvm::AnalysisManager<llvm::Module>&) + 324
# | 10 clspv-opt                0x0000000104e9e6f4 main + 2504
# | 11 dyld                     0x000000018bf150e0 start + 2360

This seems to mostly work out in SPIR-V because it is unlikely we have enough instructions to evidence the mismatch in address spaces.

alan-baker added a commit to alan-baker/clspv that referenced this issue Feb 7, 2024
refs google#1292
refs google#1303

* Fixed a bug in remove unused arguments
  * attributes on parameters weren't handled parameters
* rewrite tests to be more targeted at the actual feature
rjodinchr pushed a commit that referenced this issue Feb 8, 2024
refs #1292
refs #1303

* Fixed a bug in remove unused arguments
  * attributes on parameters weren't handled parameters
* rewrite tests to be more targeted at the actual feature
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant