This is an archive of the discontinued LLVM Phabricator instance.

[OpenCL][CUDA][HIP][SYCL] Add norecurse
ClosedPublic

Authored by yaxunl on Jan 29 2020, 11:21 AM.

Details

Summary

norecurse function attr indicates the function is not called recursively directly or indirectly.

Add norecurse to OpenCL functions, SYCL functions in device compilation and CUDA/HIP kernels.

Although there is LLVM pass adding norecurse to functions, it only works for whole-program compilation. Also FE adding norecurse can make that pass run faster since functions with norecurse do not need to be checked again.

Diff Detail

Event Timeline

yaxunl created this revision.Jan 29 2020, 11:21 AM
tra accepted this revision.Jan 29 2020, 11:29 AM

LGTM for CUDA.

clang/lib/CodeGen/CodeGenFunction.cpp
929

I believe dynamic parallelism is more like spawning a separate process, not recursion, so we should be OK.

This revision is now accepted and ready to land.Jan 29 2020, 11:29 AM
bader added a subscriber: bader.Jan 29 2020, 11:41 AM
bader added inline comments.
clang/lib/CodeGen/CodeGenFunction.cpp
934

SYCL doesn't support recursion neither. Does it make sense to add it too?

yaxunl updated this revision to Diff 241256.Jan 29 2020, 12:40 PM
yaxunl retitled this revision from [OpenCL][CUDA][HIP] Add norecurse to [OpenCL][CUDA][HIP][SYCL] Add norecurse.
yaxunl edited the summary of this revision. (Show Details)

Added handling of SYCL kernels by Alexey's comments. I cannot add a codegen test for SYCL since I cannot find a way to instantiate a SYCL kernel.

Added handling of SYCL kernels by Alexey's comments. I cannot add a codegen test for SYCL since I cannot find a way to instantiate a SYCL kernel.

I think SYCL implementation should follow OpenCL logic and apply attribute to any function, not only kernels.

yaxunl updated this revision to Diff 241265.Jan 29 2020, 1:01 PM
yaxunl edited the summary of this revision. (Show Details)

revised by Alexey's comments.

t-tye added a subscriber: t-tye.Jan 29 2020, 3:02 PM
t-tye added inline comments.
clang/lib/CodeGen/CodeGenFunction.cpp
929

Right, dynamic parallelism is equivalent to spawn a new thread to execute the function. Recursion is only about the same thread calling the function. So I agree that the TODO about it can be removed.

yaxunl updated this revision to Diff 241335.Jan 29 2020, 6:31 PM

Remove the ToDo for CUDA/HIP.

bader accepted this revision.Jan 29 2020, 11:53 PM

Thanks!

keryell requested changes to this revision.Jan 31 2020, 7:05 PM
keryell added a subscriber: keryell.
keryell added inline comments.
clang/lib/CodeGen/CodeGenFunction.cpp
929

Can you change the SYCL version to "SYCL 1.2.1 s3.10"?
SYCL 2.2 is non normative: it was just a provisional version which is now deprecated and the SYCL committee is working on a newer version.
Thanks.

This revision now requires changes to proceed.Jan 31 2020, 7:05 PM
yaxunl updated this revision to Diff 241877.Feb 1 2020, 6:26 AM

Fix SYCL version.

Anastasia added inline comments.Feb 4 2020, 3:04 AM
clang/lib/CodeGen/CodeGenFunction.cpp
925

Can we remove reference to OpenCL C++ since it's not implemented. Feel free to add C++ for OpenCL instead. The comment explanation still makes sense. You can just remove references to docs.

yaxunl updated this revision to Diff 242300.Feb 4 2020, 5:09 AM
yaxunl marked 5 inline comments as done.

revised by Anastasia's comments.

Anastasia accepted this revision.Feb 5 2020, 5:19 AM

LGTM! Thanks!

This revision was not accepted when it landed; it landed in state Needs Review.Feb 16 2020, 5:53 PM
This revision was automatically updated to reflect the committed changes.
Herald added a project: Restricted Project. · View Herald TranscriptFeb 16 2020, 5:53 PM