This is an archive of the discontinued LLVM Phabricator instance.

AMDGPU: Set amdgpu_kernel calling convention for OpenCL kernels.
ClosedPublic

Authored by nhaustov on Jun 15 2016, 3:42 AM.

Details

Summary

Summary:
Change Clang calling convention SpirKernel to OpenCLKernel.
Set calling convention OpenCLKernel for amdgcn as well.
Add virtual method .getOpenCLKernelCallingConv() to TargetCodeGenInfo
and use it to set target calling convention for AMDGPU and SPIR.
Update tests.

Diff Detail

Repository
rL LLVM

Event Timeline

nhaustov updated this revision to Diff 60805.Jun 15 2016, 3:42 AM
nhaustov retitled this revision from to AMDGPU: Set amdgpu_kernel calling convention for OpenCL kernels..
nhaustov updated this object.
nhaustov added reviewers: tstellarAMD, rsmith.
nhaustov added a subscriber: cfe-commits.

Does this new patch fix the OpenCL regression?

Does this new patch fix the OpenCL regression?

Yes, it fixes the problem with calling kernel from kernel. I'll add a test too.

nhaustov updated this revision to Diff 61247.Jun 20 2016, 4:26 AM
nhaustov edited edge metadata.

Add test for calling OpenCL kernel from kernel.

This seems OK to me.

yaxunl edited edge metadata.Jun 27 2016, 7:51 AM

I am wondering what's the difference between this calling convention and spir_kernel.

I am wondering what's the difference between this calling convention and spir_kernel.

spir_kernel has only effect and was created only for SPIR target. In theory it could perhaps be reused for AMDGPU target, however it seems not very clear to me.

I am wondering what's the difference between this calling convention and spir_kernel.

spir_kernel has only effect and was created only for SPIR target. In theory it could perhaps be reused for AMDGPU target, however it seems not very clear to me.

If the new calling convention is to indicate a function is an OpenCL kernel, then it has exactly the same meaning as spir_kernel. Can we just use it in AMDGPU target?

I am wondering what's the difference between this calling convention and spir_kernel.

spir_kernel has only effect and was created only for SPIR target. In theory it could perhaps be reused for AMDGPU target, however it seems not very clear to me.

I am wondering what's the difference between this calling convention and spir_kernel.

spir_kernel has only effect and was created only for SPIR target. In theory it could perhaps be reused for AMDGPU target, however it seems not very clear to me.

If the new calling convention is to indicate a function is an OpenCL kernel, then it has exactly the same meaning as spir_kernel. Can we just use it in AMDGPU target?

This looks like a hack to me. Calling conventions in LLVM IR are really target specific. spir_kernel really doesn't make sense for anything other than SPIR.

Maybe we can use the fact that calling conventions are separate in AST and in LLVM IR. So in AST maybe we can have opencl_kernel and in LLVM it can map to spir_kernel for SPIR and amdgpu_kernel for AMDGPU.

Anastasia added inline comments.Jun 28 2016, 9:07 AM
lib/Basic/Targets.cpp
2122 ↗(On Diff #61247)

One extra line?

Maybe we can use the fact that calling conventions are separate in AST and in LLVM IR. So in AST maybe we can have opencl_kernel and in LLVM it can map to spir_kernel for SPIR and amdgpu_kernel for AMDGPU.

This approach looks good to me. Thanks.

nhaustov updated this revision to Diff 62198.Jun 29 2016, 3:17 AM
nhaustov edited edge metadata.

Update after discussion with Sam and review from Anastasia.

nhaustov updated this object.Jun 29 2016, 3:18 AM
yaxunl accepted this revision.Jun 29 2016, 10:14 AM
yaxunl edited edge metadata.

LGTM. Thanks!

This revision is now accepted and ready to land.Jun 29 2016, 10:14 AM
This revision was automatically updated to reflect the committed changes.