[PATCH] D20444: [OpenCL] Include opencl-c.h by default as a clang module

[Anastasia Stulova via cfe-commits]

Anastasia added inline comments.

================
Comment at: test/Headers/opencl-c-header.cl:53-54
@@ +52,4 @@
+// RUN: %clang_cc1 -triple spir-unknown-unknown -emit-llvm -o - -finclude-default-header -fmodules -fimplicit-module-maps -fmodules-cache-path=%t -fdisable-module-hash -ftime-report %s 2>&1 | FileCheck --check-prefix=CHECK --check-prefix=CHECK-MOD %s
+// RUN: chmod u+w %t/opencl_c.pcm
+// RUN: mv %t/opencl_c.pcm %t/1_0.pcm
+
----------------
yaxunl wrote:
> I checked the size of the pch file. The largest chunk is for function declarations. Since it contains about 30k declarations, each one is about 50 bytes on average, so total size is about 1.6MB.
> 
> The AST of function decl is written by ASTDeclWriter::VisitFunctionDecl http://clang.llvm.org/doxygen/ASTWriterDecl_8cpp_source.html 
> 
> A code snippet is as follows:
> 
>  515   Record.push_back((int)D->SClass); // FIXME: stable encoding
>   516   Record.push_back(D->IsInline);
>   517   Record.push_back(D->IsInlineSpecified);
>   518   Record.push_back(D->IsVirtualAsWritten);
>   519   Record.push_back(D->IsPure);
>   520   Record.push_back(D->HasInheritedPrototype);
>   521   Record.push_back(D->HasWrittenPrototype);
>   522   Record.push_back(D->IsDeleted);
>   523   Record.push_back(D->IsTrivial);
>   524   Record.push_back(D->IsDefaulted);
>   525   Record.push_back(D->IsExplicitlyDefaulted);
>   526   Record.push_back(D->HasImplicitReturnZero);
>   527   Record.push_back(D->IsConstexpr);
>   528   Record.push_back(D->HasSkippedBody);
>   529   Record.push_back(D->IsLateTemplateParsed);
>   530   Record.push_back(D->getLinkageInternal());
>   531   Record.AddSourceLocation(D->getLocEnd());
> 
> Record is like a buffer which will be written to file. It uses a vector of int64 to store the values, so it takes space.
It feels the problem is that it has to record all the attributes even though most of them are missing. Quite wasteful! Wondering if variable length solution would be possible...

================
Comment at: test/Headers/opencl-c-header.cl:70
@@ +69,3 @@
+// RUN: %clang_cc1 -triple spir-unknown-unknown -emit-llvm -o - -finclude-default-header -fmodules -fimplicit-module-maps -fmodules-cache-path=%t %s | FileCheck %s
+// RUN: %clang_cc1 -triple spir-unknown-unknown -emit-llvm -o - -cl-std=CL2.0 -finclude-default-header -fmodules -fimplicit-module-maps -fmodules-cache-path=%t %s | FileCheck --check-prefix=CHECK20 %s
+// RUN: %clang_cc1 -triple amdgcn--amdhsa -emit-llvm -o - -cl-std=CL2.0  -finclude-default-header -fmodules -fimplicit-module-maps -fmodules-cache-path=%t %s | FileCheck --check-prefix=CHECK20 %s
----------------
yaxunl wrote:
> Anastasia wrote:
> > yaxunl wrote:
> > > I added check to the second compilation to make sure module is read, also changed the modules to be read only so that they won't be created again.
> > Ok, now I see what you are testing here. :)
> > 
> > Do you think we could add:
> >   CHECK-NOT: Reading modules
> > 
> > For the cases the modules are regenerated new?
> In the case the modules are generated as new, Clang will generate the module first and then load it. So in the time report, you still see 'Reading modules'.
In this case, should you be testing it in regenerated mode too?

Also would it make sense to add:
  CHECK-NOT: _Z3ctzc
to make sure there isn't OpenCL 2.0 functionality.


http://reviews.llvm.org/D20444