Merge pull request #81 from frasercrmck/fix-sampler-attrs

[compiler] Fix bugs in passing samplers as i32s
uxlfoundation · Aug 7, 2023 · 4a85d73 · 4a85d73
2 parents 6c7d580 + 1b2971d
commit 4a85d73
Show file tree

Hide file tree

Showing 3 changed files with 20 additions and 1 deletion.
diff --git a/modules/compiler/source/base/source/image_argument_substitution_pass.cpp b/modules/compiler/source/base/source/image_argument_substitution_pass.cpp
@@ -130,21 +130,38 @@ PreservedAnalyses compiler::ImageArgumentSubstitutionPass::run(
       IRBuilder<> B(BasicBlock::Create(Ctx, "entry", WrapperKernel));
 
       SmallVector<Value *, 8> Args;
+      // Our wrapper hasn't been created with any parameter attributes, as the
+      // parameter types have changed. We must copy across all attributes from
+      // the non-sampler arguments to maintain program semantics.
+      AttributeList KernelAttrs = KernelF->getAttributes();
+      SmallVector<AttributeSet, 4> WrapperParamAttrs(KernelF->arg_size());
+
       for (auto [OldArg, NewArg] :
            zip(KernelF->args(), WrapperKernel->args())) {
         // Copy parameter names across
+        unsigned ArgIdx = Args.size();
         NewArg.setName(OldArg.getName());
         if (OldArg.getType() == NewArg.getType()) {
           Args.push_back(&NewArg);
+          WrapperParamAttrs[ArgIdx] = KernelAttrs.getParamAttrs(ArgIdx);
         } else {
           // Must be a sampler: simply cast the i32 to a pointer. This mirrors
           // what we'll do down the line when changing the pointer back to an
           // i32 via the opposite operation.
           Args.push_back(B.CreateIntToPtr(&NewArg, OldArg.getType(),
                                           NewArg.getName() + ".ptrcast"));
+          // Don't copy across any parameter attributes here, as this was a
+          // pointer and is now an i32.
+          WrapperParamAttrs[ArgIdx] = AttributeSet();
         }
       }
 
+      // Set our parameter attributes, but maintain the function/return
+      // attributes the wrapper has already received.
+      WrapperKernel->setAttributes(AttributeList::get(
+          KernelF->getContext(), WrapperKernel->getAttributes().getFnAttrs(),
+          WrapperKernel->getAttributes().getRetAttrs(), WrapperParamAttrs));
+
       auto *const CI = B.CreateCall(KernelF, Args);
 
       CI->setCallingConv(KernelF->getCallingConv());

diff --git a/modules/compiler/test/lit/passes/image-arg-subst.ll b/modules/compiler/test/lit/passes/image-arg-subst.ll
@@ -56,7 +56,7 @@ entry:
 }
 
 ; We've updated the old kernel to pass samplers as i32
-; CHECK: define spir_kernel void @image_sampler(ptr addrspace(1) %out, ptr addrspace(1) %img, i32 %sampler1, i32 %sampler2) [[NEW_ATTRS:#[0-9]+]] {
+; CHECK: define spir_kernel void @image_sampler(ptr addrspace(1) nocapture writeonly align 4 %out, ptr addrspace(1) %img, i32 %sampler1, i32 %sampler2) [[NEW_ATTRS:#[0-9]+]] {
 ; CHECK:   %sampler1.ptrcast = inttoptr i32 %sampler1 to ptr addrspace(2)
 ; CHECK:   %sampler2.ptrcast = inttoptr i32 %sampler2 to ptr addrspace(2)
 ; CHECK:   call spir_kernel void @image_sampler.old(

diff --git a/modules/mux/targets/host/source/command_buffer.cpp b/modules/mux/targets/host/source/command_buffer.cpp
@@ -41,6 +41,8 @@ size_t calcPackedArgsAllocSize(
     size_t size = 0;
     switch (descriptor.type) {
       case mux_descriptor_info_type_sampler:
+        size = sizeof(uint32_t);
+        break;
       case mux_descriptor_info_type_buffer:
       case mux_descriptor_info_type_null_buffer:
       case mux_descriptor_info_type_image: