Skip to content

[SYCL] Update aspect propagation logic for virtual functions #16288

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.

Sign up for GitHub

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

Closed
jzc wants to merge 1 commit into from
Closed

[SYCL] Update aspect propagation logic for virtual functions #16288

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 18 additions & 0 deletions llvm/lib/SYCLLowerIR/SYCLPropagateAspectsUsage.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -665,6 +665,24 @@ void processDeclaredVirtualFunctionSets(
StringMap<SmallVector<Function *, 4>> &VirtualFunctionSets) {
if (!F->hasFnAttribute("calls-indirectly"))
return;

// "Construction" kernels which reference vtables but do not actually
// perform any virtual calls have the calls-indirectly attribute
// attached by SYCLVirtualFunctionAnalysis pass. We do not want to
// attach sycl_used_aspects metadata to such kernels.
bool hasVirtualCall = false;
Copy link
Contributor

@AlexeySachkov AlexeySachkov Jan 10, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
bool hasVirtualCall = false;
bool HasVirtualCall = false;

LLVM Coding Standards

for (const Instruction &I : instructions(F)) {
Copy link
Contributor

@AlexeySachkov AlexeySachkov Jan 10, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is purely a FYI, LLVM has llvm::none_of which works with their ranges if you want to rewrite this into if (none_of(instructions(F), []() { multi-line lambda here }) return;. See STLExtras

const auto *CI = dyn_cast<CallInst>(&I);
if (!CI)
continue;
if (CI->isIndirectCall() && CI->hasFnAttr("virtual-call")) {
hasVirtualCall = true;
break;
}
}
if (!hasVirtualCall)
return;

Attribute CallsIndirectlyAttr = F->getFnAttribute("calls-indirectly");
SmallVector<StringRef, 4> DeclaredVirtualFunctionSetNames;
CallsIndirectlyAttr.getValueAsString().split(DeclaredVirtualFunctionSetNames,
Expand Down
6 changes: 4 additions & 2 deletions llvm/test/SYCLLowerIR/PropagateAspectsUsage/VirtualFunctions/virtual-functions-1.ll
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,15 +6,17 @@ define spir_func void @vfn() #0 {
ret void
}

; CHECK: @foo() #1 !sycl_used_aspects ![[#aspects]]
define spir_kernel void @foo() #1 {
; CHECK: @foo({{.*}}) #1 !sycl_used_aspects ![[#aspects]]
define spir_kernel void @foo(ptr %f) #1 {
call void %f() #2
ret void
}

; CHECK: ![[#aspects]] = !{i32 6}

attributes #0 = { "indirectly-callable"="_ZTSv" }
attributes #1 = { "calls-indirectly"="_ZTSv" }
attributes #2 = { "virtual-call" }

!sycl_aspects = !{!0}
!0 = !{!"fp64", i32 6}
6 changes: 4 additions & 2 deletions llvm/test/SYCLLowerIR/PropagateAspectsUsage/VirtualFunctions/virtual-functions-2.ll
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,9 @@ define spir_func void @vfnBar() #1 {
ret void
}

; CHECK: @kernel() #2 !sycl_used_aspects ![[#aspectsKernel:]]
define spir_kernel void @kernel() #2 {
; CHECK: @kernel({{.*}}) #2 !sycl_used_aspects ![[#aspectsKernel:]]
define spir_kernel void @kernel(ptr %f) #2 {
call void %f() #3
ret void
}

Expand All @@ -27,6 +28,7 @@ define spir_kernel void @kernel() #2 {
attributes #0 = { "indirectly-callable"="setFoo" }
attributes #1 = { "indirectly-callable"="setBar" }
attributes #2 = { "calls-indirectly"="setFoo,setBar" }
attributes #3 = { "virtual-call" }

!sycl_aspects = !{!0}
!0 = !{!"fp64", i32 6}
Expand Down
11 changes: 7 additions & 4 deletions llvm/test/SYCLLowerIR/PropagateAspectsUsage/VirtualFunctions/virtual-functions-3.ll
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,13 +25,15 @@ define spir_func void @subBar() {
ret void
}

; CHECK: @kernelA() #2 !sycl_used_aspects ![[#aspectsFoo]]
define spir_kernel void @kernelA() #2 {
; CHECK: @kernelA({{.*}}) #2 !sycl_used_aspects ![[#aspectsFoo]]
define spir_kernel void @kernelA(ptr %f) #2 {
call void %f() #4
ret void
}

; CHECK: @kernelB() #3 !sycl_used_aspects ![[#aspectsBar]]
define spir_kernel void @kernelB() #3 {
; CHECK: @kernelB({{.*}}) #3 !sycl_used_aspects ![[#aspectsBar]]
define spir_kernel void @kernelB(ptr %f) #3 {
call void %f() #4
ret void
}

Expand All @@ -42,6 +44,7 @@ attributes #0 = { "indirectly-callable"="setFoo" }
attributes #1 = { "indirectly-callable"="setBar" }
attributes #2 = { "calls-indirectly"="setFoo" }
attributes #3 = { "calls-indirectly"="setBar" }
attributes #4 = { "virtual-call" }

!sycl_aspects = !{!0}
!0 = !{!"fp64", i32 6}
Expand Down
13 changes: 13 additions & 0 deletions llvm/test/SYCLLowerIR/PropagateAspectsUsage/VirtualFunctions/virtual-functions-4.ll
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,16 +11,29 @@ entry:
ret void
}

; "Construction" kernels which reference vtables but do not actually
; perform any virtual calls should not have aspects propagated to them.
; CHECK-NOT: @construct({{.*}}){{.*}}!sycl_used_aspects
define weak_odr dso_local spir_kernel void @construct(ptr addrspace(1) noundef align 8 %_arg_StorageAcc) {
entry:
store ptr addrspace(1) getelementptr inbounds inrange(-16, 8) (i8, ptr addrspace(1) @vtable, i64 16), ptr addrspace(1) %_arg_StorageAcc, align 8
ret void
}

; Note: after SYCLVirtualFunctionAnalysis pass, the construction kernels will have
; "calls-indirectly" attribute, but even so they should not have aspects propagated
; to them (as the construction kernels have no virtual calls).
; CHECK-NOT: @construct2({{.*}}){{.*}}!sycl_used_aspects
define weak_odr dso_local spir_kernel void @construct2(ptr addrspace(1) noundef align 8 %_arg_StorageAcc) #1 {
entry:
store ptr addrspace(1) getelementptr inbounds inrange(-16, 8) (i8, ptr addrspace(1) @vtable, i64 16), ptr addrspace(1) %_arg_StorageAcc, align 8
ret void
}

; CHECK: ![[#aspects]] = !{i32 6}

attributes #0 = { "indirectly-callable"="set-foo" }
attributes #1 = { "calls-indirectly"="set-foo" }

!sycl_aspects = !{!0}
!0 = !{!"fp64", i32 6}
Loading