Skip to content

Conversation

@wenju-he
Copy link
Contributor

Newly created non-kernel function and function call should have spirv_func calling convention.

…ir target

Newly created non-kernel function and function call should have
spirv_func calling convention.
@wenju-he wenju-he requested a review from a team as a code owner October 16, 2024 08:18
@wenju-he
Copy link
Contributor Author

one motivation is intel gpu backend requires calling conv to be consistent.

FunctionCallee FC =
M.getOrInsertFunction(Name, Attr, RetTy, ScopeTy, ScopeTy, SemanticsTy);
assert(FC.getCallee() && "spirv intrinsic creation failed");
if (TT.isSPIR())
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
if (TT.isSPIR())
if (TT.isSPIROrSPIRV())

same for other spot too i think

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done
Also fixed SpecConstantsPass

@wenju-he wenju-he changed the title [SYCL][LowerWGScope] Set new call's calling conv to spirv_func for spir target [SYCL][LowerWGScope][SpecConstantsPass] fix new call's calling conv for spir/spirv Oct 17, 2024
@wenju-he wenju-he requested a review from sarnex October 17, 2024 03:23
@asudarsa
Copy link
Contributor

asudarsa commented Oct 17, 2024

one motivation is intel gpu backend requires calling conv to be consistent.

Thanks for adding this support. Looks good. One question. Is there documentation anywhere that talks about this calling convention and about which functions need this. Surely not a blocker. Thanks

Copy link
Contributor

@sarnex sarnex left a comment

Choose a reason for hiding this comment

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

lgtm as in my feedback is addressed, thx

@wenju-he
Copy link
Contributor Author

wenju-he commented Oct 18, 2024

one motivation is intel gpu backend requires calling conv to be consistent.

Thanks for adding this support. Looks good. One question. Is there documentation anywhere that talks about this calling convention and about which functions need this. Surely not a blocker. Thanks

@asudarsa For spir target, quote from page 33 of https://registry.khronos.org/SPIR/specs/spir_spec-2.0.pdf

3.8 Calling Conventions
SPIR kernels should use "spir_kernel" calling convention. Non-kernel functions use "spir_func"
calling convention. All other calling conventions are disallowed.

So I assume every function call should have either spir_kernel or spir_func calling convention.

I am not ware of any documentation of the requirement of consistent calling convention in the intel gpu backend.

@wenju-he
Copy link
Contributor Author

@intel/llvm-gatekeepers please merge, thank you

@martygrant martygrant merged commit df186f3 into intel:sycl Oct 21, 2024
12 checks passed
@wenju-he wenju-he deleted the LowerWGScope-genWGBarrier-spir_func branch October 22, 2024 00:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants