Skip to content

Commit

Permalink
[compiler] Fix missing parameter attrs on sampler wrappers
Browse files Browse the repository at this point in the history
This fixes a bug introduced recently, where the wrappers we create
(before LLVM 17) were losing the parameter attributes from the original
kernel. This could lead to incorrect codegen in certain cases, most
notably when we lose `byval` attributes.
  • Loading branch information
frasercrmck committed Aug 7, 2023
1 parent 759235a commit 1b2971d
Show file tree
Hide file tree
Showing 2 changed files with 18 additions and 1 deletion.
Original file line number Diff line number Diff line change
Expand Up @@ -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());
Expand Down
2 changes: 1 addition & 1 deletion modules/compiler/test/lit/passes/image-arg-subst.ll
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down

0 comments on commit 1b2971d

Please sign in to comment.