This is an archive of the discontinued LLVM Phabricator instance.

[PR41007][OpenCL] Allow printf and toolchain reserved variadic functions in C++
ClosedPublic

Authored by Anastasia on Mar 11 2019, 9:39 AM.

Details

Summary

As for OpenCL C, we will allow printf and other variadic functions (prefixed by "__") in C++ mode.

Diff Detail

Repository
rL LLVM

Event Timeline

Anastasia created this revision.Mar 11 2019, 9:39 AM
This revision is now accepted and ready to land.Mar 11 2019, 10:20 AM
This revision was automatically updated to reflect the committed changes.
Herald added a project: Restricted Project. · View Herald TranscriptMar 12 2019, 5:45 AM
bader added a subscriber: bader.Mar 12 2019, 9:58 AM
bader added inline comments.
cfe/trunk/test/SemaOpenCL/extensions.cl
31

Shoudn't we move this test to test/SemaOpenCLCXX?
Does -finclude-default-header include opencl-c.h? I think it's an overkill to test that OpenCL C++ allows printf. Ideally we should minimize number of times we parsing 11500+ lines header in LIT tests.

Anastasia marked an inline comment as done.Mar 14 2019, 9:52 AM
Anastasia added inline comments.
cfe/trunk/test/SemaOpenCL/extensions.cl
31

Shoudn't we move this test to test/SemaOpenCLCXX?

Right now test/SemaOpenCLCXX and other corresponding folders contain only functionality specific to C++.

In test/SemaOpenCL we will keep tests that are OpenCL C compatible even though we are parsing some of them in C++ mode too. We could create a common folder but I don't see much value in this. Let me know if you have other thoughts.

Does -finclude-default-header include opencl-c.h? I think it's an overkill to test that OpenCL C++ allows printf. Ideally we should minimize number of times we parsing 11500+ lines header in LIT tests.

Yes, I perfectly agree. I would like to make sure that the header is parsed successfully in C++ mode. So it is not just for printf. There are a few other places where we parse the header, I can move this into them if you prefer. May be test/Driver/include-default-header.cl would make more sense to show the intent?

bader added a comment.Mar 15 2019, 7:58 AM

May be test/Driver/include-default-header.cl would make more sense to show the intent?

test/Headers/opencl-c-header.cl sounds like good candidate.