Skip to content

[AMDGPU][Verifier] Mark calls to entry functions as invalid in the IR verifier #134910

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

Merged
merged 1 commit into from
Apr 11, 2025
Merged

[AMDGPU][Verifier] Mark calls to entry functions as invalid in the IR verifier #134910

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
21 changes: 21 additions & 0 deletions llvm/include/llvm/IR/CallingConv.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -290,6 +290,27 @@ namespace CallingConv {

} // end namespace CallingConv

/// \return true if the calling convention allows the function to be called
/// directly or indirectly via a call-like instruction.
constexpr bool isCallableCC(CallingConv::ID CC) {
switch (CC) {
case CallingConv::AMDGPU_CS_Chain:
case CallingConv::AMDGPU_CS_ChainPreserve:
case CallingConv::AMDGPU_CS:
case CallingConv::AMDGPU_ES:
case CallingConv::AMDGPU_GS:
case CallingConv::AMDGPU_HS:
case CallingConv::AMDGPU_KERNEL:
case CallingConv::AMDGPU_LS:
case CallingConv::AMDGPU_PS:
case CallingConv::AMDGPU_VS:
case CallingConv::SPIR_KERNEL:
return false;
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we also add CallingConv::SPIR_KERNEL here since its not supported as well : https://github.com/llvm/llvm-project/blob/main/llvm/lib/Target/AMDGPU/AMDGPUISelLowering.cpp#L1137

Copy link
Contributor Author

Choose a reason for hiding this comment

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

There are "valid" test case calling to SPIR-V kernel directly. I thought that is allowed by the SPIR-V spec?

Copy link
Contributor

Choose a reason for hiding this comment

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

It should not be allowed. Also SPIR_KERNEL actually came from spir classic and SPIR-V just started picking it up.

Copy link
Contributor

Choose a reason for hiding this comment

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

"– No function can be targeted by both an OpEntryPoint instruction and an OpFunctionCall instruction."

Copy link
Contributor

@MrSidims MrSidims Apr 14, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

I lean to disagree. SPIR_KERNEL can be included conditionally. If triple is spirv, then sure, validation rule makes sense. But for spir triple it does look like a breaking change, as there is no such restriction in OpenCL or SPIR specification. Tagging @svenvh @michalpaszkowski @VyacheslavLevytskyy

Copy link
Contributor

Choose a reason for hiding this comment

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

But for spir triple it does look like a breaking change, as there is no such restriction in OpenCL or SPIR specification

+1, this is valid in these conditions

default:
return true;
}
}

} // end namespace llvm

#endif // LLVM_IR_CALLINGCONV_H
11 changes: 3 additions & 8 deletions llvm/lib/IR/Verifier.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3596,14 +3596,9 @@ void Verifier::visitCallBase(CallBase &Call) {
Check(Callee->getValueType() == FTy,
"Intrinsic called with incompatible signature", Call);

// Disallow calls to functions with the amdgpu_cs_chain[_preserve] calling
// convention.
auto CC = Call.getCallingConv();
Check(CC != CallingConv::AMDGPU_CS_Chain &&
CC != CallingConv::AMDGPU_CS_ChainPreserve,
"Direct calls to amdgpu_cs_chain/amdgpu_cs_chain_preserve functions "
"not allowed. Please use the @llvm.amdgpu.cs.chain intrinsic instead.",
Call);
// Verify if the calling convention of the callee is callable.
Check(isCallableCC(Call.getCallingConv()),
"calling convention does not permit calls", Call);

// Disallow passing/returning values with alignment higher than we can
// represent.
Expand Down
6 changes: 0 additions & 6 deletions llvm/test/Bitcode/calling-conventions.3.2.ll
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -80,12 +80,6 @@ define void @call_cc10 () {
ret void
}

define void @call_spir_kernel() {
; CHECK: call spir_kernel void @spir_kernel
call spir_kernel void @spir_kernel()
ret void
}

define void @call_spir_func() {
; CHECK: call spir_func void @spir_func
call spir_func void @spir_func()
Expand Down
Binary file modified llvm/test/Bitcode/calling-conventions.3.2.ll.bc
View file
Open in desktop
Binary file not shown.
15 changes: 0 additions & 15 deletions llvm/test/CodeGen/AMDGPU/attributor-flatscratchinit.ll
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -849,21 +849,6 @@ define amdgpu_kernel void @calls_intrin_ascast_cc_kernel(ptr addrspace(3) %ptr)
ret void
}

define amdgpu_kernel void @call_calls_intrin_ascast_cc_kernel(ptr addrspace(3) %ptr) {
; GFX9-LABEL: define amdgpu_kernel void @call_calls_intrin_ascast_cc_kernel(
; GFX9-SAME: ptr addrspace(3) [[PTR:%.*]]) #[[ATTR1]] {
; GFX9-NEXT: call void @calls_intrin_ascast_cc_kernel(ptr addrspace(3) [[PTR]])
; GFX9-NEXT: ret void
;
; GFX10-LABEL: define amdgpu_kernel void @call_calls_intrin_ascast_cc_kernel(
; GFX10-SAME: ptr addrspace(3) [[PTR:%.*]]) #[[ATTR1]] {
; GFX10-NEXT: call void @calls_intrin_ascast_cc_kernel(ptr addrspace(3) [[PTR]])
; GFX10-NEXT: ret void
;
call void @calls_intrin_ascast_cc_kernel(ptr addrspace(3) %ptr)
ret void
}

define amdgpu_kernel void @with_inline_asm() {
; GFX9-LABEL: define amdgpu_kernel void @with_inline_asm(
; GFX9-SAME: ) #[[ATTR3]] {
Expand Down
20 changes: 0 additions & 20 deletions llvm/test/CodeGen/AMDGPU/call-to-kernel-undefined.ll
View file
Open in desktop

This file was deleted.

18 changes: 0 additions & 18 deletions llvm/test/CodeGen/AMDGPU/call-to-kernel.ll
View file
Open in desktop

This file was deleted.

1 change: 0 additions & 1 deletion llvm/test/Other/spir_cc.ll
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,5 @@ define spir_func void @foo() {

define spir_kernel void @bar() {
call spir_func void @foo( )
call spir_kernel void @bar( )
ret void
}
23 changes: 0 additions & 23 deletions llvm/test/Verifier/amdgpu-cc.ll
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -217,26 +217,3 @@ define amdgpu_cs_chain_preserve void @preallocated_cc_amdgpu_cs_chain_preserve(p
define amdgpu_cs_chain_preserve void @inalloca_cc_amdgpu_cs_chain_preserve(ptr inalloca(i32) %ptr) {
ret void
}

declare amdgpu_cs_chain void @amdgpu_cs_chain_call_target()
declare amdgpu_cs_chain_preserve void @amdgpu_cs_chain_preserve_call_target()

define amdgpu_cs_chain void @cant_call_amdgpu_cs_chain_functions(ptr %f) {
; CHECK: Direct calls to amdgpu_cs_chain/amdgpu_cs_chain_preserve functions not allowed. Please use the @llvm.amdgpu.cs.chain intrinsic instead.
; CHECK-NEXT: call amdgpu_cs_chain
call amdgpu_cs_chain void @amdgpu_cs_chain_call_target()

; CHECK: Direct calls to amdgpu_cs_chain/amdgpu_cs_chain_preserve functions not allowed. Please use the @llvm.amdgpu.cs.chain intrinsic instead.
; CHECK-NEXT: call amdgpu_cs_chain_preserve
call amdgpu_cs_chain_preserve void @amdgpu_cs_chain_preserve_call_target()

; CHECK: Direct calls to amdgpu_cs_chain/amdgpu_cs_chain_preserve functions not allowed. Please use the @llvm.amdgpu.cs.chain intrinsic instead.
; CHECK-NEXT: call amdgpu_cs_chain
call amdgpu_cs_chain void %f()

; CHECK: Direct calls to amdgpu_cs_chain/amdgpu_cs_chain_preserve functions not allowed. Please use the @llvm.amdgpu.cs.chain intrinsic instead.
; CHECK-NEXT: call amdgpu_cs_chain
call amdgpu_cs_chain_preserve void %f()

ret void
}
Loading