This is an archive of the discontinued LLVM Phabricator instance.

[OpenCL] Do not use vararg in emitted functions for enqueue_kernel
ClosedPublic

Authored by yaxunl on Aug 14 2017, 6:39 AM.

Details

Summary

Not all targets support vararg (e.g. amdgpu). Instead of using vararg in the emitted functions for enqueue_kernel,
this patch creates a temporary array of size_t, stores the size arguments in the temporary array
and passes it to the emitted functions for enqueue_kernel.

Diff Detail

Repository
rL LLVM

Event Timeline

yaxunl created this revision.Aug 14 2017, 6:39 AM
Anastasia added inline comments.Aug 15 2017, 10:23 AM
test/CodeGenOpenCL/cl20-device-side-enqueue.cl
64 ↗(On Diff #110956)

This is no longer needed?

Could we check the code for array generation too?

Also could we modify one test to take more than argument to the block? It seems to be missing in testing.

yaxunl marked an inline comment as done.Aug 15 2017, 8:36 PM
yaxunl added inline comments.
test/CodeGenOpenCL/cl20-device-side-enqueue.cl
64 ↗(On Diff #110956)

will do.

yaxunl updated this revision to Diff 111302.Aug 15 2017, 8:38 PM
yaxunl marked an inline comment as done.

Revised by Anastasia's comments.

Anastasia added inline comments.Aug 22 2017, 11:21 AM
test/CodeGenOpenCL/cl20-device-side-enqueue.cl
116 ↗(On Diff #111302)

You are not checking the arrays in the other calls too?

yaxunl marked an inline comment as done.Aug 22 2017, 8:24 PM
yaxunl added inline comments.
test/CodeGenOpenCL/cl20-device-side-enqueue.cl
116 ↗(On Diff #111302)

The logic is the same and the same lamba is called for emitting the IR. Is it necessary to do the same check for all the cases?

Anastasia added inline comments.Aug 29 2017, 3:57 AM
test/CodeGenOpenCL/cl20-device-side-enqueue.cl
116 ↗(On Diff #111302)

Ideally yes, we are doing this for other features too... there is only one element in other cases... should be easier.

yaxunl updated this revision to Diff 113483.Aug 31 2017, 3:05 PM
yaxunl marked 3 inline comments as done.

update tests.

Anastasia accepted this revision.Sep 1 2017, 9:36 AM

LGTM. Thanks!

This revision is now accepted and ready to land.Sep 1 2017, 9:36 AM
This revision was automatically updated to reflect the committed changes.