Page MenuHomePhabricator

[OpenCL] Split type and macro definitions into opencl-c-base.h
ClosedPublic

Authored by svenvh on Jun 13 2019, 5:37 AM.

Details

Summary

Using the -fdeclare-opencl-builtins option will require a way to
predefine types and macros such as int4, CLK_GLOBAL_MEM_FENCE,
etc. Move these out of opencl-c.h into opencl-c-base.h that is shared
by -fdeclare-opencl-builtins and -finclude-default-header.

Diff Detail

Repository
rL LLVM

Event Timeline

svenvh created this revision.Jun 13 2019, 5:37 AM
Herald added a project: Restricted Project. · View Herald TranscriptJun 13 2019, 5:37 AM

LGTM, except for the IncludeDefaultHeader and DeclareOpenCLBuiltins logic.

lib/Frontend/CompilerInvocation.cpp
2194 ↗(On Diff #204501)

Can we have the following logic instead? Otherwise DeclareOpenCLBuiltins implies IncludeDefaultHeader which is not always the case.

    if (Opts.IncludeDefaultHeader) {
      if (Opts.DeclareOpenCLBuiltins) {
	PPOpts.Includes.push_back("opencl-c-base.h");
      } else {
	PPOpts.Includes.push_back("opencl-c.h");
      }
    }
svenvh updated this revision to Diff 204796.Jun 14 2019, 9:47 AM

Change IncludeDefaultHeader and DeclareOpenCLBuiltins logic as per @asavonic's comments. Also extend test/SemaOpenCL/fdeclare-opencl-builtins.cl to test the interplay of both options.

asavonic accepted this revision.Jun 18 2019, 3:22 AM

Thanks. LGTM.

This revision is now accepted and ready to land.Jun 18 2019, 3:22 AM
This revision was automatically updated to reflect the committed changes.
Herald added a project: Restricted Project. · View Herald TranscriptJun 19 2019, 5:45 AM
cfe/trunk/lib/Headers/opencl-c.h
13638–13640

Any reason this piece of code got completely removed?

yaxunl added inline comments.Jun 21 2019, 1:50 PM
cfe/trunk/lib/Headers/opencl-c.h
13638

Removing ATOMIC_VAR_INIT caused regression in our compiler.

Could you please put it back?

Thanks.

svenvh marked 3 inline comments as done.Mon, Jun 24, 3:09 AM
svenvh added inline comments.
cfe/trunk/lib/Headers/opencl-c.h
13638–13640

Apologies, that was not intended; thanks for reporting! Restored the macro in r364174 together with a test.

kzhuravl added inline comments.Mon, Jun 24, 9:28 AM
cfe/trunk/lib/Headers/opencl-c.h
13638–13640

Thanks!