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.
Details
Details
Diff Detail
Diff Detail
Event Timeline
Comment Actions
LGTM, except for the IncludeDefaultHeader and DeclareOpenCLBuiltins logic.
| lib/Frontend/CompilerInvocation.cpp | ||
|---|---|---|
| 2194 | 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");
}
} | |
Comment Actions
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.
Comment Actions
| cfe/trunk/lib/Headers/opencl-c.h | ||
|---|---|---|
| 13638–13640 ↗ | (On Diff #205558) | Any reason this piece of code got completely removed? |
| cfe/trunk/lib/Headers/opencl-c.h | ||
|---|---|---|
| 13638 ↗ | (On Diff #205558) | Removing ATOMIC_VAR_INIT caused regression in our compiler. Could you please put it back? Thanks. |
| cfe/trunk/lib/Headers/opencl-c.h | ||
|---|---|---|
| 13638–13640 ↗ | (On Diff #205558) | Apologies, that was not intended; thanks for reporting! Restored the macro in r364174 together with a test. |
| cfe/trunk/lib/Headers/opencl-c.h | ||
|---|---|---|
| 13638–13640 ↗ | (On Diff #205558) | Thanks! |
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"); } }