Fix for issue #259

[michel2323]

I heavily used LLMs to fix this. I think I understand the code and it makes sense to me, but I want one of you @maleadt @vchuravy @giordano to take a look.

Issue:

insertvalue %agg, %val, 1, 0 in LLVM IR leads to OpCompositeInsert in SPIRV, which leads to this in the Intel stack (not supported).

Fix:

Flatten the nested insertvalue into read, modify, insert. The LLM agent was even able to add support for this into the Intel stack, but this is above my paygrade. I opened intel/intel-graphics-compiler#378.

[github-actions]

Your PR requires formatting changes to meet the project's style guidelines.
Please consider running Runic (git runic master) to apply these changes.

Click here to view the suggested changes.

diff --git a/src/compiler/compilation.jl b/src/compiler/compilation.jl
index 4ca04b0..0c57486 100644
--- a/src/compiler/compilation.jl
+++ b/src/compiler/compilation.jl
@@ -38,11 +38,15 @@ function GPUCompiler.finish_module!(job::oneAPICompilerJob, mod::LLVM.Module,
 end
 
 # finish_ir! runs later in the pipeline, after optimizations that create nested insertvalue
-function GPUCompiler.finish_ir!(job::oneAPICompilerJob, mod::LLVM.Module,
-                                entry::LLVM.Function)
-    entry = invoke(GPUCompiler.finish_ir!,
-                   Tuple{CompilerJob{SPIRVCompilerTarget}, typeof(mod), typeof(entry)},
-                   job, mod, entry)
+function GPUCompiler.finish_ir!(
+        job::oneAPICompilerJob, mod::LLVM.Module,
+        entry::LLVM.Function
+    )
+    entry = invoke(
+        GPUCompiler.finish_ir!,
+        Tuple{CompilerJob{SPIRVCompilerTarget}, typeof(mod), typeof(entry)},
+        job, mod, entry
+    )
 
     # FIX: Flatten nested insertvalue instructions to work around SPIR-V bug
     # See: https://github.com/JuliaGPU/oneAPI.jl/issues/259
@@ -84,7 +88,7 @@ function flatten_nested_insertvalue!(mod::LLVM.Module)
                     changed = true
                     count += 1
                 catch e
-                    @warn "Failed to flatten nested insertvalue" exception=(e, catch_backtrace())
+                    @warn "Failed to flatten nested insertvalue" exception = (e, catch_backtrace())
                 end
             end
         end
@@ -135,7 +139,7 @@ function flatten_insert!(inst::LLVM.Instruction)
         # Multiple levels: need to extract down, insert, then insert back up
         # For now, recursively extract to the deepest level
         temps = [extracted]
-        for i in 1:(length(rest_indices)-1)
+        for i in 1:(length(rest_indices) - 1)
             temp = LLVM.extract_value!(builder, temps[end], rest_indices[i])
             push!(temps, temp)
         end
@@ -144,7 +148,7 @@ function flatten_insert!(inst::LLVM.Instruction)
         inserted = LLVM.insert_value!(builder, temps[end], value, rest_indices[end])
 
         # Insert back up the chain
-        for i in (length(rest_indices)-1):-1:1
+        for i in (length(rest_indices) - 1):-1:1
             inserted = LLVM.insert_value!(builder, temps[i], inserted, rest_indices[i])
         end
     end
@@ -154,7 +158,7 @@ function flatten_insert!(inst::LLVM.Instruction)
 
     LLVM.replace_uses!(inst, result)
     LLVM.API.LLVMInstructionEraseFromParent(inst)
-    LLVM.dispose(builder)
+    return LLVM.dispose(builder)
 end
 
 
@@ -190,7 +194,7 @@ end
     # TODO: emit printf format strings in constant memory
     extensions = String[
         "SPV_EXT_relaxed_printf_string_address_space",
-        "SPV_EXT_shader_atomic_float_add"
+        "SPV_EXT_shader_atomic_float_add",
     ]
 
     # create GPUCompiler objects
diff --git a/test/indexing.jl b/test/indexing.jl
index db63254..7470fa6 100644
--- a/test/indexing.jl
+++ b/test/indexing.jl
@@ -33,33 +33,39 @@ end
     indices = CartesianIndices((2, 2))
 
     # Map to tuple of (value, index), then reduce by summing the values
-    result = mapreduce(tuple, (t1, t2) -> (t1[1] + t2[1], t1[2]), x, indices;
-                       init = (0, CartesianIndex(0, 0)))
+    result = mapreduce(
+        tuple, (t1, t2) -> (t1[1] + t2[1], t1[2]), x, indices;
+        init = (0, CartesianIndex(0, 0))
+    )
     @test result[1] == 4  # sum of four 1s
 
     # Test with 1D array
     y = oneArray(ones(Int, 4))
     indices_1d = CartesianIndices((4,))
-    result_1d = mapreduce(tuple, (t1, t2) -> (t1[1] + t2[1], t1[2]), y, indices_1d;
-                          init = (0, CartesianIndex(0,)))
+    result_1d = mapreduce(
+        tuple, (t1, t2) -> (t1[1] + t2[1], t1[2]), y, indices_1d;
+        init = (0, CartesianIndex(0))
+    )
     @test result_1d[1] == 4
 
     # Test with boolean array and index comparison (closer to original failure case)
     # This pattern is similar to what findfirst would use internally
     z = oneArray([false, true, false, true])
     indices_z = CartesianIndices((4,))
-    result_z = mapreduce(tuple,
-                         (t1, t2) -> begin
-                             (found1, idx1), (found2, idx2) = t1, t2
-                             # Return the first found index (smallest index if both found)
-                             if found1
-                                 return (found1, idx1)
-                             else
-                                 return (found2, idx2)
-                             end
-                         end,
-                         z, indices_z;
-                         init = (false, CartesianIndex(0,)))
+    result_z = mapreduce(
+        tuple,
+        (t1, t2) -> begin
+            (found1, idx1), (found2, idx2) = t1, t2
+            # Return the first found index (smallest index if both found)
+            if found1
+                return (found1, idx1)
+            else
+                return (found2, idx2)
+            end
+        end,
+        z, indices_z;
+        init = (false, CartesianIndex(0))
+    )
     @test result_z[1] == true  # Found a true value
-    @test result_z[2] == CartesianIndex(2,)  # First true is at index 2
+    @test result_z[2] == CartesianIndex(2)  # First true is at index 2
 end

[michel2323]

Needs a test based on the issue.

[codecov]

Codecov Report

❌ Patch coverage is 96.36364% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 79.41%. Comparing base (24110a8) to head (884f4e9).
⚠️ Report is 1 commits behind head on master.

Files with missing lines	Patch %	Lines
src/compiler/compilation.jl	96.36%	2 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master     #548      +/-   ##
==========================================
+ Coverage   79.10%   79.41%   +0.30%     
==========================================
  Files          47       47              
  Lines        3001     3055      +54     
==========================================
+ Hits         2374     2426      +52     
- Misses        627      629       +2     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.