Page MenuHomePhabricator

[OpenCL] Use SPIR_CC by default for kernels
ClosedPublic

Authored by pekka.jaaskelainen on May 29 2017, 12:05 AM.

Details

Reviewers
Anastasia
Summary

This patch makes kernels use the SPIR_KERNEL CC by default.

Rationale: OpenCL kernels are called via an explicit runtime API with arguments set with clSetKernelArg(), not as normal sub-functions. Return SPIR_KERNEL by default as the kernel calling convention to ensure the fingerprint is fixed such way that each OpenCL argument gets one matching argument in the produced kernel function argument list to enable feasible implementation of clSetKernelArg() with aggregates etc. In case we would use the default C calling conv here, clSetKernelArg() might break depending on the target-specific conventions; different targets might split structs passed as values to multiple function arguments etc.

See: https://github.com/pocl/pocl/issues/1

Diff Detail

Event Timeline

It seems the reviewer is blocked by accident.

test/CodeGenOpenCL/kernels-have-spir-cc-by-default.cl
5

amdgcn target has specific calling convention for OpenCL kernels. Can we add a run line for amdgcn target to make sure there is no regression? Thanks.

pekka.jaaskelainen removed 1 blocking reviewer(s): Anastasia.May 29 2017, 9:44 PM

It seems the reviewer is blocked by accident.

Yes! Sorry Anastasia. I thought "blocked" meant: "this revision is blocked being accepted before reviewed by XX" :)

Added the requested AMDGCN test case.

Anastasia accepted this revision.May 31 2017, 10:39 AM

LGTM! I think it makes sense to use default C calling convention for kernels in all targets.

Thanks!

lib/CodeGen/TargetInfo.cpp
400

The lines above are missing... is this for default target?

411

I think we can remove the link, since it can be found from the review.

test/CodeGenOpenCL/kernels-have-spir-cc-by-default.cl
4

We normally have RUN line first!

This revision is now accepted and ready to land.May 31 2017, 10:39 AM
pekka.jaaskelainen marked 4 inline comments as done.

Committed in r304389.

lib/CodeGen/TargetInfo.cpp
400

Yes.