[PATCH] D88680: Add ability to turn off -fpch-instantiate-templates in clang-cl

[Luboš Luňák via Phabricator via cfe-commits]

llunak added a comment.

LGTM, although I'm not sure if me saying that is enough. I'll commit this in 
few days if nobody says anything else.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D88680/new/

https://reviews.llvm.org/D88680

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D88680: Add ability to turn off -fpch-instantiate-templates in clang-cl

[Luboš Luňák via Phabricator via cfe-commits]

llunak added a comment.

> I tried pretty hard to get a small repro for these failures, but couldn't.

Can you get at least some testcase, even if not small? You can use -E 
-frewrite-includes to create a single large file from all the input. Although 
the patch looks fine to me as such, I consider it a workaround. The reason for 
-fpch-instantiate-templates being the default for clang-cl is that MSVC does 
some kind of template instantiation for PCHs too (although seeing your email 
you probably know more than I do). So preferably the core problem should be 
fixed.

In D88680#2307564 , @rnk wrote:

> I think the flag was originally intended to be an internal -cc1 flag not 
> exposed to users.

No, it's intentionally exposed in the gcc-compatible driver, because there the 
option is not the default because of corner cases if the PCH is not 
self-contained. But since MSVC requires PCHs to be self-contained, it was 
deemed safe to always enable it in clang-cl. And, as said above, I still think 
it generally should.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D88680/new/

https://reviews.llvm.org/D88680

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[Differential] D83716: Accept 'clang++ -c a.pch -o a.o' to create PCH's object file

[Luboš Luňák via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG3895466e2c33: accept clang++ -c a.pch -o a.o to 
create PCHs object file (authored by llunak).
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Changed prior to commit:
  https://reviews.llvm.org/D83716?vs=277670=279073#toc

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D83716/new/

https://reviews.llvm.org/D83716

Files:
  clang/lib/Driver/Types.cpp
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/test/Driver/pch-codegen.cpp
  clang/test/PCH/codegen.cpp

Index: clang/test/PCH/codegen.cpp
===
--- clang/test/PCH/codegen.cpp
+++ clang/test/PCH/codegen.cpp
@@ -9,8 +9,8 @@
 // RUN: %clang_cc1 -triple=x86_64-linux-gnu -fmodules-codegen -x c++-header -building-pch-with-obj -emit-pch %S/../Modules/Inputs/codegen-flags/foo.h -o %t/foo-cg.pch
 // RUN: %clang_cc1 -triple=x86_64-linux-gnu -fmodules-debuginfo -x c++-header -building-pch-with-obj -emit-pch %S/../Modules/Inputs/codegen-flags/foo.h -o %t/foo-di.pch
 
-// RUN: %clang_cc1 -triple x86_64-linux-gnu -emit-llvm -debug-info-kind=limited -o - %s -include-pch %t/foo-cg.pch -building-pch-with-obj -fmodules-codegen | FileCheck --check-prefix=CG %s
-// RUN: %clang_cc1 -triple x86_64-linux-gnu -emit-llvm -debug-info-kind=limited -o - %s -include-pch %t/foo-di.pch -building-pch-with-obj -fmodules-debuginfo | FileCheck --check-prefix=DI %s
+// RUN: %clang_cc1 -triple x86_64-linux-gnu -emit-llvm -debug-info-kind=limited -o - -x precompiled-header %t/foo-cg.pch | FileCheck --check-prefix=CG %s
+// RUN: %clang_cc1 -triple x86_64-linux-gnu -emit-llvm -debug-info-kind=limited -o - -x precompiled-header %t/foo-di.pch | FileCheck --check-prefix=DI %s
 
 // RUN: %clang_cc1 -triple x86_64-linux-gnu -emit-llvm -debug-info-kind=limited -o - -include-pch %t/foo-cg.pch %S/../Modules/Inputs/codegen-flags/use.cpp | FileCheck --check-prefix=CG-USE %s
 // RUN: %clang_cc1 -triple x86_64-linux-gnu -emit-llvm -debug-info-kind=limited -o - -include-pch %t/foo-di.pch %S/../Modules/Inputs/codegen-flags/use.cpp | FileCheck --check-prefix=DI-USE %s
@@ -20,8 +20,8 @@
 // RUN: %clang_cc1 -triple=x86_64-linux-gnu -fmodules-codegen -x c++-header -building-pch-with-obj -emit-pch -fpch-instantiate-templates %S/../Modules/Inputs/codegen-flags/foo.h -o %t/foo-cg.pch
 // RUN: %clang_cc1 -triple=x86_64-linux-gnu -fmodules-debuginfo -x c++-header -building-pch-with-obj -emit-pch -fpch-instantiate-templates %S/../Modules/Inputs/codegen-flags/foo.h -o %t/foo-di.pch
 
-// RUN: %clang_cc1 -triple x86_64-linux-gnu -emit-llvm -debug-info-kind=limited -o - %s -include-pch %t/foo-cg.pch -building-pch-with-obj -fmodules-codegen | FileCheck --check-prefix=CG %s
-// RUN: %clang_cc1 -triple x86_64-linux-gnu -emit-llvm -debug-info-kind=limited -o - %s -include-pch %t/foo-di.pch -building-pch-with-obj -fmodules-debuginfo | FileCheck --check-prefix=DI %s
+// RUN: %clang_cc1 -triple x86_64-linux-gnu -emit-llvm -debug-info-kind=limited -o - -x precompiled-header %t/foo-cg.pch | FileCheck --check-prefix=CG %s
+// RUN: %clang_cc1 -triple x86_64-linux-gnu -emit-llvm -debug-info-kind=limited -o - -x precompiled-header %t/foo-di.pch | FileCheck --check-prefix=DI %s
 
 // RUN: %clang_cc1 -triple x86_64-linux-gnu -emit-llvm -debug-info-kind=limited -o - -include-pch %t/foo-cg.pch %S/../Modules/Inputs/codegen-flags/use.cpp | FileCheck --check-prefix=CG-USE %s
 // RUN: %clang_cc1 -triple x86_64-linux-gnu -emit-llvm -debug-info-kind=limited -o - -include-pch %t/foo-di.pch %S/../Modules/Inputs/codegen-flags/use.cpp | FileCheck --check-prefix=DI-USE %s
Index: clang/test/Driver/pch-codegen.cpp
===
--- /dev/null
+++ clang/test/Driver/pch-codegen.cpp
@@ -0,0 +1,38 @@
+// RUN: rm -rf %t
+// RUN: mkdir -p %t
+
+// Create PCH without codegen.
+// RUN: %clang -x c++-header %S/../Modules/Inputs/codegen-flags/foo.h -o %t/foo-cg.pch -### 2>&1 | FileCheck %s -check-prefix=CHECK-PCH-CREATE
+// CHECK-PCH-CREATE: -emit-pch
+// CHECK-PCH-CREATE-NOT: -fmodules-codegen
+// CHECK-PCH-CREATE-NOT: -fmodules-debuginfo
+
+// Create PCH with -fmodules-codegen.
+// RUN: %clang -x c++-header -Xclang -fmodules-codegen %S/../Modules/Inputs/codegen-flags/foo.h -o %t/foo-cg.pch -### 2>&1 | FileCheck %s -check-prefix=CHECK-PCH-CODEGEN-CREATE
+// CHECK-PCH-CODEGEN-CREATE: -emit-pch
+// CHECK-PCH-CODEGEN-CREATE: -fmodules-codegen
+// CHECK-PCH-CODEGEN-CREATE: "-x" "c++-header"
+// CHECK-PCH-CODEGEN-CREATE-NOT: -fmodules-debuginfo
+
+// Create PCH with -fmodules-debuginfo.
+// RUN: %clang -x c++-header -Xclang -fmodules-debuginfo %S/../Modules/Inputs/codegen-flags/foo.h -g -o %t/foo-di.pch -### 2>&1 | FileCheck %s -check-prefix=CHECK-PCH-DEBUGINFO-CREATE
+// CHECK-PCH-DEBUGINFO-CREATE: -emit-pch
+// CHECK-PCH-DEBUGINFO-CREATE: -fmodules-debuginfo
+// 

[Differential] D83623: add -fpch-codegen/debuginfo options mapping to -fmodules-codegen/debuginfo

[Luboš Luňák via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG54eea6127c4d: add -fpch-codegen/debuginfo mapping to 
-fmodules-codegen/debuginfo (authored by llunak).
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Changed prior to commit:
  https://reviews.llvm.org/D83623?vs=277671=279074#toc

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D83623/new/

https://reviews.llvm.org/D83623

Files:
  clang/docs/ReleaseNotes.rst
  clang/include/clang/Driver/Options.td
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/test/Driver/pch-codegen.cpp

Index: clang/test/Driver/pch-codegen.cpp
===
--- clang/test/Driver/pch-codegen.cpp
+++ clang/test/Driver/pch-codegen.cpp
@@ -7,21 +7,21 @@
 // CHECK-PCH-CREATE-NOT: -fmodules-codegen
 // CHECK-PCH-CREATE-NOT: -fmodules-debuginfo
 
-// Create PCH with -fmodules-codegen.
-// RUN: %clang -x c++-header -Xclang -fmodules-codegen %S/../Modules/Inputs/codegen-flags/foo.h -o %t/foo-cg.pch -### 2>&1 | FileCheck %s -check-prefix=CHECK-PCH-CODEGEN-CREATE
+// Create PCH with -fpch-codegen.
+// RUN: %clang -x c++-header -fpch-codegen %S/../Modules/Inputs/codegen-flags/foo.h -o %t/foo-cg.pch -### 2>&1 | FileCheck %s -check-prefix=CHECK-PCH-CODEGEN-CREATE
 // CHECK-PCH-CODEGEN-CREATE: -emit-pch
 // CHECK-PCH-CODEGEN-CREATE: -fmodules-codegen
 // CHECK-PCH-CODEGEN-CREATE: "-x" "c++-header"
 // CHECK-PCH-CODEGEN-CREATE-NOT: -fmodules-debuginfo
 
-// Create PCH with -fmodules-debuginfo.
-// RUN: %clang -x c++-header -Xclang -fmodules-debuginfo %S/../Modules/Inputs/codegen-flags/foo.h -g -o %t/foo-di.pch -### 2>&1 | FileCheck %s -check-prefix=CHECK-PCH-DEBUGINFO-CREATE
+// Create PCH with -fpch-debuginfo.
+// RUN: %clang -x c++-header -fpch-debuginfo %S/../Modules/Inputs/codegen-flags/foo.h -g -o %t/foo-di.pch -### 2>&1 | FileCheck %s -check-prefix=CHECK-PCH-DEBUGINFO-CREATE
 // CHECK-PCH-DEBUGINFO-CREATE: -emit-pch
 // CHECK-PCH-DEBUGINFO-CREATE: -fmodules-debuginfo
 // CHECK-PCH-DEBUGINFO-CREATE: "-x" "c++-header"
 // CHECK-PCH-DEBUGINFO-CREATE-NOT: -fmodules-codegen
 
-// Create PCH's object file for -fmodules-codegen.
+// Create PCH's object file for -fpch-codegen.
 // RUN: touch %t/foo-cg.pch
 // RUN: %clang -c %t/foo-cg.pch -o %t/foo-cg.o -### 2>&1 | FileCheck %s -check-prefix=CHECK-PCH-CODEGEN-OBJ
 // CHECK-PCH-CODEGEN-OBJ: -emit-obj
@@ -29,7 +29,7 @@
 // CHECK-PCH-CODEGEN-OBJ: "-o" "{{.*}}foo-cg.o"
 // CHECK-PCH-CODEGEN-OBJ: "-x" "precompiled-header"
 
-// Create PCH's object file for -fmodules-debuginfo.
+// Create PCH's object file for -fpch-debuginfo.
 // RUN: touch %t/foo-di.pch
 // RUN: %clang -c %t/foo-di.pch -g -o %t/foo-di.o -### 2>&1 | FileCheck %s -check-prefix=CHECK-PCH-DEBUGINFO-OBJ
 // CHECK-PCH-DEBUGINFO-OBJ: -emit-obj
Index: clang/lib/Driver/ToolChains/Clang.cpp
===
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -5642,6 +5642,12 @@
   if (Args.hasFlag(options::OPT_fpch_instantiate_templates,
options::OPT_fno_pch_instantiate_templates, false))
 CmdArgs.push_back("-fpch-instantiate-templates");
+  if (Args.hasFlag(options::OPT_fpch_codegen, options::OPT_fno_pch_codegen,
+   false))
+CmdArgs.push_back("-fmodules-codegen");
+  if (Args.hasFlag(options::OPT_fpch_debuginfo, options::OPT_fno_pch_debuginfo,
+   false))
+CmdArgs.push_back("-fmodules-debuginfo");
 
   Args.AddLastArg(CmdArgs, options::OPT_fexperimental_new_pass_manager,
   options::OPT_fno_experimental_new_pass_manager);
Index: clang/include/clang/Driver/Options.td
===
--- clang/include/clang/Driver/Options.td
+++ clang/include/clang/Driver/Options.td
@@ -1440,6 +1440,10 @@
 def fno_pch_instantiate_templates:
   Flag <["-"], "fno-pch-instantiate-templates">,
   Group, Flags<[CC1Option]>;
+defm pch_codegen: OptInFFlag<"pch-codegen", "Generate ", "Do not generate ",
+  "code for uses of this PCH that assumes an explicit object file will be built for the PCH">;
+defm pch_debuginfo: OptInFFlag<"pch-debuginfo", "Generate ", "Do not generate ",
+  "debug info for types in an object file built from this PCH and do not generate them elsewhere">;
 
 def fmodules : Flag <["-"], "fmodules">, Group,
   Flags<[DriverOption, CC1Option]>,
Index: clang/docs/ReleaseNotes.rst
===
--- clang/docs/ReleaseNotes.rst
+++ clang/docs/ReleaseNotes.rst
@@ -63,6 +63,32 @@
 
 - ...
 
+- -fpch-codegen and -fpch-debuginfo generate shared code and/or debuginfo
+  for contents of a precompiled header in a separate object file. This object
+  file needs to be linked in, but its contents do not need to be generated
+  for other objects using the precompiled header. This should 

[PATCH] D83716: Accept 'clang++ -c a.pch -o a.o' to create PCH's object file

[Luboš Luňák via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG3895466e2c33: accept clang++ -c a.pch -o a.o to 
create PCHs object file (authored by llunak).
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D83716/new/

https://reviews.llvm.org/D83716

Files:
  clang/lib/Driver/Types.cpp
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/test/Driver/pch-codegen.cpp
  clang/test/PCH/codegen.cpp

Index: clang/test/PCH/codegen.cpp
===
--- clang/test/PCH/codegen.cpp
+++ clang/test/PCH/codegen.cpp
@@ -9,8 +9,8 @@
 // RUN: %clang_cc1 -triple=x86_64-linux-gnu -fmodules-codegen -x c++-header -building-pch-with-obj -emit-pch %S/../Modules/Inputs/codegen-flags/foo.h -o %t/foo-cg.pch
 // RUN: %clang_cc1 -triple=x86_64-linux-gnu -fmodules-debuginfo -x c++-header -building-pch-with-obj -emit-pch %S/../Modules/Inputs/codegen-flags/foo.h -o %t/foo-di.pch
 
-// RUN: %clang_cc1 -triple x86_64-linux-gnu -emit-llvm -debug-info-kind=limited -o - %s -include-pch %t/foo-cg.pch -building-pch-with-obj -fmodules-codegen | FileCheck --check-prefix=CG %s
-// RUN: %clang_cc1 -triple x86_64-linux-gnu -emit-llvm -debug-info-kind=limited -o - %s -include-pch %t/foo-di.pch -building-pch-with-obj -fmodules-debuginfo | FileCheck --check-prefix=DI %s
+// RUN: %clang_cc1 -triple x86_64-linux-gnu -emit-llvm -debug-info-kind=limited -o - -x precompiled-header %t/foo-cg.pch | FileCheck --check-prefix=CG %s
+// RUN: %clang_cc1 -triple x86_64-linux-gnu -emit-llvm -debug-info-kind=limited -o - -x precompiled-header %t/foo-di.pch | FileCheck --check-prefix=DI %s
 
 // RUN: %clang_cc1 -triple x86_64-linux-gnu -emit-llvm -debug-info-kind=limited -o - -include-pch %t/foo-cg.pch %S/../Modules/Inputs/codegen-flags/use.cpp | FileCheck --check-prefix=CG-USE %s
 // RUN: %clang_cc1 -triple x86_64-linux-gnu -emit-llvm -debug-info-kind=limited -o - -include-pch %t/foo-di.pch %S/../Modules/Inputs/codegen-flags/use.cpp | FileCheck --check-prefix=DI-USE %s
@@ -20,8 +20,8 @@
 // RUN: %clang_cc1 -triple=x86_64-linux-gnu -fmodules-codegen -x c++-header -building-pch-with-obj -emit-pch -fpch-instantiate-templates %S/../Modules/Inputs/codegen-flags/foo.h -o %t/foo-cg.pch
 // RUN: %clang_cc1 -triple=x86_64-linux-gnu -fmodules-debuginfo -x c++-header -building-pch-with-obj -emit-pch -fpch-instantiate-templates %S/../Modules/Inputs/codegen-flags/foo.h -o %t/foo-di.pch
 
-// RUN: %clang_cc1 -triple x86_64-linux-gnu -emit-llvm -debug-info-kind=limited -o - %s -include-pch %t/foo-cg.pch -building-pch-with-obj -fmodules-codegen | FileCheck --check-prefix=CG %s
-// RUN: %clang_cc1 -triple x86_64-linux-gnu -emit-llvm -debug-info-kind=limited -o - %s -include-pch %t/foo-di.pch -building-pch-with-obj -fmodules-debuginfo | FileCheck --check-prefix=DI %s
+// RUN: %clang_cc1 -triple x86_64-linux-gnu -emit-llvm -debug-info-kind=limited -o - -x precompiled-header %t/foo-cg.pch | FileCheck --check-prefix=CG %s
+// RUN: %clang_cc1 -triple x86_64-linux-gnu -emit-llvm -debug-info-kind=limited -o - -x precompiled-header %t/foo-di.pch | FileCheck --check-prefix=DI %s
 
 // RUN: %clang_cc1 -triple x86_64-linux-gnu -emit-llvm -debug-info-kind=limited -o - -include-pch %t/foo-cg.pch %S/../Modules/Inputs/codegen-flags/use.cpp | FileCheck --check-prefix=CG-USE %s
 // RUN: %clang_cc1 -triple x86_64-linux-gnu -emit-llvm -debug-info-kind=limited -o - -include-pch %t/foo-di.pch %S/../Modules/Inputs/codegen-flags/use.cpp | FileCheck --check-prefix=DI-USE %s
Index: clang/test/Driver/pch-codegen.cpp
===
--- /dev/null
+++ clang/test/Driver/pch-codegen.cpp
@@ -0,0 +1,38 @@
+// RUN: rm -rf %t
+// RUN: mkdir -p %t
+
+// Create PCH without codegen.
+// RUN: %clang -x c++-header %S/../Modules/Inputs/codegen-flags/foo.h -o %t/foo-cg.pch -### 2>&1 | FileCheck %s -check-prefix=CHECK-PCH-CREATE
+// CHECK-PCH-CREATE: -emit-pch
+// CHECK-PCH-CREATE-NOT: -fmodules-codegen
+// CHECK-PCH-CREATE-NOT: -fmodules-debuginfo
+
+// Create PCH with -fmodules-codegen.
+// RUN: %clang -x c++-header -Xclang -fmodules-codegen %S/../Modules/Inputs/codegen-flags/foo.h -o %t/foo-cg.pch -### 2>&1 | FileCheck %s -check-prefix=CHECK-PCH-CODEGEN-CREATE
+// CHECK-PCH-CODEGEN-CREATE: -emit-pch
+// CHECK-PCH-CODEGEN-CREATE: -fmodules-codegen
+// CHECK-PCH-CODEGEN-CREATE: "-x" "c++-header"
+// CHECK-PCH-CODEGEN-CREATE-NOT: -fmodules-debuginfo
+
+// Create PCH with -fmodules-debuginfo.
+// RUN: %clang -x c++-header -Xclang -fmodules-debuginfo %S/../Modules/Inputs/codegen-flags/foo.h -g -o %t/foo-di.pch -### 2>&1 | FileCheck %s -check-prefix=CHECK-PCH-DEBUGINFO-CREATE
+// CHECK-PCH-DEBUGINFO-CREATE: -emit-pch
+// CHECK-PCH-DEBUGINFO-CREATE: -fmodules-debuginfo
+// CHECK-PCH-DEBUGINFO-CREATE: "-x" "c++-header"
+// CHECK-PCH-DEBUGINFO-CREATE-NOT: -fmodules-codegen
+
+// 

[PATCH] D83623: add -fpch-codegen/debuginfo options mapping to -fmodules-codegen/debuginfo

[Luboš Luňák via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG54eea6127c4d: add -fpch-codegen/debuginfo mapping to 
-fmodules-codegen/debuginfo (authored by llunak).
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Changed prior to commit:
  https://reviews.llvm.org/D83623?vs=278998=279720#toc

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D83623/new/

https://reviews.llvm.org/D83623

Files:
  clang/docs/ReleaseNotes.rst
  clang/include/clang/Driver/Options.td
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/test/Driver/pch-codegen.cpp

Index: clang/test/Driver/pch-codegen.cpp
===
--- clang/test/Driver/pch-codegen.cpp
+++ clang/test/Driver/pch-codegen.cpp
@@ -7,21 +7,21 @@
 // CHECK-PCH-CREATE-NOT: -fmodules-codegen
 // CHECK-PCH-CREATE-NOT: -fmodules-debuginfo
 
-// Create PCH with -fmodules-codegen.
-// RUN: %clang -x c++-header -Xclang -fmodules-codegen %S/../Modules/Inputs/codegen-flags/foo.h -o %t/foo-cg.pch -### 2>&1 | FileCheck %s -check-prefix=CHECK-PCH-CODEGEN-CREATE
+// Create PCH with -fpch-codegen.
+// RUN: %clang -x c++-header -fpch-codegen %S/../Modules/Inputs/codegen-flags/foo.h -o %t/foo-cg.pch -### 2>&1 | FileCheck %s -check-prefix=CHECK-PCH-CODEGEN-CREATE
 // CHECK-PCH-CODEGEN-CREATE: -emit-pch
 // CHECK-PCH-CODEGEN-CREATE: -fmodules-codegen
 // CHECK-PCH-CODEGEN-CREATE: "-x" "c++-header"
 // CHECK-PCH-CODEGEN-CREATE-NOT: -fmodules-debuginfo
 
-// Create PCH with -fmodules-debuginfo.
-// RUN: %clang -x c++-header -Xclang -fmodules-debuginfo %S/../Modules/Inputs/codegen-flags/foo.h -g -o %t/foo-di.pch -### 2>&1 | FileCheck %s -check-prefix=CHECK-PCH-DEBUGINFO-CREATE
+// Create PCH with -fpch-debuginfo.
+// RUN: %clang -x c++-header -fpch-debuginfo %S/../Modules/Inputs/codegen-flags/foo.h -g -o %t/foo-di.pch -### 2>&1 | FileCheck %s -check-prefix=CHECK-PCH-DEBUGINFO-CREATE
 // CHECK-PCH-DEBUGINFO-CREATE: -emit-pch
 // CHECK-PCH-DEBUGINFO-CREATE: -fmodules-debuginfo
 // CHECK-PCH-DEBUGINFO-CREATE: "-x" "c++-header"
 // CHECK-PCH-DEBUGINFO-CREATE-NOT: -fmodules-codegen
 
-// Create PCH's object file for -fmodules-codegen.
+// Create PCH's object file for -fpch-codegen.
 // RUN: touch %t/foo-cg.pch
 // RUN: %clang -c %t/foo-cg.pch -o %t/foo-cg.o -### 2>&1 | FileCheck %s -check-prefix=CHECK-PCH-CODEGEN-OBJ
 // CHECK-PCH-CODEGEN-OBJ: -emit-obj
@@ -29,7 +29,7 @@
 // CHECK-PCH-CODEGEN-OBJ: "-o" "{{.*}}foo-cg.o"
 // CHECK-PCH-CODEGEN-OBJ: "-x" "precompiled-header"
 
-// Create PCH's object file for -fmodules-debuginfo.
+// Create PCH's object file for -fpch-debuginfo.
 // RUN: touch %t/foo-di.pch
 // RUN: %clang -c %t/foo-di.pch -g -o %t/foo-di.o -### 2>&1 | FileCheck %s -check-prefix=CHECK-PCH-DEBUGINFO-OBJ
 // CHECK-PCH-DEBUGINFO-OBJ: -emit-obj
Index: clang/lib/Driver/ToolChains/Clang.cpp
===
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -5642,6 +5642,12 @@
   if (Args.hasFlag(options::OPT_fpch_instantiate_templates,
options::OPT_fno_pch_instantiate_templates, false))
 CmdArgs.push_back("-fpch-instantiate-templates");
+  if (Args.hasFlag(options::OPT_fpch_codegen, options::OPT_fno_pch_codegen,
+   false))
+CmdArgs.push_back("-fmodules-codegen");
+  if (Args.hasFlag(options::OPT_fpch_debuginfo, options::OPT_fno_pch_debuginfo,
+   false))
+CmdArgs.push_back("-fmodules-debuginfo");
 
   Args.AddLastArg(CmdArgs, options::OPT_fexperimental_new_pass_manager,
   options::OPT_fno_experimental_new_pass_manager);
Index: clang/include/clang/Driver/Options.td
===
--- clang/include/clang/Driver/Options.td
+++ clang/include/clang/Driver/Options.td
@@ -1440,6 +1440,10 @@
 def fno_pch_instantiate_templates:
   Flag <["-"], "fno-pch-instantiate-templates">,
   Group, Flags<[CC1Option]>;
+defm pch_codegen: OptInFFlag<"pch-codegen", "Generate ", "Do not generate ",
+  "code for uses of this PCH that assumes an explicit object file will be built for the PCH">;
+defm pch_debuginfo: OptInFFlag<"pch-debuginfo", "Generate ", "Do not generate ",
+  "debug info for types in an object file built from this PCH and do not generate them elsewhere">;
 
 def fmodules : Flag <["-"], "fmodules">, Group,
   Flags<[DriverOption, CC1Option]>,
Index: clang/docs/ReleaseNotes.rst
===
--- clang/docs/ReleaseNotes.rst
+++ clang/docs/ReleaseNotes.rst
@@ -63,6 +63,32 @@
 
 - ...
 
+- -fpch-codegen and -fpch-debuginfo generate shared code and/or debuginfo
+  for contents of a precompiled header in a separate object file. This object
+  file needs to be linked in, but its contents do not need to be generated
+  for other objects using the precompiled header. This should 

[PATCH] D83652: Merge some of the PCH object support with modular codegen

[Luboš Luňák via Phabricator via cfe-commits]

llunak added a comment.

> @llunak Would you be able to test this on anything you've got?

No, but thinking more about this, I think dllexport specifically voids the 
possible problems I listed. If I'm getting it right, dllexport is used only for 
code in the current library, so codegen won't create code referencing private 
symbols in other libraries, and dllexport means the code will be emitted 
anyway, so no unnecessary code generating. So I'm actually fine with the patch 
as it is.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D83652/new/

https://reviews.llvm.org/D83652



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D83652: Merge some of the PCH object support with modular codegen

[Luboš Luňák via Phabricator via cfe-commits]

llunak added a comment.

In D83652#2147539 , @dblaikie wrote:

> Not quite - it's intended to implement the D48426 
>  functionality using an implementation 
> strategy that is closer to modular code generation. Removing the need for the 
> module flag, using the MODULAR_CODEGEN_DECLS list, etc. But only putting the 
> dllexported entities in there (when using only building-pch-with-obj without 
> -fmodules-codegen).


I see. I still suggest testing this on a real codebase for the reasons outlined 
above.

> Yep, that was essentially what I was wondering - since you were proposing a 
> build mode/flags that would not be /Yc compatible, adding an explicit pch 
> build and pch object build step - was wondering if the build system support 
> for that was sufficiently available that it would be practical for users 
> (like Chromium) to migrate to that kind of model and no longer need /Yc 
> compatibility.

Not that I know for sure, but I personally doubt there's build system support 
for D83716 . The only build system I've tried 
is LibreOffice's custom gbuild, where I've added the support for 
-fpch-codegen/debuginfo myself, and even there's I'm going to stick with 
-building-pch-with-obj, since it's simpler to compile yet another .cxx file 
with an extra flag rather than have a build step with .pch as yet another kind 
of input file. But /Yc is presumably supported by pretty much anything on 
Windows.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D83652/new/

https://reviews.llvm.org/D83652



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D83622: document -fpch-instantiate-templates in release notes

[Luboš Luňák via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGd1ca9960bc19: document -fpch-instantiate-templates in 
release notes (authored by llunak).
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D83622/new/

https://reviews.llvm.org/D83622

Files:
  clang/docs/ReleaseNotes.rst


Index: clang/docs/ReleaseNotes.rst
===
--- clang/docs/ReleaseNotes.rst
+++ clang/docs/ReleaseNotes.rst
@@ -108,6 +108,13 @@
   By default, this is ~/.cache but on some platforms or installations, this
   might be elsewhere. The -fmodules-cache-path=... flag continues to work.
 
+- -fpch-instantiate-templates tries to instantiate templates already while
+  generating a precompiled header. Such templates do not need to be
+  instantiated every time the precompiled header is used, which saves compile
+  time. This may result in an error during the precompiled header generation
+  if the source header file is not self-contained. This option is enabled
+  by default for clang-cl.
+
 Deprecated Compiler Flags
 -
 


Index: clang/docs/ReleaseNotes.rst
===
--- clang/docs/ReleaseNotes.rst
+++ clang/docs/ReleaseNotes.rst
@@ -108,6 +108,13 @@
   By default, this is ~/.cache but on some platforms or installations, this
   might be elsewhere. The -fmodules-cache-path=... flag continues to work.
 
+- -fpch-instantiate-templates tries to instantiate templates already while
+  generating a precompiled header. Such templates do not need to be
+  instantiated every time the precompiled header is used, which saves compile
+  time. This may result in an error during the precompiled header generation
+  if the source header file is not self-contained. This option is enabled
+  by default for clang-cl.
+
 Deprecated Compiler Flags
 -
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D83652: Merge some of the PCH object support with modular codegen

[Luboš Luňák via Phabricator via cfe-commits]

llunak added a comment.

The patch is incomplete, isn't it? It removes DeclIsFromPCHWithObjectFile(), 
but it's still called from ASTContext::DeclMustBeEmitted(). The description 
also mentions updating of the pch-codegen test, but that's not included.

But assuming this is intended to replace the D48426 
 functionality with modular codegen, I 
mentioned already in D69778  (starting with 
'Mar 17 2020, 10:59 PM') that it's a question if this is possible. Modular 
codegen is a much bigger hammer than  D48426  
and it has also bigger possible problems: Possibly making compilation actually 
slower (even f63556d8b44b209e741833b0e6be3f8e72a1d91a mentions this), and 
possibly causing build problems because of referencing private symbols.  D48426 
 is much smaller both in gains and in problems 
(=safer).

> Do either of you know if it'd be practical to move to something more similar 
> to .pcm handling, where the pch itself is passed to the compilation, rather 
> than homed as a side effect of compiling some other source file?

Do you mean dropping compatibility with 'cl.exe /Yc'?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D83652/new/

https://reviews.llvm.org/D83652



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D69778: Make -fmodules-codegen and -fmodules-debuginfo work also with precompiled headers

[Luboš Luňák via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG31b05692cd33: make -fmodules-codegen and -fmodules-debuginfo 
work also with PCHs (authored by llunak).

Changed prior to commit:
  https://reviews.llvm.org/D69778?vs=263839=276726#toc

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D69778/new/

https://reviews.llvm.org/D69778

Files:
  clang/lib/Serialization/ASTReader.cpp
  clang/lib/Serialization/ASTReaderDecl.cpp
  clang/lib/Serialization/ASTWriter.cpp
  clang/lib/Serialization/ASTWriterDecl.cpp
  clang/test/Modules/Inputs/codegen-flags/foo.h
  clang/test/PCH/codegen.cpp

Index: clang/test/PCH/codegen.cpp
===
--- /dev/null
+++ clang/test/PCH/codegen.cpp
@@ -0,0 +1,42 @@
+// This test is the PCH version of Modules/codegen-flags.test . It uses its inputs.
+// The purpose of this test is just verify that the codegen options work with PCH as well.
+// All the codegen functionality should be tested in Modules/.
+
+// RUN: rm -rf %t
+// RUN: mkdir -p %t
+// REQUIRES: x86-registered-target
+
+// RUN: %clang_cc1 -triple=x86_64-linux-gnu -fmodules-codegen -x c++-header -building-pch-with-obj -emit-pch %S/../Modules/Inputs/codegen-flags/foo.h -o %t/foo-cg.pch
+// RUN: %clang_cc1 -triple=x86_64-linux-gnu -fmodules-debuginfo -x c++-header -building-pch-with-obj -emit-pch %S/../Modules/Inputs/codegen-flags/foo.h -o %t/foo-di.pch
+
+// RUN: %clang_cc1 -triple x86_64-linux-gnu -emit-llvm -debug-info-kind=limited -o - %s -include-pch %t/foo-cg.pch -building-pch-with-obj -fmodules-codegen | FileCheck --check-prefix=CG %s
+// RUN: %clang_cc1 -triple x86_64-linux-gnu -emit-llvm -debug-info-kind=limited -o - %s -include-pch %t/foo-di.pch -building-pch-with-obj -fmodules-debuginfo | FileCheck --check-prefix=DI %s
+
+// RUN: %clang_cc1 -triple x86_64-linux-gnu -emit-llvm -debug-info-kind=limited -o - -include-pch %t/foo-cg.pch %S/../Modules/Inputs/codegen-flags/use.cpp | FileCheck --check-prefix=CG-USE %s
+// RUN: %clang_cc1 -triple x86_64-linux-gnu -emit-llvm -debug-info-kind=limited -o - -include-pch %t/foo-di.pch %S/../Modules/Inputs/codegen-flags/use.cpp | FileCheck --check-prefix=DI-USE %s
+
+// Test with template instantiation in the pch.
+
+// RUN: %clang_cc1 -triple=x86_64-linux-gnu -fmodules-codegen -x c++-header -building-pch-with-obj -emit-pch -fpch-instantiate-templates %S/../Modules/Inputs/codegen-flags/foo.h -o %t/foo-cg.pch
+// RUN: %clang_cc1 -triple=x86_64-linux-gnu -fmodules-debuginfo -x c++-header -building-pch-with-obj -emit-pch -fpch-instantiate-templates %S/../Modules/Inputs/codegen-flags/foo.h -o %t/foo-di.pch
+
+// RUN: %clang_cc1 -triple x86_64-linux-gnu -emit-llvm -debug-info-kind=limited -o - %s -include-pch %t/foo-cg.pch -building-pch-with-obj -fmodules-codegen | FileCheck --check-prefix=CG %s
+// RUN: %clang_cc1 -triple x86_64-linux-gnu -emit-llvm -debug-info-kind=limited -o - %s -include-pch %t/foo-di.pch -building-pch-with-obj -fmodules-debuginfo | FileCheck --check-prefix=DI %s
+
+// RUN: %clang_cc1 -triple x86_64-linux-gnu -emit-llvm -debug-info-kind=limited -o - -include-pch %t/foo-cg.pch %S/../Modules/Inputs/codegen-flags/use.cpp | FileCheck --check-prefix=CG-USE %s
+// RUN: %clang_cc1 -triple x86_64-linux-gnu -emit-llvm -debug-info-kind=limited -o - -include-pch %t/foo-di.pch %S/../Modules/Inputs/codegen-flags/use.cpp | FileCheck --check-prefix=DI-USE %s
+
+
+// CG: define weak_odr void @_Z2f1v
+// CG: DICompileUnit
+// CG-NOT: DICompositeType
+
+// CG-USE: declare void @_Z2f1v
+// CG-USE: DICompileUnit
+// CG-USE: distinct !DICompositeType(tag: DW_TAG_structure_type, name: "foo"
+
+// DI-NOT: define
+// DI: distinct !DICompositeType(tag: DW_TAG_structure_type, name: "foo"
+
+// DI-USE: define linkonce_odr void @_Z2f1v
+// DI-USE: = !DICompositeType(tag: DW_TAG_structure_type, name: "foo", {{.*}}, flags: DIFlagFwdDecl
Index: clang/test/Modules/Inputs/codegen-flags/foo.h
===
--- clang/test/Modules/Inputs/codegen-flags/foo.h
+++ clang/test/Modules/Inputs/codegen-flags/foo.h
@@ -1,4 +1,7 @@
+#ifndef FOO_H
+#define FOO_H
 struct foo {
 };
 inline void f1() {
 }
+#endif
Index: clang/lib/Serialization/ASTWriterDecl.cpp
===
--- clang/lib/Serialization/ASTWriterDecl.cpp
+++ clang/lib/Serialization/ASTWriterDecl.cpp
@@ -2458,9 +2458,10 @@
 
   assert(FD->doesThisDeclarationHaveABody());
   bool ModulesCodegen = false;
-  if (Writer->WritingModule && !FD->isDependentContext()) {
+  if (!FD->isDependentContext()) {
 Optional Linkage;
-if (Writer->WritingModule->Kind == Module::ModuleInterfaceUnit) {
+if (Writer->WritingModule &&
+Writer->WritingModule->Kind == Module::ModuleInterfaceUnit) {
   // When building a C++ Modules TS module interface unit, a strong
   // definition in the module 

[PATCH] D69778: Make -fmodules-codegen and -fmodules-debuginfo work also with precompiled headers

[Luboš Luňák via Phabricator via cfe-commits]

llunak added a comment.

Ping ...


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D69778/new/

https://reviews.llvm.org/D69778



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D69778: Make -fmodules-codegen and -fmodules-debuginfo work also with precompiled headers

[Luboš Luňák via Phabricator via cfe-commits]

llunak added a comment.

Ping.


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D69778/new/

https://reviews.llvm.org/D69778



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D69585: Add option to instantiate templates already in the PCH

[Luboš Luňák via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGa45f713c6730: add option to instantiate templates already in 
the PCH (authored by llunak).

Changed prior to commit:
  https://reviews.llvm.org/D69585?vs=265918=272312#toc

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D69585/new/

https://reviews.llvm.org/D69585

Files:
  clang/include/clang/Basic/LangOptions.def
  clang/include/clang/Driver/Options.td
  clang/include/clang/Sema/Sema.h
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/lib/Sema/Sema.cpp
  clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
  clang/test/PCH/crash-12631281.cpp
  clang/test/PCH/cxx-alias-decl.cpp
  clang/test/PCH/cxx-dependent-sized-ext-vector.cpp
  clang/test/PCH/cxx-explicit-specifier.cpp
  clang/test/PCH/cxx-exprs.cpp
  clang/test/PCH/cxx-friends.cpp
  clang/test/PCH/cxx-member-init.cpp
  clang/test/PCH/cxx-ms-function-specialization-class-scope.cpp
  clang/test/PCH/cxx-static_assert.cpp
  clang/test/PCH/cxx-templates.cpp
  clang/test/PCH/cxx-variadic-templates-with-default-params.cpp
  clang/test/PCH/cxx-variadic-templates.cpp
  clang/test/PCH/cxx0x-default-delete.cpp
  clang/test/PCH/cxx11-constexpr.cpp
  clang/test/PCH/cxx11-enum-template.cpp
  clang/test/PCH/cxx11-exception-spec.cpp
  clang/test/PCH/cxx11-inheriting-ctors.cpp
  clang/test/PCH/cxx11-user-defined-literals.cpp
  clang/test/PCH/cxx1y-decltype-auto.cpp
  clang/test/PCH/cxx1y-deduced-return-type.cpp
  clang/test/PCH/cxx1y-default-initializer.cpp
  clang/test/PCH/cxx1y-init-captures.cpp
  clang/test/PCH/cxx1y-variable-templates.cpp
  clang/test/PCH/cxx1z-aligned-alloc.cpp
  clang/test/PCH/cxx1z-decomposition.cpp
  clang/test/PCH/cxx1z-using-declaration.cpp
  clang/test/PCH/cxx2a-bitfield-init.cpp
  clang/test/PCH/cxx2a-concept-specialization-expr.cpp
  clang/test/PCH/cxx2a-constraints.cpp
  clang/test/PCH/cxx2a-defaulted-comparison.cpp
  clang/test/PCH/cxx2a-requires-expr.cpp
  clang/test/PCH/cxx2a-template-lambdas.cpp
  clang/test/PCH/delayed-pch-instantiate.cpp
  clang/test/PCH/friend-template.cpp
  clang/test/PCH/implicitly-deleted.cpp
  clang/test/PCH/late-parsed-instantiations.cpp
  clang/test/PCH/local_static.cpp
  clang/test/PCH/macro-undef.cpp
  clang/test/PCH/make-integer-seq.cpp
  clang/test/PCH/ms-if-exists.cpp
  clang/test/PCH/pch-instantiate-templates-forward-decl.cpp
  clang/test/PCH/pch-instantiate-templates.cpp
  clang/test/PCH/pr18806.cpp
  clang/test/PCH/pragma-diag-section.cpp
  clang/test/PCH/rdar10830559.cpp
  clang/test/PCH/specialization-after-instantiation.cpp
  clang/test/PCH/type_pack_element.cpp

Index: clang/test/PCH/type_pack_element.cpp
===
--- clang/test/PCH/type_pack_element.cpp
+++ clang/test/PCH/type_pack_element.cpp
@@ -1,6 +1,9 @@
 // RUN: %clang_cc1 -std=c++14 -x c++-header %s -emit-pch -o %t.pch
 // RUN: %clang_cc1 -std=c++14 -x c++ /dev/null -include-pch %t.pch
 
+// RUN: %clang_cc1 -std=c++14 -x c++-header %s -emit-pch -fpch-instantiate-templates -o %t.pch
+// RUN: %clang_cc1 -std=c++14 -x c++ /dev/null -include-pch %t.pch
+
 template 
 struct X { };
 
Index: clang/test/PCH/specialization-after-instantiation.cpp
===
--- /dev/null
+++ clang/test/PCH/specialization-after-instantiation.cpp
@@ -0,0 +1,32 @@
+// Test this without pch.
+// RUN: %clang_cc1 -fsyntax-only -verify -DBODY %s
+
+// Test with pch.
+// RUN: %clang_cc1 -emit-pch -o %t %s
+// RUN: %clang_cc1 -include-pch %t -fsyntax-only -verify -DBODY %s
+
+// RUN: %clang_cc1 -emit-pch -fpch-instantiate-templates -o %t %s
+// RUN: %clang_cc1 -include-pch %t -fsyntax-only -verify -DBODY %s
+
+#ifndef HEADER_H
+#define HEADER_H
+
+template 
+struct A {
+  int foo() const;
+};
+
+int bar(A *a) {
+  return a->foo();
+}
+
+#endif // HEADER_H
+
+#ifdef BODY
+
+template <>
+int A::foo() const { // expected-error {{explicit specialization of 'foo' after instantiation}}  // expected-note@20 {{implicit instantiation first required here}}
+  return 10;
+}
+
+#endif // BODY
Index: clang/test/PCH/rdar10830559.cpp
===
--- clang/test/PCH/rdar10830559.cpp
+++ clang/test/PCH/rdar10830559.cpp
@@ -6,6 +6,9 @@
 // RUN: %clang_cc1 -emit-pch -o %t %s
 // RUN: %clang_cc1 -include-pch %t -emit-llvm-only %t.empty.cpp 
 
+// RUN: %clang_cc1 -emit-pch -fpch-instantiate-templates -o %t %s
+// RUN: %clang_cc1 -include-pch %t -emit-llvm-only %t.empty.cpp
+
 // rdar://10830559
 
 //#pragma ms_struct on
Index: clang/test/PCH/pragma-diag-section.cpp
===
--- clang/test/PCH/pragma-diag-section.cpp
+++ clang/test/PCH/pragma-diag-section.cpp
@@ -5,6 +5,9 @@
 // RUN: %clang_cc1 %s -emit-pch -o %t
 // RUN: %clang_cc1 %s -include-pch %t -verify -fsyntax-only -Wuninitialized
 
+// 

[PATCH] D69778: Make -fmodules-codegen and -fmodules-debuginfo work also with precompiled headers

[Luboš Luňák via Phabricator via cfe-commits]

llunak added a comment.

So what more needs to be done here to get this change accepted? It's already 
missed 10.0 and 11.1 is getting branched off in about 3 weeks.

- The change was already accepted once and the problem leading to its revert 
has already been fixed.
- It just enables for PCH something that's been enabled for modules for a long 
time (so whatever may be wrong with it will probably be wrong with modules as 
well).
- I've been using the feature for 8 months.
- There's no publicly reported problem with it (I know aganea reported them 
above, but they are private, so nobody can do anything about them).
- The feature is opt-in and disabled by default.


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D69778/new/

https://reviews.llvm.org/D69778



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D69585: Add option to instantiate templates already in the PCH

[Luboš Luňák via Phabricator via cfe-commits]

llunak updated this revision to Diff 265918.
llunak edited the summary of this revision.
llunak added a comment.

Enabled the option by default for clang-cl to match MSVC.


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D69585/new/

https://reviews.llvm.org/D69585

Files:
  clang/include/clang/Basic/LangOptions.def
  clang/include/clang/Driver/Options.td
  clang/include/clang/Sema/Sema.h
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/lib/Sema/Sema.cpp
  clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
  clang/test/PCH/codegen.cpp
  clang/test/PCH/crash-12631281.cpp
  clang/test/PCH/cxx-alias-decl.cpp
  clang/test/PCH/cxx-dependent-sized-ext-vector.cpp
  clang/test/PCH/cxx-explicit-specifier.cpp
  clang/test/PCH/cxx-exprs.cpp
  clang/test/PCH/cxx-friends.cpp
  clang/test/PCH/cxx-member-init.cpp
  clang/test/PCH/cxx-ms-function-specialization-class-scope.cpp
  clang/test/PCH/cxx-static_assert.cpp
  clang/test/PCH/cxx-templates.cpp
  clang/test/PCH/cxx-variadic-templates-with-default-params.cpp
  clang/test/PCH/cxx-variadic-templates.cpp
  clang/test/PCH/cxx0x-default-delete.cpp
  clang/test/PCH/cxx11-constexpr.cpp
  clang/test/PCH/cxx11-enum-template.cpp
  clang/test/PCH/cxx11-exception-spec.cpp
  clang/test/PCH/cxx11-inheriting-ctors.cpp
  clang/test/PCH/cxx11-user-defined-literals.cpp
  clang/test/PCH/cxx1y-decltype-auto.cpp
  clang/test/PCH/cxx1y-deduced-return-type.cpp
  clang/test/PCH/cxx1y-default-initializer.cpp
  clang/test/PCH/cxx1y-init-captures.cpp
  clang/test/PCH/cxx1y-variable-templates.cpp
  clang/test/PCH/cxx1z-aligned-alloc.cpp
  clang/test/PCH/cxx1z-decomposition.cpp
  clang/test/PCH/cxx1z-using-declaration.cpp
  clang/test/PCH/cxx2a-bitfield-init.cpp
  clang/test/PCH/cxx2a-concept-specialization-expr.cpp
  clang/test/PCH/cxx2a-constraints.cpp
  clang/test/PCH/cxx2a-defaulted-comparison.cpp
  clang/test/PCH/cxx2a-requires-expr.cpp
  clang/test/PCH/cxx2a-template-lambdas.cpp
  clang/test/PCH/delayed-pch-instantiate.cpp
  clang/test/PCH/friend-template.cpp
  clang/test/PCH/implicitly-deleted.cpp
  clang/test/PCH/late-parsed-instantiations.cpp
  clang/test/PCH/local_static.cpp
  clang/test/PCH/macro-undef.cpp
  clang/test/PCH/make-integer-seq.cpp
  clang/test/PCH/ms-if-exists.cpp
  clang/test/PCH/pch-instantiate-templates-forward-decl.cpp
  clang/test/PCH/pch-instantiate-templates.cpp
  clang/test/PCH/pr18806.cpp
  clang/test/PCH/pragma-diag-section.cpp
  clang/test/PCH/rdar10830559.cpp
  clang/test/PCH/specialization-after-instantiation.cpp
  clang/test/PCH/type_pack_element.cpp

Index: clang/test/PCH/type_pack_element.cpp
===
--- clang/test/PCH/type_pack_element.cpp
+++ clang/test/PCH/type_pack_element.cpp
@@ -1,6 +1,9 @@
 // RUN: %clang_cc1 -std=c++14 -x c++-header %s -emit-pch -o %t.pch
 // RUN: %clang_cc1 -std=c++14 -x c++ /dev/null -include-pch %t.pch
 
+// RUN: %clang_cc1 -std=c++14 -x c++-header %s -emit-pch -fpch-instantiate-templates -o %t.pch
+// RUN: %clang_cc1 -std=c++14 -x c++ /dev/null -include-pch %t.pch
+
 template 
 struct X { };
 
Index: clang/test/PCH/specialization-after-instantiation.cpp
===
--- /dev/null
+++ clang/test/PCH/specialization-after-instantiation.cpp
@@ -0,0 +1,32 @@
+// Test this without pch.
+// RUN: %clang_cc1 -fsyntax-only -verify -DBODY %s
+
+// Test with pch.
+// RUN: %clang_cc1 -emit-pch -o %t %s
+// RUN: %clang_cc1 -include-pch %t -fsyntax-only -verify -DBODY %s
+
+// RUN: %clang_cc1 -emit-pch -fpch-instantiate-templates -o %t %s
+// RUN: %clang_cc1 -include-pch %t -fsyntax-only -verify -DBODY %s
+
+#ifndef HEADER_H
+#define HEADER_H
+
+template 
+struct A {
+  int foo() const;
+};
+
+int bar(A *a) {
+  return a->foo();
+}
+
+#endif // HEADER_H
+
+#ifdef BODY
+
+template <>
+int A::foo() const { // expected-error {{explicit specialization of 'foo' after instantiation}}  // expected-note@20 {{implicit instantiation first required here}}
+  return 10;
+}
+
+#endif // BODY
Index: clang/test/PCH/rdar10830559.cpp
===
--- clang/test/PCH/rdar10830559.cpp
+++ clang/test/PCH/rdar10830559.cpp
@@ -6,6 +6,9 @@
 // RUN: %clang_cc1 -emit-pch -o %t %s
 // RUN: %clang_cc1 -include-pch %t -emit-llvm-only %t.empty.cpp 
 
+// RUN: %clang_cc1 -emit-pch -fpch-instantiate-templates -o %t %s
+// RUN: %clang_cc1 -include-pch %t -emit-llvm-only %t.empty.cpp
+
 // rdar://10830559
 
 //#pragma ms_struct on
Index: clang/test/PCH/pragma-diag-section.cpp
===
--- clang/test/PCH/pragma-diag-section.cpp
+++ clang/test/PCH/pragma-diag-section.cpp
@@ -5,6 +5,9 @@
 // RUN: %clang_cc1 %s -emit-pch -o %t
 // RUN: %clang_cc1 %s -include-pch %t -verify -fsyntax-only -Wuninitialized
 
+// RUN: %clang_cc1 %s -emit-pch -fpch-instantiate-templates -o %t
+// RUN: 

[PATCH] D69585: Add option to instantiate templates already in the PCH

[Luboš Luňák via Phabricator via cfe-commits]

llunak marked an inline comment as done.
llunak added inline comments.



Comment at: clang/lib/Driver/ToolChains/Clang.cpp:5610
+  if (Args.hasFlag(options::OPT_fpch_instantiate_templates,
+   options::OPT_fno_pch_instantiate_templates, false))
+CmdArgs.push_back("-fpch-instantiate-templates");

rnk wrote:
> llunak wrote:
> > rnk wrote:
> > > Does MSVC default to this behavior? Should this default to true with 
> > > clang-cl /Yu / /Yc? This can be future work and does not need to be part 
> > > of this patch.
> > Since MSVC is noticeably faster for an equivalent PCH compile than current 
> > Clang, presumably it instantiates templates already in the PCH. But that 
> > doesn't really matter for this patch, if it were ok to enable this by 
> > default for clang-cl, than it would be ok also for clang itself. That 
> > cannot be done now though, https://reviews.llvm.org/D69585#1946765 points 
> > out a corner case where this change makes a valid compilation error out, 
> > and that's the reason for having this behind a flag. I expect Clang could 
> > possibly be adjusted to bail out and delay template instantiantion in such 
> > a case until it can be performed successfully, but given the response rate 
> > to my PCH patches I first wanted to get the feature in somehow, and I can 
> > try to make the flag default/irrelevant later.
> > 
> Right, I guess what I mean is, for that example where instantiation at the 
> end of the PCH creates an error, does MSVC emit an error? I just checked, and 
> it looks like the answer is yes, so it seems like we could make this the 
> default behavior for `clang-cl /Yc` without much further discussion.
That's a good point. Yes, since MSVC creates PCHs by basically compiling an 
empty .cpp, it apparently does instantiate templates as a side-effect of that, 
and https://reviews.llvm.org/D69585#1946765 indeed doesn't work with MSVC. So 
no harm in enabling the option for clang-cl.

I'll update the patch.



Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D69585/new/

https://reviews.llvm.org/D69585



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D69585: Add option to instantiate templates already in the PCH

[Luboš Luňák via Phabricator via cfe-commits]

llunak marked an inline comment as done.
llunak added inline comments.



Comment at: clang/lib/Driver/ToolChains/Clang.cpp:5610
+  if (Args.hasFlag(options::OPT_fpch_instantiate_templates,
+   options::OPT_fno_pch_instantiate_templates, false))
+CmdArgs.push_back("-fpch-instantiate-templates");

rnk wrote:
> Does MSVC default to this behavior? Should this default to true with clang-cl 
> /Yu / /Yc? This can be future work and does not need to be part of this patch.
Since MSVC is noticeably faster for an equivalent PCH compile than current 
Clang, presumably it instantiates templates already in the PCH. But that 
doesn't really matter for this patch, if it were ok to enable this by default 
for clang-cl, than it would be ok also for clang itself. That cannot be done 
now though, https://reviews.llvm.org/D69585#1946765 points out a corner case 
where this change makes a valid compilation error out, and that's the reason 
for having this behind a flag. I expect Clang could possibly be adjusted to 
bail out and delay template instantiantion in such a case until it can be 
performed successfully, but given the response rate to my PCH patches I first 
wanted to get the feature in somehow, and I can try to make the flag 
default/irrelevant later.



Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D69585/new/

https://reviews.llvm.org/D69585



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D69778: Make -fmodules-codegen and -fmodules-debuginfo work also with precompiled headers

[Luboš Luňák via Phabricator via cfe-commits]

llunak added a subscriber: aganea.
llunak added a comment.

In D69778#2035318 , @dblaikie wrote:

> Do you have a sense of the larger testing that PR44953 was reduced from? Have 
> you tried compiling a non-trivial codebase (I assume you might've tested it 
> with Open Office, for instance) - wonder why PR44953 didn't show up there? 
> (perhaps just the construct needed to tickle the issue isn't common/isn't 
> common in Open Office code, etc)


I've been building LibreOffice and some of its 3rd-party dependencies such as 
Skia for months with this patch, without problems. PR44953 seems to be a corner 
case, we do not use D3DX12 headers in LO, and (I presume) __declspec(selectany) 
is a niche feature. And while I do not understand why PR44953 was happening, I 
do understand why my original patch was triggering it, and it will not happen 
with this patch (or, if it will happen, then it'll happen with modules as 
well). And I've tested with the testcase in PR44953.

(It's LibreOffice BTW, OpenOffice is anything but dead.)

> and/or maybe see if the person who filed PR44953 might be able to test this 
> version of the patch on the original codebase that bug was reduced from?

@aganea Have you tried how this version of the patch affects PR44953? If not, 
could you please try?


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D69778/new/

https://reviews.llvm.org/D69778



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D69778: Make -fmodules-codegen and -fmodules-debuginfo work also with precompiled headers

[Luboš Luňák via Phabricator via cfe-commits]

llunak updated this revision to Diff 263839.
llunak edited the summary of this revision.

Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D69778/new/

https://reviews.llvm.org/D69778

Files:
  clang/lib/Serialization/ASTReader.cpp
  clang/lib/Serialization/ASTReaderDecl.cpp
  clang/lib/Serialization/ASTWriter.cpp
  clang/lib/Serialization/ASTWriterDecl.cpp
  clang/test/Modules/Inputs/codegen-flags/foo.h
  clang/test/PCH/codegen.cpp

Index: clang/test/PCH/codegen.cpp
===
--- /dev/null
+++ clang/test/PCH/codegen.cpp
@@ -0,0 +1,30 @@
+// This test is the PCH version of Modules/codegen-flags.test . It uses its inputs.
+// The purpose of this test is just verify that the codegen options work with PCH as well.
+// All the codegen functionality should be tested in Modules/.
+
+// RUN: rm -rf %t
+// RUN: mkdir -p %t
+// REQUIRES: x86-registered-target
+
+// RUN: %clang_cc1 -triple=x86_64-linux-gnu -fmodules-codegen -x c++-header -building-pch-with-obj -emit-pch %S/../Modules/Inputs/codegen-flags/foo.h -o %t/foo-cg.pch
+// RUN: %clang_cc1 -triple=x86_64-linux-gnu -fmodules-debuginfo -x c++-header -building-pch-with-obj -emit-pch %S/../Modules/Inputs/codegen-flags/foo.h -o %t/foo-di.pch
+
+// RUN: %clang_cc1 -triple x86_64-linux-gnu -emit-llvm -debug-info-kind=limited -o - %s -include-pch %t/foo-cg.pch -building-pch-with-obj -fmodules-codegen | FileCheck --check-prefix=CG %s
+// RUN: %clang_cc1 -triple x86_64-linux-gnu -emit-llvm -debug-info-kind=limited -o - %s -include-pch %t/foo-di.pch -building-pch-with-obj -fmodules-debuginfo | FileCheck --check-prefix=DI %s
+
+// RUN: %clang_cc1 -triple x86_64-linux-gnu -emit-llvm -debug-info-kind=limited -o - -include-pch %t/foo-cg.pch %S/../Modules/Inputs/codegen-flags/use.cpp | FileCheck --check-prefix=CG-USE %s
+// RUN: %clang_cc1 -triple x86_64-linux-gnu -emit-llvm -debug-info-kind=limited -o - -include-pch %t/foo-di.pch %S/../Modules/Inputs/codegen-flags/use.cpp | FileCheck --check-prefix=DI-USE %s
+
+// CG: define weak_odr void @_Z2f1v
+// CG: DICompileUnit
+// CG-NOT: DICompositeType
+
+// CG-USE: declare void @_Z2f1v
+// CG-USE: DICompileUnit
+// CG-USE: distinct !DICompositeType(tag: DW_TAG_structure_type, name: "foo"
+
+// DI-NOT: define
+// DI: distinct !DICompositeType(tag: DW_TAG_structure_type, name: "foo"
+
+// DI-USE: define linkonce_odr void @_Z2f1v
+// DI-USE: = !DICompositeType(tag: DW_TAG_structure_type, name: "foo", {{.*}}, flags: DIFlagFwdDecl
Index: clang/test/Modules/Inputs/codegen-flags/foo.h
===
--- clang/test/Modules/Inputs/codegen-flags/foo.h
+++ clang/test/Modules/Inputs/codegen-flags/foo.h
@@ -1,4 +1,7 @@
+#ifndef FOO_H
+#define FOO_H
 struct foo {
 };
 inline void f1() {
 }
+#endif
Index: clang/lib/Serialization/ASTWriterDecl.cpp
===
--- clang/lib/Serialization/ASTWriterDecl.cpp
+++ clang/lib/Serialization/ASTWriterDecl.cpp
@@ -2460,9 +2460,10 @@
 
   assert(FD->doesThisDeclarationHaveABody());
   bool ModulesCodegen = false;
-  if (Writer->WritingModule && !FD->isDependentContext()) {
+  if (!FD->isDependentContext()) {
 Optional Linkage;
-if (Writer->WritingModule->Kind == Module::ModuleInterfaceUnit) {
+if (Writer->WritingModule &&
+Writer->WritingModule->Kind == Module::ModuleInterfaceUnit) {
   // When building a C++ Modules TS module interface unit, a strong
   // definition in the module interface is provided by the compilation of
   // that module interface unit, not by its users. (Inline functions are
Index: clang/lib/Serialization/ASTWriter.cpp
===
--- clang/lib/Serialization/ASTWriter.cpp
+++ clang/lib/Serialization/ASTWriter.cpp
@@ -5631,8 +5631,8 @@
 
   // getODRHash will compute the ODRHash if it has not been previously computed.
   Record->push_back(D->getODRHash());
-  bool ModulesDebugInfo = Writer->Context->getLangOpts().ModulesDebugInfo &&
-  Writer->WritingModule && !D->isDependentType();
+  bool ModulesDebugInfo =
+  Writer->Context->getLangOpts().ModulesDebugInfo && !D->isDependentType();
   Record->push_back(ModulesDebugInfo);
   if (ModulesDebugInfo)
 Writer->ModularCodegenDecls.push_back(Writer->GetDeclRef(D));
Index: clang/lib/Serialization/ASTReaderDecl.cpp
===
--- clang/lib/Serialization/ASTReaderDecl.cpp
+++ clang/lib/Serialization/ASTReaderDecl.cpp
@@ -503,8 +503,12 @@
 }
 
 void ASTDeclReader::ReadFunctionDefinition(FunctionDecl *FD) {
-  if (Record.readInt())
+  if (Record.readInt()) {
 Reader.DefinitionSource[FD] = Loc.F->Kind == ModuleKind::MK_MainFile;
+if (Reader.getContext().getLangOpts().BuildingPCHWithObjectFile &&
+Reader.DeclIsFromPCHWithObjectFile(FD))
+  

[PATCH] D69778: Make -fmodules-codegen and -fmodules-debuginfo work also with precompiled headers

[Luboš Luňák via Phabricator via cfe-commits]

llunak added a comment.

In D69778#2032363 , @dblaikie wrote:

> So the original commit ( cbc9d22e49b4 
>  ) was 
> reverted at some point, and now you're proposing recommitting it with a 
> slight change?


Yes.

> Could you summarize (in the summary (which should hopefully be included in 
> the commit message eventually)) the commit (include the hash), the revert 
> (including the hash), reasons for revert and what's changed in this patch 
> compared to the original commit that addresses the reasons for reverting?

Done and see D74846  for details and the exact 
change.

(& ideally what extra testing was done on the newer version of the patch to 
ensure the original issue or another like it would now be caught before 
committing)

There's no testing besides checking that PR44953 no longer crashes. As said in 
D74846 , I don't see how to do a test for 
this, and if there's a problem with this patch then the same problem should 
also exist with modules.


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D69778/new/

https://reviews.llvm.org/D69778



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D69778: Make -fmodules-codegen and -fmodules-debuginfo work also with precompiled headers

[Luboš Luňák via Phabricator via cfe-commits]

llunak updated this revision to Diff 263838.
llunak edited the summary of this revision.
llunak added a comment.

Updated commit message.


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D69778/new/

https://reviews.llvm.org/D69778

Files:
  clang/lib/Serialization/ASTReader.cpp
  clang/lib/Serialization/ASTReaderDecl.cpp
  clang/lib/Serialization/ASTWriter.cpp
  clang/lib/Serialization/ASTWriterDecl.cpp
  clang/test/Modules/Inputs/codegen-flags/foo.h
  clang/test/PCH/codegen.cpp

Index: clang/test/PCH/codegen.cpp
===
--- /dev/null
+++ clang/test/PCH/codegen.cpp
@@ -0,0 +1,30 @@
+// This test is the PCH version of Modules/codegen-flags.test . It uses its inputs.
+// The purpose of this test is just verify that the codegen options work with PCH as well.
+// All the codegen functionality should be tested in Modules/.
+
+// RUN: rm -rf %t
+// RUN: mkdir -p %t
+// REQUIRES: x86-registered-target
+
+// RUN: %clang_cc1 -triple=x86_64-linux-gnu -fmodules-codegen -x c++-header -building-pch-with-obj -emit-pch %S/../Modules/Inputs/codegen-flags/foo.h -o %t/foo-cg.pch
+// RUN: %clang_cc1 -triple=x86_64-linux-gnu -fmodules-debuginfo -x c++-header -building-pch-with-obj -emit-pch %S/../Modules/Inputs/codegen-flags/foo.h -o %t/foo-di.pch
+
+// RUN: %clang_cc1 -triple x86_64-linux-gnu -emit-llvm -debug-info-kind=limited -o - %s -include-pch %t/foo-cg.pch -building-pch-with-obj -fmodules-codegen | FileCheck --check-prefix=CG %s
+// RUN: %clang_cc1 -triple x86_64-linux-gnu -emit-llvm -debug-info-kind=limited -o - %s -include-pch %t/foo-di.pch -building-pch-with-obj -fmodules-debuginfo | FileCheck --check-prefix=DI %s
+
+// RUN: %clang_cc1 -triple x86_64-linux-gnu -emit-llvm -debug-info-kind=limited -o - -include-pch %t/foo-cg.pch %S/../Modules/Inputs/codegen-flags/use.cpp | FileCheck --check-prefix=CG-USE %s
+// RUN: %clang_cc1 -triple x86_64-linux-gnu -emit-llvm -debug-info-kind=limited -o - -include-pch %t/foo-di.pch %S/../Modules/Inputs/codegen-flags/use.cpp | FileCheck --check-prefix=DI-USE %s
+
+// CG: define weak_odr void @_Z2f1v
+// CG: DICompileUnit
+// CG-NOT: DICompositeType
+
+// CG-USE: declare void @_Z2f1v
+// CG-USE: DICompileUnit
+// CG-USE: distinct !DICompositeType(tag: DW_TAG_structure_type, name: "foo"
+
+// DI-NOT: define
+// DI: distinct !DICompositeType(tag: DW_TAG_structure_type, name: "foo"
+
+// DI-USE: define linkonce_odr void @_Z2f1v
+// DI-USE: = !DICompositeType(tag: DW_TAG_structure_type, name: "foo", {{.*}}, flags: DIFlagFwdDecl
Index: clang/test/Modules/Inputs/codegen-flags/foo.h
===
--- clang/test/Modules/Inputs/codegen-flags/foo.h
+++ clang/test/Modules/Inputs/codegen-flags/foo.h
@@ -1,4 +1,7 @@
+#ifndef FOO_H
+#define FOO_H
 struct foo {
 };
 inline void f1() {
 }
+#endif
Index: clang/lib/Serialization/ASTWriterDecl.cpp
===
--- clang/lib/Serialization/ASTWriterDecl.cpp
+++ clang/lib/Serialization/ASTWriterDecl.cpp
@@ -2460,9 +2460,10 @@
 
   assert(FD->doesThisDeclarationHaveABody());
   bool ModulesCodegen = false;
-  if (Writer->WritingModule && !FD->isDependentContext()) {
+  if (!FD->isDependentContext()) {
 Optional Linkage;
-if (Writer->WritingModule->Kind == Module::ModuleInterfaceUnit) {
+if (Writer->WritingModule &&
+Writer->WritingModule->Kind == Module::ModuleInterfaceUnit) {
   // When building a C++ Modules TS module interface unit, a strong
   // definition in the module interface is provided by the compilation of
   // that module interface unit, not by its users. (Inline functions are
Index: clang/lib/Serialization/ASTWriter.cpp
===
--- clang/lib/Serialization/ASTWriter.cpp
+++ clang/lib/Serialization/ASTWriter.cpp
@@ -5631,8 +5631,8 @@
 
   // getODRHash will compute the ODRHash if it has not been previously computed.
   Record->push_back(D->getODRHash());
-  bool ModulesDebugInfo = Writer->Context->getLangOpts().ModulesDebugInfo &&
-  Writer->WritingModule && !D->isDependentType();
+  bool ModulesDebugInfo =
+  Writer->Context->getLangOpts().ModulesDebugInfo && !D->isDependentType();
   Record->push_back(ModulesDebugInfo);
   if (ModulesDebugInfo)
 Writer->ModularCodegenDecls.push_back(Writer->GetDeclRef(D));
Index: clang/lib/Serialization/ASTReaderDecl.cpp
===
--- clang/lib/Serialization/ASTReaderDecl.cpp
+++ clang/lib/Serialization/ASTReaderDecl.cpp
@@ -503,8 +503,12 @@
 }
 
 void ASTDeclReader::ReadFunctionDefinition(FunctionDecl *FD) {
-  if (Record.readInt())
+  if (Record.readInt()) {
 Reader.DefinitionSource[FD] = Loc.F->Kind == ModuleKind::MK_MainFile;
+if (Reader.getContext().getLangOpts().BuildingPCHWithObjectFile &&
+

[PATCH] D69585: Add option to instantiate templates already in the PCH

[Luboš Luňák via Phabricator via cfe-commits]

llunak added a comment.

Ping.


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D69585/new/

https://reviews.llvm.org/D69585



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D69778: Make -fmodules-codegen and -fmodules-debuginfo work also with precompiled headers

[Luboš Luňák via Phabricator via cfe-commits]

llunak added a comment.

Ping? I've reopened this one as suggested in D74846 
, but apparently it's kept the accepted state, 
so I'm not sure if this needs another approval or what to do here.


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D69778/new/

https://reviews.llvm.org/D69778



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D69778: Make -fmodules-codegen and -fmodules-debuginfo work also with precompiled headers

[Luboš Luňák via Phabricator via cfe-commits]

llunak updated this revision to Diff 261722.
llunak added a comment.

Reopening because of https://reviews.llvm.org/D74846, updated version includes 
the partial revert from there.


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D69778/new/

https://reviews.llvm.org/D69778

Files:
  clang/lib/Serialization/ASTReader.cpp
  clang/lib/Serialization/ASTReaderDecl.cpp
  clang/lib/Serialization/ASTWriter.cpp
  clang/lib/Serialization/ASTWriterDecl.cpp
  clang/test/Modules/Inputs/codegen-flags/foo.h
  clang/test/PCH/codegen.cpp

Index: clang/test/PCH/codegen.cpp
===
--- /dev/null
+++ clang/test/PCH/codegen.cpp
@@ -0,0 +1,30 @@
+// This test is the PCH version of Modules/codegen-flags.test . It uses its inputs.
+// The purpose of this test is just verify that the codegen options work with PCH as well.
+// All the codegen functionality should be tested in Modules/.
+
+// RUN: rm -rf %t
+// RUN: mkdir -p %t
+// REQUIRES: x86-registered-target
+
+// RUN: %clang_cc1 -triple=x86_64-linux-gnu -fmodules-codegen -x c++-header -building-pch-with-obj -emit-pch %S/../Modules/Inputs/codegen-flags/foo.h -o %t/foo-cg.pch
+// RUN: %clang_cc1 -triple=x86_64-linux-gnu -fmodules-debuginfo -x c++-header -building-pch-with-obj -emit-pch %S/../Modules/Inputs/codegen-flags/foo.h -o %t/foo-di.pch
+
+// RUN: %clang_cc1 -triple x86_64-linux-gnu -emit-llvm -debug-info-kind=limited -o - %s -include-pch %t/foo-cg.pch -building-pch-with-obj -fmodules-codegen | FileCheck --check-prefix=CG %s
+// RUN: %clang_cc1 -triple x86_64-linux-gnu -emit-llvm -debug-info-kind=limited -o - %s -include-pch %t/foo-di.pch -building-pch-with-obj -fmodules-debuginfo | FileCheck --check-prefix=DI %s
+
+// RUN: %clang_cc1 -triple x86_64-linux-gnu -emit-llvm -debug-info-kind=limited -o - -include-pch %t/foo-cg.pch %S/../Modules/Inputs/codegen-flags/use.cpp | FileCheck --check-prefix=CG-USE %s
+// RUN: %clang_cc1 -triple x86_64-linux-gnu -emit-llvm -debug-info-kind=limited -o - -include-pch %t/foo-di.pch %S/../Modules/Inputs/codegen-flags/use.cpp | FileCheck --check-prefix=DI-USE %s
+
+// CG: define weak_odr void @_Z2f1v
+// CG: DICompileUnit
+// CG-NOT: DICompositeType
+
+// CG-USE: declare void @_Z2f1v
+// CG-USE: DICompileUnit
+// CG-USE: distinct !DICompositeType(tag: DW_TAG_structure_type, name: "foo"
+
+// DI-NOT: define
+// DI: distinct !DICompositeType(tag: DW_TAG_structure_type, name: "foo"
+
+// DI-USE: define linkonce_odr void @_Z2f1v
+// DI-USE: = !DICompositeType(tag: DW_TAG_structure_type, name: "foo", {{.*}}, flags: DIFlagFwdDecl
Index: clang/test/Modules/Inputs/codegen-flags/foo.h
===
--- clang/test/Modules/Inputs/codegen-flags/foo.h
+++ clang/test/Modules/Inputs/codegen-flags/foo.h
@@ -1,4 +1,7 @@
+#ifndef FOO_H
+#define FOO_H
 struct foo {
 };
 inline void f1() {
 }
+#endif
Index: clang/lib/Serialization/ASTWriterDecl.cpp
===
--- clang/lib/Serialization/ASTWriterDecl.cpp
+++ clang/lib/Serialization/ASTWriterDecl.cpp
@@ -2460,9 +2460,10 @@
 
   assert(FD->doesThisDeclarationHaveABody());
   bool ModulesCodegen = false;
-  if (Writer->WritingModule && !FD->isDependentContext()) {
+  if (!FD->isDependentContext()) {
 Optional Linkage;
-if (Writer->WritingModule->Kind == Module::ModuleInterfaceUnit) {
+if (Writer->WritingModule &&
+Writer->WritingModule->Kind == Module::ModuleInterfaceUnit) {
   // When building a C++ Modules TS module interface unit, a strong
   // definition in the module interface is provided by the compilation of
   // that module interface unit, not by its users. (Inline functions are
Index: clang/lib/Serialization/ASTWriter.cpp
===
--- clang/lib/Serialization/ASTWriter.cpp
+++ clang/lib/Serialization/ASTWriter.cpp
@@ -5631,8 +5631,8 @@
 
   // getODRHash will compute the ODRHash if it has not been previously computed.
   Record->push_back(D->getODRHash());
-  bool ModulesDebugInfo = Writer->Context->getLangOpts().ModulesDebugInfo &&
-  Writer->WritingModule && !D->isDependentType();
+  bool ModulesDebugInfo =
+  Writer->Context->getLangOpts().ModulesDebugInfo && !D->isDependentType();
   Record->push_back(ModulesDebugInfo);
   if (ModulesDebugInfo)
 Writer->ModularCodegenDecls.push_back(Writer->GetDeclRef(D));
Index: clang/lib/Serialization/ASTReaderDecl.cpp
===
--- clang/lib/Serialization/ASTReaderDecl.cpp
+++ clang/lib/Serialization/ASTReaderDecl.cpp
@@ -503,8 +503,12 @@
 }
 
 void ASTDeclReader::ReadFunctionDefinition(FunctionDecl *FD) {
-  if (Record.readInt())
+  if (Record.readInt()) {
 Reader.DefinitionSource[FD] = Loc.F->Kind == ModuleKind::MK_MainFile;
+if 

[PATCH] D74846: fix -fcodegen-modules code when used with PCH (PR44953)

[Luboš Luňák via Phabricator via cfe-commits]

llunak abandoned this revision.
llunak added a comment.

This review did make sense when it was created, it's just that it took so long 
to get this processed that it was simpler to revert the original patch.

Anyway, I've merged the change to https://reviews.llvm.org/D69778 .


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D74846/new/

https://reviews.llvm.org/D74846



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D69778: Make -fmodules-codegen and -fmodules-debuginfo work also with precompiled headers

[Luboš Luňák via Phabricator via cfe-commits]

llunak reopened this revision.
llunak added a comment.
This revision is now accepted and ready to land.

Reopening because of https://reviews.llvm.org/D74846, updated version includes 
the partial revert from there.


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D69778/new/

https://reviews.llvm.org/D69778



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D74846: fix -fcodegen-modules code when used with PCH (PR44953)

[Luboš Luňák via Phabricator via cfe-commits]

llunak added a comment.

What is the practical difference? Either way the end result will be the same. 
And given that I get to wait ages for reviews of my PCH changes I find it more 
likely to get a review for a small patch rather than a large one.


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D74846/new/

https://reviews.llvm.org/D74846



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D74846: fix -fcodegen-modules code when used with PCH (PR44953)

[Luboš Luňák via Phabricator via cfe-commits]

llunak added a comment.

@aganea: This change reverts only a small part of my previous change (see my 
comment here from Feb 23). Hans reverted the original change only until this 
problem gets sorted out (see his comment from Feb 27 right before the revert).


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D74846/new/

https://reviews.llvm.org/D74846



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D73846: [PCH] make sure to not warn about unused macros from -D

[Luboš Luňák via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG5c8c9905c249: make sure to not warn about unused macros from 
-D (authored by llunak).

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D73846/new/

https://reviews.llvm.org/D73846

Files:
  clang/lib/Lex/PPDirectives.cpp
  clang/test/PCH/cli-macro.c


Index: clang/test/PCH/cli-macro.c
===
--- /dev/null
+++ clang/test/PCH/cli-macro.c
@@ -0,0 +1,12 @@
+// Test this without pch.
+// RUN: %clang_cc1 -Wunused-macros -Dunused=1 -fsyntax-only -verify %s
+
+// Test with pch.
+// RUN: %clang_cc1 -Wunused-macros -emit-pch -o %t %s
+// RUN: %clang_cc1 -Wunused-macros -Dunused=1 -include-pch %t -fsyntax-only 
-verify %s
+
+// expected-no-diagnostics
+
+// -Dunused=1 is intentionally not set for the pch.
+// There still should be no unused warning for a macro from the command line.
+
Index: clang/lib/Lex/PPDirectives.cpp
===
--- clang/lib/Lex/PPDirectives.cpp
+++ clang/lib/Lex/PPDirectives.cpp
@@ -2802,7 +2802,9 @@
   // warn-because-unused-macro set. If it gets used it will be removed from 
set.
   if (getSourceManager().isInMainFile(MI->getDefinitionLoc()) &&
   !Diags->isIgnored(diag::pp_macro_not_used, MI->getDefinitionLoc()) &&
-  !MacroExpansionInDirectivesOverride) {
+  !MacroExpansionInDirectivesOverride &&
+  getSourceManager().getFileID(MI->getDefinitionLoc()) !=
+  getPredefinesFileID()) {
 MI->setIsWarnIfUnused(true);
 WarnUnusedMacroLocs.insert(MI->getDefinitionLoc());
   }


Index: clang/test/PCH/cli-macro.c
===
--- /dev/null
+++ clang/test/PCH/cli-macro.c
@@ -0,0 +1,12 @@
+// Test this without pch.
+// RUN: %clang_cc1 -Wunused-macros -Dunused=1 -fsyntax-only -verify %s
+
+// Test with pch.
+// RUN: %clang_cc1 -Wunused-macros -emit-pch -o %t %s
+// RUN: %clang_cc1 -Wunused-macros -Dunused=1 -include-pch %t -fsyntax-only -verify %s
+
+// expected-no-diagnostics
+
+// -Dunused=1 is intentionally not set for the pch.
+// There still should be no unused warning for a macro from the command line.
+
Index: clang/lib/Lex/PPDirectives.cpp
===
--- clang/lib/Lex/PPDirectives.cpp
+++ clang/lib/Lex/PPDirectives.cpp
@@ -2802,7 +2802,9 @@
   // warn-because-unused-macro set. If it gets used it will be removed from set.
   if (getSourceManager().isInMainFile(MI->getDefinitionLoc()) &&
   !Diags->isIgnored(diag::pp_macro_not_used, MI->getDefinitionLoc()) &&
-  !MacroExpansionInDirectivesOverride) {
+  !MacroExpansionInDirectivesOverride &&
+  getSourceManager().getFileID(MI->getDefinitionLoc()) !=
+  getPredefinesFileID()) {
 MI->setIsWarnIfUnused(true);
 WarnUnusedMacroLocs.insert(MI->getDefinitionLoc());
   }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D74846: fix -fcodegen-modules code when used with PCH (PR44953)

[Luboš Luňák via Phabricator via cfe-commits]

llunak added a comment.

Ping


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D74846/new/

https://reviews.llvm.org/D74846



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D69585: Add option to instantiate templates already in the PCH

[Luboš Luňák via Phabricator via cfe-commits]

llunak added a comment.

Ping.


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D69585/new/

https://reviews.llvm.org/D69585



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D73846: [PCH] make sure to not warn about unused macros from -D

[Luboš Luňák via Phabricator via cfe-commits]

llunak added a comment.

In D73846#1991330 , @rsmith wrote:

> Looks OK as a workaround. Do you know why we consider these to be in the main 
> file? If we could fix that in the source manager, that'd seem preferable.


According to my testing, SourceManager::isInMainFile() can handle "" 
locations in 3 ways:

- for macros defined using -D on the command line the control flow returns 
false in the "if (FI.hasLineDirectives())" block
- for built-in macros such as __clang__ the control flow enters the same "if 
(FI.hasLineDirectives())" block, but Entry->IncludeOffset is 0, so the flow 
then reaches the final "return FI.getIncludeLoc().isInvalid();", which returns 
true
- if PCH is used, macros defined using -D on the command line do not even enter 
"if (FI.hasLineDirectives())" and so they end up returning true the same way 
built-in macros do

But I don't understand this enough to know why and what that actually means.

I've also tried a patch that added SourceManager::setPredefinesFileID() and 
moved the check from this patch to SourceManager::isInMainFile(), but then 
tests fail because apparently Preprocessor::setPredefinesFileID() may be called 
multiple times.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D73846/new/

https://reviews.llvm.org/D73846



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D69585: Add option to instantiate templates already in the PCH

[Luboš Luňák via Phabricator via cfe-commits]

llunak updated this revision to Diff 258605.
llunak retitled this revision from "PerformPendingInstatiations() already in 
the PCH" to "Add option to instantiate templates already in the PCH".
llunak edited the summary of this revision.
llunak added a comment.

Changed to use -fpch-instantiate-templates to control the feature.


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D69585/new/

https://reviews.llvm.org/D69585

Files:
  clang/include/clang/Basic/LangOptions.def
  clang/include/clang/Driver/Options.td
  clang/include/clang/Sema/Sema.h
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/lib/Sema/Sema.cpp
  clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
  clang/test/PCH/codegen.cpp
  clang/test/PCH/crash-12631281.cpp
  clang/test/PCH/cxx-alias-decl.cpp
  clang/test/PCH/cxx-dependent-sized-ext-vector.cpp
  clang/test/PCH/cxx-explicit-specifier.cpp
  clang/test/PCH/cxx-exprs.cpp
  clang/test/PCH/cxx-friends.cpp
  clang/test/PCH/cxx-member-init.cpp
  clang/test/PCH/cxx-ms-function-specialization-class-scope.cpp
  clang/test/PCH/cxx-static_assert.cpp
  clang/test/PCH/cxx-templates.cpp
  clang/test/PCH/cxx-variadic-templates-with-default-params.cpp
  clang/test/PCH/cxx-variadic-templates.cpp
  clang/test/PCH/cxx0x-default-delete.cpp
  clang/test/PCH/cxx11-constexpr.cpp
  clang/test/PCH/cxx11-enum-template.cpp
  clang/test/PCH/cxx11-exception-spec.cpp
  clang/test/PCH/cxx11-inheriting-ctors.cpp
  clang/test/PCH/cxx11-user-defined-literals.cpp
  clang/test/PCH/cxx1y-decltype-auto.cpp
  clang/test/PCH/cxx1y-deduced-return-type.cpp
  clang/test/PCH/cxx1y-default-initializer.cpp
  clang/test/PCH/cxx1y-init-captures.cpp
  clang/test/PCH/cxx1y-variable-templates.cpp
  clang/test/PCH/cxx1z-aligned-alloc.cpp
  clang/test/PCH/cxx1z-decomposition.cpp
  clang/test/PCH/cxx1z-using-declaration.cpp
  clang/test/PCH/cxx2a-bitfield-init.cpp
  clang/test/PCH/cxx2a-concept-specialization-expr.cpp
  clang/test/PCH/cxx2a-constraints.cpp
  clang/test/PCH/cxx2a-defaulted-comparison.cpp
  clang/test/PCH/cxx2a-requires-expr.cpp
  clang/test/PCH/cxx2a-template-lambdas.cpp
  clang/test/PCH/delayed-pch-instantiate.cpp
  clang/test/PCH/friend-template.cpp
  clang/test/PCH/implicitly-deleted.cpp
  clang/test/PCH/late-parsed-instantiations.cpp
  clang/test/PCH/local_static.cpp
  clang/test/PCH/macro-undef.cpp
  clang/test/PCH/make-integer-seq.cpp
  clang/test/PCH/ms-if-exists.cpp
  clang/test/PCH/pch-instantiate-templates-forward-decl.cpp
  clang/test/PCH/pch-instantiate-templates.cpp
  clang/test/PCH/pr18806.cpp
  clang/test/PCH/pragma-diag-section.cpp
  clang/test/PCH/rdar10830559.cpp
  clang/test/PCH/specialization-after-instantiation.cpp
  clang/test/PCH/type_pack_element.cpp

Index: clang/test/PCH/type_pack_element.cpp
===
--- clang/test/PCH/type_pack_element.cpp
+++ clang/test/PCH/type_pack_element.cpp
@@ -1,6 +1,9 @@
 // RUN: %clang_cc1 -std=c++14 -x c++-header %s -emit-pch -o %t.pch
 // RUN: %clang_cc1 -std=c++14 -x c++ /dev/null -include-pch %t.pch
 
+// RUN: %clang_cc1 -std=c++14 -x c++-header %s -emit-pch -fpch-instantiate-templates -o %t.pch
+// RUN: %clang_cc1 -std=c++14 -x c++ /dev/null -include-pch %t.pch
+
 template 
 struct X { };
 
Index: clang/test/PCH/specialization-after-instantiation.cpp
===
--- /dev/null
+++ clang/test/PCH/specialization-after-instantiation.cpp
@@ -0,0 +1,32 @@
+// Test this without pch.
+// RUN: %clang_cc1 -fsyntax-only -verify -DBODY %s
+
+// Test with pch.
+// RUN: %clang_cc1 -emit-pch -o %t %s
+// RUN: %clang_cc1 -include-pch %t -fsyntax-only -verify -DBODY %s
+
+// RUN: %clang_cc1 -emit-pch -fpch-instantiate-templates -o %t %s
+// RUN: %clang_cc1 -include-pch %t -fsyntax-only -verify -DBODY %s
+
+#ifndef HEADER_H
+#define HEADER_H
+
+template 
+struct A {
+  int foo() const;
+};
+
+int bar(A *a) {
+  return a->foo();
+}
+
+#endif // HEADER_H
+
+#ifdef BODY
+
+template <>
+int A::foo() const { // expected-error {{explicit specialization of 'foo' after instantiation}}  // expected-note@20 {{implicit instantiation first required here}}
+  return 10;
+}
+
+#endif // BODY
Index: clang/test/PCH/rdar10830559.cpp
===
--- clang/test/PCH/rdar10830559.cpp
+++ clang/test/PCH/rdar10830559.cpp
@@ -6,6 +6,9 @@
 // RUN: %clang_cc1 -emit-pch -o %t %s
 // RUN: %clang_cc1 -include-pch %t -emit-llvm-only %t.empty.cpp 
 
+// RUN: %clang_cc1 -emit-pch -fpch-instantiate-templates -o %t %s
+// RUN: %clang_cc1 -include-pch %t -emit-llvm-only %t.empty.cpp
+
 // rdar://10830559
 
 //#pragma ms_struct on
Index: clang/test/PCH/pragma-diag-section.cpp
===
--- clang/test/PCH/pragma-diag-section.cpp
+++ clang/test/PCH/pragma-diag-section.cpp
@@ -5,6 +5,9 @@
 // RUN: %clang_cc1 %s -emit-pch -o %t
 // 

[PATCH] D73846: [PCH] make sure to not warn about unused macros from -D

[Luboš Luňák via Phabricator via cfe-commits]

llunak added a comment.

Ping..


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D73846/new/

https://reviews.llvm.org/D73846



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D74846: fix -fcodegen-modules code when used with PCH (PR44953)

[Luboš Luňák via Phabricator via cfe-commits]

llunak added a comment.

Ping...


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D74846/new/

https://reviews.llvm.org/D74846



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D69585: PerformPendingInstatiations() already in the PCH

[Luboš Luňák via Phabricator via cfe-commits]

llunak added a comment.

In D69585#1942431 , @rsmith wrote:

> This needs to be done behind a flag. It's an explicit design goal that 
> compilation behavior using a PCH or precompiled preamble behaves identically 
> to compilation not using a PCH / precompiled preamble. The behavior of this 
> patch is also non-conforming: we're only allowed to perform function template 
> instantiations at their points of instantiation (essentially, at points of 
> use + at the end of the translation unit).


How is that different from -fmodules? Given that AFAICT this makes PCHs work 
the same way as modules, this should mean -fmodules also both fails the 
identical behaviour goal and is non-conforming.

And to make sure I understand correctly, by behaving identically you mean that 
all the compiler output should be identical without even benign differences?


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D69585/new/

https://reviews.llvm.org/D69585



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D69585: PerformPendingInstatiations() already in the PCH

[Luboš Luňák via Phabricator via cfe-commits]

llunak added a comment.

Ping


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D69585/new/

https://reviews.llvm.org/D69585



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D73846: [PCH] make sure to not warn about unused macros from -D

[Luboš Luňák via Phabricator via cfe-commits]

llunak added a comment.

Ping.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D73846/new/

https://reviews.llvm.org/D73846



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D74846: fix -fcodegen-modules code when used with PCH (PR44953)

[Luboš Luňák via Phabricator via cfe-commits]

llunak added a comment.

Ping..


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D74846/new/

https://reviews.llvm.org/D74846



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D69778: Make -fmodules-codegen and -fmodules-debuginfo work also with precompiled headers

[Luboš Luňák via Phabricator via cfe-commits]

llunak added a comment.

In D69778#1929737 , @dblaikie wrote:

> I'm not sure how that could be possible - there's no data transferred between 
> the compilation of the TU's object files and the PCH's object file, right?


Yes, there is, in a way - the PCH itself. I didn't say everything from the PCH 
was shared, in fact it's exactly the point that it's not everything from the 
PCH, unlike -fmodules-codegen.

> Looks to me like the original patch claims the PCH object file contains all 
> the dllexported inline functions from the header. Which, yes, is different 
> from -fmodules-codegen, but doesn't sound like it's based on usage & I'm not 
> sure how it could be based on usage.

It's based on what's to be emitted. See ASTContext::DeclMustBeEmitted(), I 
think the comment there says it all.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D69778/new/

https://reviews.llvm.org/D69778



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D69778: Make -fmodules-codegen and -fmodules-debuginfo work also with precompiled headers

[Luboš Luňák via Phabricator via cfe-commits]

llunak added a comment.

In D69778#1928111 , @dblaikie wrote:

> In D69778#1927761 , @llunak wrote:
>
> > It comes from 08c5a7b8fda1b97df9d027d1ac096f93edeb6c2f . AFAICT 
> > -fmodules-codegen is a superset of -building-pch-with-obj, the important 
> > difference is that -building-pch-with-obj decides which code will be shared 
> > while building TUs, while -fmodules-codegen decides while building the PCH.
>
>
> I'm not sure I follow - how would that be possible? building the object from 
> the PCH is done independently of any usage, so it can't depend on the way the 
> PCH is used by any TU that includes it, I think?


If a TU uses a PCH, it'll result in some code emitted in every TU that uses the 
same PCH (e.g. template instantiations from the PCH). And 
-building-pch-with-obj detects these and discards them in all TUs except the 
PCH's object file. That means that the code shared in the PCH's object file is 
only code that would be duplicated in all TUs using the PCH. This is 
fundamentally different from -fmodules-codegen, which decides what will be 
emitted and shared already when creating the PCH, which has the consequence 
that it selects also a lot of code that will be eventually unused, which causes 
problems like the -Wl,--gc-sections part or not being necessarily worth it 
(IIRC even your -fmodules-codegen commit says so). So while -fmodules-codegen 
should be generally superior, -building-pch-with-obj is trivial to use. That's 
my understanding of both the approaches (and I spent quite a lot of time 
looking at both when working on D69778  and 
D69585 ).


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D69778/new/

https://reviews.llvm.org/D69778



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D69778: Make -fmodules-codegen and -fmodules-debuginfo work also with precompiled headers

[Luboš Luňák via Phabricator via cfe-commits]

llunak added a comment.

In D69778#1927526 , @dblaikie wrote:

> @rnk  - know anything about the history of -building-pch-with-obj and whether 
> it could be replaced/merged with -fmodules-codegen? (-fmodules-codegen + 
> -fmodules-debuginfo, perhaps?)


It comes from 08c5a7b8fda1b97df9d027d1ac096f93edeb6c2f . AFAICT 
-fmodules-codegen is a superset of -building-pch-with-obj, the important 
difference is that -building-pch-with-obj decides which code will be shared 
while building TUs, while -fmodules-codegen decides while building the PCH. 
This in practice means that -building-pch-with-obj shares only code that would 
be actually used, which makes the result easier to use - using PCH with 
-fmodules-codegen pretty much requires something like -Wl,--gc-sections, 
otherwise there are many undefined references to internal symbols from other 
libraries caused by emitted code that isn't actually used. Since 
-building-pch-with-obj is internally used by clang-cl /Yc , -fmodules-codegen 
cannot be a drop-in replacement for it, unless the Microsoft linker 
automatically discards unused symbols (I don't know, no idea about Windows 
stuff).


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D69778/new/

https://reviews.llvm.org/D69778



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D69585: PerformPendingInstatiations() already in the PCH

[Luboš Luňák via Phabricator via cfe-commits]

llunak added a comment.

In D69585#1922108 , @dblaikie wrote:

> Does this still have an OpenMP special case? The production code changes 
> don't seem to mention OpenMP anymore, but there's still a lot of test updates 
> for OpenMP - what are they for?


It's been handled by b841b9e96e605bed5a1f9b846a07aae88c65ce02 
 . The 
tests need updates because they test compilation both with and without PCH use, 
and this patch reorders templates in the output, without any functional change.


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D69585/new/

https://reviews.llvm.org/D69585



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D74846: fix -fcodegen-modules code when used with PCH (PR44953)

[Luboš Luňák via Phabricator via cfe-commits]

llunak added a comment.

Ping. I don't particularly care about the declspec(selectany) corner case, but 
at least the mistake from  D69778  should be 
fixed (and it's a simple fix), so that it can be committed again.


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D74846/new/

https://reviews.llvm.org/D74846



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D69585: PerformPendingInstatiations() already in the PCH

[Luboš Luňák via Phabricator via cfe-commits]

llunak added a comment.

Ping...


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D69585/new/

https://reviews.llvm.org/D69585



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D73846: make sure to not warn about unused macros from -D

[Luboš Luňák via Phabricator via cfe-commits]

llunak added a comment.

Ping


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D73846/new/

https://reviews.llvm.org/D73846



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D73852: [clang] detect switch fallthrough marked by a comment (PR43465)

[Luboš Luňák via Phabricator via cfe-commits]

llunak added a comment.

In D73852#1901895 , @rsmith wrote:

> Thank you, Luboš, and sorry for the process problems here.


FWIW, the Clang repository here in Phabricator is still 'Inactive', so even 
though 
https://llvm.org/docs/Phabricator.html#requesting-a-review-via-the-web-interface
 does say to use it, reviews may again not get sent to the correct commits list 
if somebody else gets confused by that too (especially for projects other than 
LLVM itself or Clang, as the page doesn't even mention what the mailing lists 
for those are).


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D73852/new/

https://reviews.llvm.org/D73852



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D73852: [clang] detect switch fallthrough marked by a comment (PR43465)

[Luboš Luňák via Phabricator via cfe-commits]

llunak added a comment.

In D73852#1901186 , @rsmith wrote:

> We shouldn't enable the warning under -Wextra in language modes where there's 
> no standard way to suppress it.


That may be true, but that is not what the bugreport is about, it explicitly 
mentions flex. There are codebases that explicitly use -Wimplicit-fallthrough, 
and flex generates the same code even for C++. So either Clang needs to handle 
it, or flex needs to change (which may be non-trivial if 
https://bugs.llvm.org/show_bug.cgi?id=43465#c24 is right), or codebases using 
flex will need to either special-case Clang or live with the warning.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D73852/new/

https://reviews.llvm.org/D73852



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D69585: PerformPendingInstatiations() already in the PCH

[Luboš Luňák via Phabricator via cfe-commits]

llunak added a comment.

Ping..


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D69585/new/

https://reviews.llvm.org/D69585



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D73846: make sure to not warn about unused macros from -D

[Luboš Luňák via Phabricator via cfe-commits]

llunak added a comment.

Ping...


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D73846/new/

https://reviews.llvm.org/D73846



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D74846: fix -fcodegen-modules code when used with PCH (PR44953)

[Luboš Luňák via Phabricator via cfe-commits]

llunak added a comment.

In D74846#1891580 , @dblaikie wrote:

> In D74846#1891486 , @llunak wrote:
>
> > But before we get to this, can we please first fix my patch for 10.0? I 
> > didn't check properly for -fmodules-codegen in D69778 
> > , that's for certain. That can be fixed 
> > before finding out why exactly it causes the trouble it causes.
>
>
> Generally I'm not in favor of committing fixes/changes that are not 
> understood (that leaves traps in the codebase & potentially other bugs, if 
> we're committing code we don't understand). I'd rather revert the original 
> patch until it's better understood.


I think it'll help to split this into smaller parts:

1. In D69778  I made it possible to use 
-fmodules-codegen also for PCHs by simply enabling the ModulesCodegen code also 
when -fmodules-codegen is used together with -building-pch-with-obj. Looking at 
ASTDeclWriter::VisitVarDecl() in current git it can be clearly seen that I 
messed up, as the code around line 1020 depends on -building-pch-with-obj but 
not -fmodules-codegen, so the ModulesCodegen code gets activated also when 
clang-cl /Yc is used, without -fmodules-codegen. That's clearly wrong and 
that's why aganea runs into a crash caused by ModulesCodegen code despite not 
asking for it. I find that part very clear and understood, both versions of my 
patch fixed that in some way, and that's what should be fixed for 10.0.
2. In  D69778  I included all the 
ModulesCodegen code, even the one checking for Module::ModuleInterfaceUnit 
instead of -fmodules-codegen (and that's how I made the mistake). I assumed 
Module::ModuleInterfaceUnit was some kind of equivalent to the PCH's own object 
file. That might have been a mistake, I don't know. Given that I'm unable to 
find a variable for which that piece of code would make a difference, I can't 
even check. Regardless of what it is, it's unimportant for 10.0.
3. There's the crash about the _declspec(selectany) variable from the 
bugreport. It was triggered by my mistake, but I don't know what the actual 
cause of it is. And as said before, it may not be related to my patch at all 
other than that it got triggered by it. Again, it's presumably not important 
enough for 10.0.

>>> I know it's a bit of an awkward situation to test - but please include one 
>>> to demonstrate why the original code was inappropriate. At least useful for 
>>> reviewing/validating the patch, but also might help someone else not to 
>>> make the same change again in the future.
>> 
>> I don't know how to do that, except by doing the does-not-crash test that 
>> you refused above. The only way I can trigger the change in 
>> ASTDeclWriter::VisitVarDecl() to make any difference is by using the 
>> _declspec(selectany), but that crashes without the fix and does nothing with 
>> it.
> 
> Could you explain more what you mean by "does nothing"? I would've assumed it 
> would go down a path to generate IR that was otherwise/previously 
> untested/unreachable (due to the crash) - so testing that the resulting IR is 
> as expected (that the IR contains a selectany (or however that's lowered to 
> IR) declaration and that declaration is used to initialize the 'desc' 
> variable in main?) is what I would think would be appropriate here.

"Does nothing" means that my current patch disables the ModulesCodegen code for 
it, so it "does nothing" other than taking the normal non-ModulesCodegen paths, 
so there's nothing to test compared to the normal case. At least for the 1) 
case I don't see what to test and how to do it. And for case 2) I also don't 
know what to test, because I can't find what difference it makes in the PCH 
case, except for triggering the crash. And I don't know how to test the crash 
either (except by crashing/not-crashing), because it crashes before it 
generates any code.

>> Also, since I apparently don't know what the code in fact causes or why 
>> _declspec(selectany) crashes, I actually also don't know why the original 
>> code was inappropriate, other than that it crashes for an unknown reason. 
>> Since I simply reused the modules code for PCH, maybe _declspec(selectany) 
>> would crash with modules too? I don't know.
> 
> Could you verify this? My attempt to do so doesn't seem to have reproduced 
> the failure with modular codegen:

Your testcase is not equivalent. In your case ASTDeclWriter::VisitVarDecl() has 
ModulesCodegen set to false for 'D3D12_DEFAULT', because 
'Writer.WritingModule->Kind == Module::ModuleInterfaceUnit' is false. Due to my 
mistake, ModulesCodegen was true in the PCH case.


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D74846/new/

https://reviews.llvm.org/D74846



___
cfe-commits mailing list
cfe-commits@lists.llvm.org

[PATCH] D74846: fix -fcodegen-modules code when used with PCH (PR44953)

[Luboš Luňák via Phabricator via cfe-commits]

llunak added a comment.

In D74846#1889826 , @dblaikie wrote:

> I know it's a bit of an awkward situation to test - but please include one to 
> demonstrate why the original code was inappropriate. At least useful for 
> reviewing/validating the patch, but also might help someone else not to make 
> the same change again in the future.


I don't know how to do that, except by doing the does-not-crash test that you 
refused above. The only way I can trigger the change in 
ASTDeclWriter::VisitVarDecl() to make any difference is by using the 
_declspec(selectany), but that crashes without the fix and does nothing with 
it. I've tried static variables, static const variables, templated statics, 
inline variables, statics in functions and I don't know what the code actually 
affects other than _declspec(selectany) crashing.

Also, since I apparently don't know what the code in fact causes or why 
_declspec(selectany) crashes, I actually also don't know why the original code 
was inappropriate, other than that it crashes for an unknown reason. Since I 
simply reused the modules code for PCH, maybe _declspec(selectany) would crash 
with modules too? I don't know. But before we get to this, can we please first 
fix my patch for 10.0? I didn't check properly for -fmodules-codegen in D69778 
, that's for certain. That can be fixed before 
finding out why exactly it causes the trouble it causes.

In D74846#1891208 , @aganea wrote:

> @llunak Do you have time to address this patch this week? If not, I can 
> finish it and land it. Please let me know. We'd like to see it merged into 
> 10.0.


Feel free to do whatever you think would help you.


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D74846/new/

https://reviews.llvm.org/D74846



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D74846: fix -fcodegen-modules code when used with PCH (PR44953)

[Luboš Luňák via Phabricator via cfe-commits]

llunak updated this revision to Diff 246122.
llunak retitled this revision from "fix -fcodegen-modules code when used with 
PCH (PR44958)" to "fix -fcodegen-modules code when used with PCH (PR44953)".
llunak edited the summary of this revision.
llunak added a comment.

Upon further investigation it turns out I probably should not have enabled 
those two places for PCH in D69778  at all, 
since they do not deal with -fmodules-codegen. So reverting those two places 
should be the right fix. That also makes a test irrelevant (not much point in 
testing a rare corner-case for a code path never taken).


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D74846/new/

https://reviews.llvm.org/D74846

Files:
  clang/lib/Serialization/ASTWriterDecl.cpp


Index: clang/lib/Serialization/ASTWriterDecl.cpp
===
--- clang/lib/Serialization/ASTWriterDecl.cpp
+++ clang/lib/Serialization/ASTWriterDecl.cpp
@@ -1011,16 +1011,15 @@
 
   if (D->getStorageDuration() == SD_Static) {
 bool ModulesCodegen = false;
-if (!D->getDescribedVarTemplate() && !D->getMemberSpecializationInfo() &&
+if (Writer.WritingModule &&
+!D->getDescribedVarTemplate() && !D->getMemberSpecializationInfo() &&
 !isa(D)) {
   // When building a C++ Modules TS module interface unit, a strong
   // definition in the module interface is provided by the compilation of
   // that module interface unit, not by its users. (Inline variables are
   // still emitted in module users.)
   ModulesCodegen =
-  (((Writer.WritingModule &&
- Writer.WritingModule->Kind == Module::ModuleInterfaceUnit) ||
-Writer.Context->getLangOpts().BuildingPCHWithObjectFile) &&
+  (Writer.WritingModule->Kind == Module::ModuleInterfaceUnit &&
Writer.Context->GetGVALinkageForVariable(D) == GVA_StrongExternal);
 }
 Record.push_back(ModulesCodegen);
@@ -2451,9 +2450,8 @@
   bool ModulesCodegen = false;
   if (!FD->isDependentContext()) {
 Optional Linkage;
-if ((Writer->WritingModule &&
- Writer->WritingModule->Kind == Module::ModuleInterfaceUnit) ||
-Writer->Context->getLangOpts().BuildingPCHWithObjectFile) {
+if (Writer->WritingModule &&
+Writer->WritingModule->Kind == Module::ModuleInterfaceUnit) {
   // When building a C++ Modules TS module interface unit, a strong
   // definition in the module interface is provided by the compilation of
   // that module interface unit, not by its users. (Inline functions are


Index: clang/lib/Serialization/ASTWriterDecl.cpp
===
--- clang/lib/Serialization/ASTWriterDecl.cpp
+++ clang/lib/Serialization/ASTWriterDecl.cpp
@@ -1011,16 +1011,15 @@
 
   if (D->getStorageDuration() == SD_Static) {
 bool ModulesCodegen = false;
-if (!D->getDescribedVarTemplate() && !D->getMemberSpecializationInfo() &&
+if (Writer.WritingModule &&
+!D->getDescribedVarTemplate() && !D->getMemberSpecializationInfo() &&
 !isa(D)) {
   // When building a C++ Modules TS module interface unit, a strong
   // definition in the module interface is provided by the compilation of
   // that module interface unit, not by its users. (Inline variables are
   // still emitted in module users.)
   ModulesCodegen =
-  (((Writer.WritingModule &&
- Writer.WritingModule->Kind == Module::ModuleInterfaceUnit) ||
-Writer.Context->getLangOpts().BuildingPCHWithObjectFile) &&
+  (Writer.WritingModule->Kind == Module::ModuleInterfaceUnit &&
Writer.Context->GetGVALinkageForVariable(D) == GVA_StrongExternal);
 }
 Record.push_back(ModulesCodegen);
@@ -2451,9 +2450,8 @@
   bool ModulesCodegen = false;
   if (!FD->isDependentContext()) {
 Optional Linkage;
-if ((Writer->WritingModule &&
- Writer->WritingModule->Kind == Module::ModuleInterfaceUnit) ||
-Writer->Context->getLangOpts().BuildingPCHWithObjectFile) {
+if (Writer->WritingModule &&
+Writer->WritingModule->Kind == Module::ModuleInterfaceUnit) {
   // When building a C++ Modules TS module interface unit, a strong
   // definition in the module interface is provided by the compilation of
   // that module interface unit, not by its users. (Inline functions are
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D74846: fix -fcodegen-modules code when used with PCH (PR44958)

[Luboš Luňák via Phabricator via cfe-commits]

llunak created this revision.
llunak added reviewers: hans, dblaikie.
llunak added a project: clang.
Herald added a subscriber: cfe-commits.

In D69778  I incorrectly handled two cases 
(checking for -building-pch-with-obj without also checking -fmodules-codegen).


Repository:
  rC Clang

https://reviews.llvm.org/D74846

Files:
  clang/lib/Serialization/ASTWriterDecl.cpp
  clang/test/PCH/Inputs/declspec-selectany-pch.cpp
  clang/test/PCH/Inputs/declspec-selectany-pch.h
  clang/test/PCH/declspec-selectany.cpp


Index: clang/test/PCH/declspec-selectany.cpp
===
--- /dev/null
+++ clang/test/PCH/declspec-selectany.cpp
@@ -0,0 +1,9 @@
+// RUN: %clang_cl /Yc%S/Inputs/declspec-selectany-pch.h 
%S/Inputs/declspec-selectany-pch.cpp /c /Fo%t.obj /Fp%t.pch
+// RUN: %clang_cl /Yu%S/Inputs/declspec-selectany-pch.h %s /I%S/Inputs /c 
/Fo%t.obj /Fp%t.pch
+
+#include "declspec-selectany-pch.h"
+
+int main() {
+  CD3DX12_CPU_DESCRIPTOR_HANDLE desc = 
CD3DX12_CPU_DESCRIPTOR_HANDLE(D3D12_DEFAULT);
+  return desc.ptr;
+}
Index: clang/test/PCH/Inputs/declspec-selectany-pch.h
===
--- /dev/null
+++ clang/test/PCH/Inputs/declspec-selectany-pch.h
@@ -0,0 +1,7 @@
+struct CD3DX12_DEFAULT {};
+extern const __declspec(selectany) CD3DX12_DEFAULT D3D12_DEFAULT;
+
+struct CD3DX12_CPU_DESCRIPTOR_HANDLE {
+  CD3DX12_CPU_DESCRIPTOR_HANDLE(CD3DX12_DEFAULT) { ptr = 0; }
+  size_t ptr;
+};
Index: clang/test/PCH/Inputs/declspec-selectany-pch.cpp
===
--- /dev/null
+++ clang/test/PCH/Inputs/declspec-selectany-pch.cpp
@@ -0,0 +1 @@
+#include "declspec-selectany-pch.h"
Index: clang/lib/Serialization/ASTWriterDecl.cpp
===
--- clang/lib/Serialization/ASTWriterDecl.cpp
+++ clang/lib/Serialization/ASTWriterDecl.cpp
@@ -1017,10 +1017,12 @@
   // definition in the module interface is provided by the compilation of
   // that module interface unit, not by its users. (Inline variables are
   // still emitted in module users.)
+  // Similarly if a PCH is built with codegen and its own object file.
   ModulesCodegen =
   (((Writer.WritingModule &&
  Writer.WritingModule->Kind == Module::ModuleInterfaceUnit) ||
-Writer.Context->getLangOpts().BuildingPCHWithObjectFile) &&
+(Writer.Context->getLangOpts().BuildingPCHWithObjectFile &&
+ Writer.Context->getLangOpts().ModulesCodegen)) &&
Writer.Context->GetGVALinkageForVariable(D) == GVA_StrongExternal);
 }
 Record.push_back(ModulesCodegen);
@@ -2451,9 +2453,8 @@
   bool ModulesCodegen = false;
   if (!FD->isDependentContext()) {
 Optional Linkage;
-if ((Writer->WritingModule &&
- Writer->WritingModule->Kind == Module::ModuleInterfaceUnit) ||
-Writer->Context->getLangOpts().BuildingPCHWithObjectFile) {
+if (Writer->WritingModule &&
+Writer->WritingModule->Kind == Module::ModuleInterfaceUnit) {
   // When building a C++ Modules TS module interface unit, a strong
   // definition in the module interface is provided by the compilation of
   // that module interface unit, not by its users. (Inline functions are


Index: clang/test/PCH/declspec-selectany.cpp
===
--- /dev/null
+++ clang/test/PCH/declspec-selectany.cpp
@@ -0,0 +1,9 @@
+// RUN: %clang_cl /Yc%S/Inputs/declspec-selectany-pch.h %S/Inputs/declspec-selectany-pch.cpp /c /Fo%t.obj /Fp%t.pch
+// RUN: %clang_cl /Yu%S/Inputs/declspec-selectany-pch.h %s /I%S/Inputs /c /Fo%t.obj /Fp%t.pch
+
+#include "declspec-selectany-pch.h"
+
+int main() {
+  CD3DX12_CPU_DESCRIPTOR_HANDLE desc = CD3DX12_CPU_DESCRIPTOR_HANDLE(D3D12_DEFAULT);
+  return desc.ptr;
+}
Index: clang/test/PCH/Inputs/declspec-selectany-pch.h
===
--- /dev/null
+++ clang/test/PCH/Inputs/declspec-selectany-pch.h
@@ -0,0 +1,7 @@
+struct CD3DX12_DEFAULT {};
+extern const __declspec(selectany) CD3DX12_DEFAULT D3D12_DEFAULT;
+
+struct CD3DX12_CPU_DESCRIPTOR_HANDLE {
+  CD3DX12_CPU_DESCRIPTOR_HANDLE(CD3DX12_DEFAULT) { ptr = 0; }
+  size_t ptr;
+};
Index: clang/test/PCH/Inputs/declspec-selectany-pch.cpp
===
--- /dev/null
+++ clang/test/PCH/Inputs/declspec-selectany-pch.cpp
@@ -0,0 +1 @@
+#include "declspec-selectany-pch.h"
Index: clang/lib/Serialization/ASTWriterDecl.cpp
===
--- clang/lib/Serialization/ASTWriterDecl.cpp
+++ clang/lib/Serialization/ASTWriterDecl.cpp
@@ -1017,10 +1017,12 @@
   // definition in the module interface is provided by the compilation of
   // that module interface unit, not by its users. (Inline variables 

[PATCH] D69585: PerformPendingInstatiations() already in the PCH

[Luboš Luňák via Phabricator via cfe-commits]

llunak added a comment.

Ping.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D69585/new/

https://reviews.llvm.org/D69585



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D73846: make sure to not warn about unused macros from -D

[Luboš Luňák via Phabricator via cfe-commits]

llunak added a comment.

Ping..


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D73846/new/

https://reviews.llvm.org/D73846



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D73852: [clang] detect switch fallthrough marked by a comment (PR43465)

[Luboš Luňák via Phabricator via cfe-commits]

llunak marked an inline comment as done.
llunak added a comment.

In D73852#1872013 , @lebedev.ri wrote:

> This patch also omitted cfe-commits lists.


That is a Phabricator problem. 
https://llvm.org/docs/Phabricator.html#requesting-a-review-via-the-web-interface
 says to select 'Clang' as the repository, but after the monorepo switch that 
repository no longer exists and Phabricator says 'Inactive' for it. So I 
(presumably, I don't remember) selected the monorepo and tagged the issue with 
'Clang'. If it's still required to use 'Clang' as the repository, then it 
shouldn't be marked as inactive, or alternatively cfe-commits should be added 
on the 'Clang' tag.




Comment at: clang/lib/Sema/AnalysisBasedWarnings.cpp:1239
+  llvm::Regex("(/\\*[ \\t]*fall(s | |-)?thr(ough|u)\\.?[ 
\\t]*\\*/)"
+  "|(//[ \\t]*fall(s | |-)?thr(ough|u)\\.?[ \\t]*)",
+  llvm::Regex::IgnoreCase);

aaron.ballman wrote:
> thakis wrote:
> > Also, this adds a regex match for every comment line, yes? Isn't this 
> > terrible for build performance? Did you do any benchmarking of this?
> https://reviews.llvm.org/D73852#inline-671309
As said above, it's on-demand, and in code without unannotated fallthough it'll 
be triggered exactly zero times.



Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D73852/new/

https://reviews.llvm.org/D73852



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D69585: PerformPendingInstatiations() already in the PCH

[Luboš Luňák via Phabricator via cfe-commits]

llunak added a comment.

Ping again. Is there still something more to do here?


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D69585/new/

https://reviews.llvm.org/D69585



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D73852: [clang] detect switch fallthrough marked by a comment (PR43465)

[Luboš Luňák via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG398b4ed87d48: [clang] detect switch fallthrough marked by a 
comment (PR43465) (authored by llunak).
Herald added a subscriber: cfe-commits.

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D73852/new/

https://reviews.llvm.org/D73852

Files:
  clang/lib/Sema/AnalysisBasedWarnings.cpp
  clang/test/Sema/fallthrough-comment.c


Index: clang/test/Sema/fallthrough-comment.c
===
--- /dev/null
+++ clang/test/Sema/fallthrough-comment.c
@@ -0,0 +1,20 @@
+// RUN: %clang_cc1 -fsyntax-only -std=c11 -verify -Wimplicit-fallthrough %s
+
+int fallthrough_comment(int n) {
+  switch (n) {
+  case 0:
+n++;
+// FALLTHROUGH
+  case 1:
+n++;
+
+/*fall-through.*/
+
+  case 2:
+n++;
+  case 3: // expected-warning{{unannotated fall-through between switch 
labels}} expected-note{{insert '__attribute__((fallthrough));' to silence this 
warning}} expected-note{{insert 'break;' to avoid fall-through}}
+n++;
+break;
+  }
+  return n;
+}
Index: clang/lib/Sema/AnalysisBasedWarnings.cpp
===
--- clang/lib/Sema/AnalysisBasedWarnings.cpp
+++ clang/lib/Sema/AnalysisBasedWarnings.cpp
@@ -1148,6 +1148,11 @@
   continue;
 }
 
+if (isFollowedByFallThroughComment(LastStmt)) {
+  ++AnnotatedCnt;
+  continue; // Fallthrough comment, good.
+}
+
 ++UnannotatedCnt;
   }
   return !!UnannotatedCnt;
@@ -1208,10 +1213,41 @@
   return nullptr;
 }
 
+bool isFollowedByFallThroughComment(const Stmt *Statement) {
+  // Try to detect whether the fallthough is marked by a comment like
+  // /*FALLTHOUGH*/.
+  bool Invalid;
+  const char *SourceData = S.getSourceManager().getCharacterData(
+  Statement->getEndLoc(), );
+  if (Invalid)
+return false;
+  const char *LineStart = SourceData;
+  for (;;) {
+LineStart = strchr(LineStart, '\n');
+if (LineStart == nullptr)
+  return false;
+++LineStart; // Start of next line.
+const char *LineEnd = strchr(LineStart, '\n');
+StringRef Line(LineStart,
+   LineEnd ? LineEnd - LineStart : strlen(LineStart));
+if (LineStart == LineEnd ||
+Line.find_first_not_of(" \t\r") == StringRef::npos)
+  continue; // Whitespace-only line.
+if (!FallthroughRegex.isValid())
+  FallthroughRegex =
+  llvm::Regex("(/\\*[ \\t]*fall(s | |-)?thr(ough|u)\\.?[ 
\\t]*\\*/)"
+  "|(//[ \\t]*fall(s | |-)?thr(ough|u)\\.?[ \\t]*)",
+  llvm::Regex::IgnoreCase);
+assert(FallthroughRegex.isValid());
+return FallthroughRegex.match(Line);
+  }
+}
+
 bool FoundSwitchStatements;
 AttrStmts FallthroughStmts;
 Sema 
 llvm::SmallPtrSet ReachableBlocks;
+llvm::Regex FallthroughRegex;
   };
 } // anonymous namespace
 


Index: clang/test/Sema/fallthrough-comment.c
===
--- /dev/null
+++ clang/test/Sema/fallthrough-comment.c
@@ -0,0 +1,20 @@
+// RUN: %clang_cc1 -fsyntax-only -std=c11 -verify -Wimplicit-fallthrough %s
+
+int fallthrough_comment(int n) {
+  switch (n) {
+  case 0:
+n++;
+// FALLTHROUGH
+  case 1:
+n++;
+
+/*fall-through.*/
+
+  case 2:
+n++;
+  case 3: // expected-warning{{unannotated fall-through between switch labels}} expected-note{{insert '__attribute__((fallthrough));' to silence this warning}} expected-note{{insert 'break;' to avoid fall-through}}
+n++;
+break;
+  }
+  return n;
+}
Index: clang/lib/Sema/AnalysisBasedWarnings.cpp
===
--- clang/lib/Sema/AnalysisBasedWarnings.cpp
+++ clang/lib/Sema/AnalysisBasedWarnings.cpp
@@ -1148,6 +1148,11 @@
   continue;
 }
 
+if (isFollowedByFallThroughComment(LastStmt)) {
+  ++AnnotatedCnt;
+  continue; // Fallthrough comment, good.
+}
+
 ++UnannotatedCnt;
   }
   return !!UnannotatedCnt;
@@ -1208,10 +1213,41 @@
   return nullptr;
 }
 
+bool isFollowedByFallThroughComment(const Stmt *Statement) {
+  // Try to detect whether the fallthough is marked by a comment like
+  // /*FALLTHOUGH*/.
+  bool Invalid;
+  const char *SourceData = S.getSourceManager().getCharacterData(
+  Statement->getEndLoc(), );
+  if (Invalid)
+return false;
+  const char *LineStart = SourceData;
+  for (;;) {
+LineStart = strchr(LineStart, '\n');
+if (LineStart == nullptr)
+  return false;
+++LineStart; // Start of next line.
+const char *LineEnd = strchr(LineStart, '\n');
+StringRef Line(LineStart,

[PATCH] D69585: PerformPendingInstatiations() already in the PCH

[Luboš Luňák via Phabricator via cfe-commits]

llunak marked 2 inline comments as done.
llunak added inline comments.



Comment at: clang/lib/Sema/Sema.cpp:985-986
+
+// FIXME: Instantiating implicit templates already in the PCH breaks some
+// OpenMP-specific code paths, see https://reviews.llvm.org/D69585 .
+if(LangOpts.PCHInstantiateTemplates && !LangOpts.OpenMP) {

dblaikie wrote:
> llunak wrote:
> > rnk wrote:
> > > This really deserves to be debugged before we land it.
> > I debugged this more than 2 months ago, that's why the OpenMP code owner is 
> > added as a reviewer.  The initial diff (that I have no idea how to display 
> > here) included a suggested fix and the description said this was a WIP and 
> > included my findings about it. As far as I can tell the only effect that 
> > had was that this patch was sitting here all the time without a single 
> > reaction, so if nobody OpenMP related cares then I do not either.
> > 
> > 
> So the original comment said "breaks some Open-MP specific code paths" - what 
> kind of breakage occurs? (& I take it with the patch in its current form that 
> breakage is present in the patch (as opposed to the previous version that 
> conditionalized this feature so it didn't happen when OpenMP was enabled)?)
See https://reviews.llvm.org/D72759 . It's already been handled.




Comment at: clang/test/PCH/specialization-after-instantiation.cpp:2
+// Test this without pch.
+// RUN: %clang_cc1 -fsyntax-only -verify -DBODY %s
+

dblaikie wrote:
> Does this need testing without PCH here? Presumably that's already been 
> tested elsewhere for some time now?
Technically not, but it can't hurt (it seems to be the common practice with 
testing PCHs), and I'm not aware of that elsewhere place (i.e. I've looked and 
couldn't find it).



CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D69585/new/

https://reviews.llvm.org/D69585



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D69585: PerformPendingInstatiations() already in the PCH

[Luboš Luňák via Phabricator via cfe-commits]

llunak added inline comments.



Comment at: clang/lib/Sema/Sema.cpp:984
+
+PerformPendingInstantiations();
   }

aganea wrote:
> All tests pass if you add `if (LangOpts.BuildingPCHWithObjectFile)` here. But 
> if a specialization occurs inside a .CPP which includes the .PCH, not sure 
> what would/should happen, and if that'S handled. Definitely needs a test for 
> that. @rsmith WDYT?
> All tests pass if you add if (LangOpts.BuildingPCHWithObjectFile) here.

That is because BuildingPCHWithObjectFile is almost unused by tests, so this 
just disables the change. But the change should be enabled for everything, it 
doesn't depend on whether PCH's object file is used.

> But if a specialization occurs inside a .CPP which includes the .PCH, not 
> sure what would/should happen, and if that'S handled. 

You mean this?

```
a.h:
#pragma once
template< typename T >
struct A { int foo() const; };
int bar(A* a) { return a->foo(); }
a.cpp:
#include "a.h"
template<> int A::foo() const { return 10; }

```
That errors out with "explicit specialization of 'foo' after instantiation" 
with or without PCH used, with or without my patch. I don't know about tests 
for it though. I can add this as one.



CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D69585/new/

https://reviews.llvm.org/D69585



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D69585: PerformPendingInstatiations() already in the PCH

[Luboš Luňák via Phabricator via cfe-commits]

llunak added a comment.

In D69585#1825133 , @ABataev wrote:

> I thought you were going to add an option or a flag to control the behavior? 
> If so, just provide an option in tests to avoid triggering of the new 
> behavior (except for declare_target... test and those 2 you modified already) 
> and that's it.


It's not included in the latest version of the patch. As written above, I'm 
reasonably sure I was mistaken about the need for a flag, and it should be ok 
to simply do the change unconditionally. I can put the flag back just for the 
purpose of the tests if you want, that'd certainly make handling of the tests 
trivial, but then the tests wouldn't really test "normal" PCHs, so do you 
really want that?


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D69585/new/

https://reviews.llvm.org/D69585



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D69585: PerformPendingInstatiations() already in the PCH

[Luboš Luňák via Phabricator via cfe-commits]

llunak updated this revision to Diff 238477.
llunak added a comment.

In D69585#1821831 , @aganea wrote:

> What is the error?


I take that part back, actually. I don't quite remember anymore what exactly I 
did in October, but if I now revert the PCH tweaks in LibreOffice I did to 
avoid the error, the compilation fails even without the patch or with GCC. So I 
assume what really happened was that code changes triggered the error and I 
incorrectly assumed it was because of my patch. So, unless proven otherwise, I 
take it that my patch is actually technically correct without causing any code 
regressions (the OpenMP problem has just been fixed).

What remains is those 22 tests which fail because moving the instantiations 
reorders the code that is expected by FileCheck. This patch handles 2 of them, 
and it's already rather tedious (the OpenMP tests are large). Is this really 
the way to handle them, or does somebody have a better idea?


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D69585/new/

https://reviews.llvm.org/D69585

Files:
  clang/lib/Sema/Sema.cpp
  clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
  clang/test/CodeGenCXX/vla-lambda-capturing.cpp
  clang/test/OpenMP/single_codegen.cpp

Index: clang/test/OpenMP/single_codegen.cpp
===
--- clang/test/OpenMP/single_codegen.cpp
+++ clang/test/OpenMP/single_codegen.cpp
@@ -1,6 +1,6 @@
-// RUN: %clang_cc1 -verify -fopenmp -fnoopenmp-use-tls -x c++ -std=c++11 -triple x86_64-unknown-unknown -emit-llvm %s -fexceptions -fcxx-exceptions -o - | FileCheck %s
+// RUN: %clang_cc1 -verify -fopenmp -fnoopenmp-use-tls -x c++ -std=c++11 -triple x86_64-unknown-unknown -emit-llvm %s -fexceptions -fcxx-exceptions -o - | FileCheck --check-prefixes=CHECK,NOPCH %s
 // RUN: %clang_cc1 -fopenmp -fnoopenmp-use-tls -x c++ -std=c++11 -triple x86_64-unknown-unknown -fexceptions -fcxx-exceptions -emit-pch -o %t %s
-// RUN: %clang_cc1 -fopenmp -fnoopenmp-use-tls -x c++ -triple x86_64-unknown-unknown -fexceptions -fcxx-exceptions -std=c++11 -include-pch %t -verify %s -emit-llvm -o - | FileCheck %s
+// RUN: %clang_cc1 -fopenmp -fnoopenmp-use-tls -x c++ -triple x86_64-unknown-unknown -fexceptions -fcxx-exceptions -std=c++11 -include-pch %t -verify %s -emit-llvm -o - | FileCheck --check-prefixes=CHECK,PCH %s
 // RUN: %clang_cc1 -verify -triple x86_64-apple-darwin10 -std=c++11 -fopenmp -fnoopenmp-use-tls -fexceptions -fcxx-exceptions -debug-info-kind=line-tables-only -x c++ -emit-llvm %s -o - | FileCheck %s --check-prefix=TERM_DEBUG
 // RUN: %clang_cc1 -verify -fopenmp -fnoopenmp-use-tls -x c++ -std=c++11 -DARRAY -triple x86_64-apple-darwin10 -emit-llvm %s -o - | FileCheck -check-prefix=ARRAY %s
 
@@ -230,6 +230,58 @@
 // ARRAY: store %struct.St* %{{.+}}, %struct.St** %{{.+}},
 #endif
 
+// PCH-LABEL: @_ZN3SSTIdEC2Ev
+// PCH: getelementptr inbounds [[SST_TY]], [[SST_TY]]* %{{.+}}, i32 0, i32 0
+// PCH-NEXT: store double 0.00e+00, double* %
+// PCH-NEXT: getelementptr inbounds [[SST_TY]], [[SST_TY]]* %{{.+}}, i32 0, i32 0
+// PCH-NEXT: store double* %{{.+}}, double** %
+// PCH-NEXT: load double*, double** %
+// PCH-NEXT: load double, double* %
+// PCH-NEXT: bitcast i64* %{{.+}} to double*
+// PCH-NEXT: store double %{{.+}}, double* %
+// PCH-NEXT: load i64, i64* %
+// PCH-NEXT: call void ([[IDENT_T_TY]]*, i32, void (i32*, i32*, ...)*, ...) @__kmpc_fork_call([[IDENT_T_TY]]* @{{.+}}, i32 2, void (i32*, i32*, ...)* bitcast (void (i32*, i32*, [[SST_TY]]*, i64)* [[SST_MICROTASK:@.+]] to void
+// PCH-NEXT: ret void
+
+// PCH: define internal void [[SST_MICROTASK]](i32* {{[^,]+}}, i32* {{[^,]+}}, [[SST_TY]]* {{.+}}, i64 {{.+}})
+// PCH: [[RES:%.+]] = call i32 @__kmpc_single([[IDENT_T_TY]]* @{{.+}}, i32 %{{.+}})
+// PCH-NEXT: icmp ne i32 [[RES]], 0
+// PCH-NEXT: br i1
+
+// PCH: getelementptr inbounds %{{.+}}, %{{.+}}* %{{.+}}, i32 0, i32 1
+// PCH-NEXT: load double*, double** %
+// PCH-NEXT: store double* %
+// PCH-LABEL: invoke void @_ZZN3SSTIdEC1EvENKUlvE_clEv(
+
+// PCH: call void @__kmpc_end_single([[IDENT_T_TY]]* @{{.+}}, i32 %{{.+}})
+// PCH-NEXT: store i32 1, i32* [[DID_IT]],
+// PCH-NEXT: br label
+
+// PCH: call void @__kmpc_end_single([[IDENT_T_TY]]* @{{.+}}, i32 %{{.+}})
+// PCH-NEXT: br label
+
+// PCH: getelementptr inbounds [1 x i8*], [1 x i8*]* [[LIST:%.+]], i64 0, i64 0
+// PCH: load double*, double** %
+// PCH-NEXT: bitcast double* %
+// PCH-NEXT: store i8* %
+// PCH-NEXT: bitcast [1 x i8*]* [[LIST]] to i8*
+// PCH-NEXT: load i32, i32* [[DID_IT]],
+// PCH-NEXT: call void @__kmpc_copyprivate([[IDENT_T_TY]]* @{{.+}}, i32 %{{.+}}, i64 8, i8* %{{.+}}, void (i8*, i8*)* [[COPY_FUNC:@[^,]+]], i32 %{{.+}})
+// PCH-NEXT:  ret void
+
+// PCH-LABEL: @_ZZN3SSTIdEC1EvENKUlvE_clEv(
+// PCH: getelementptr inbounds %{{.+}}, %{{.+}}* %{{.+}}, i32 0, i32 1
+// PCH-NEXT: getelementptr inbounds %{{.+}}, %{{.+}}* %{{.+}}, i32 0, i32 1
+// PCH-NEXT: load double*, double** %
+// 

[PATCH] D69585: PerformPendingInstatiations() already in the PCH

[Luboš Luňák via Phabricator via cfe-commits]

llunak updated this revision to Diff 238217.
llunak edited the summary of this revision.
llunak removed a project: OpenMP.
llunak added a comment.

In order to simplify this, I've updated the patch to remove any reference to 
OpenMP and I've moved that part to https://reviews.llvm.org/D72759 .

This means that the only thing missing here should be adding tests. Which, as 
said above, I'm unsure how to do exactly given that it'd probably need 
duplicating all PCH tests to use this option, so please advise how to do this 
so that the patch can be accepted.

In D69585#1820849 , @ABataev wrote:

> I don't see any crashes in OpenMP tests, other tests just must be updated by 
> the author, if this patch will ever going to be landed. I don't think it is a 
> good idea to have a not fully functional implementation, even guarded by the 
> option.


Please see https://reviews.llvm.org/D72759 .


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D69585/new/

https://reviews.llvm.org/D69585

Files:
  clang/include/clang/Basic/LangOptions.def
  clang/include/clang/Driver/CC1Options.td
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/lib/Sema/Sema.cpp


Index: clang/lib/Sema/Sema.cpp
===
--- clang/lib/Sema/Sema.cpp
+++ clang/lib/Sema/Sema.cpp
@@ -983,6 +983,12 @@
  LateParsedInstantiations.begin(),
  LateParsedInstantiations.end());
 LateParsedInstantiations.clear();
+
+if(LangOpts.PCHInstantiateTemplates) {
+  llvm::TimeTraceScope TimeScope("PerformPendingInstantiations",
+ StringRef(""));
+  PerformPendingInstantiations();
+}
   }
 
   DiagnoseUnterminatedPragmaPack();
Index: clang/lib/Frontend/CompilerInvocation.cpp
===
--- clang/lib/Frontend/CompilerInvocation.cpp
+++ clang/lib/Frontend/CompilerInvocation.cpp
@@ -3294,6 +3294,7 @@
 
   Opts.CompleteMemberPointers = Args.hasArg(OPT_fcomplete_member_pointers);
   Opts.BuildingPCHWithObjectFile = Args.hasArg(OPT_building_pch_with_obj);
+  Opts.PCHInstantiateTemplates = Args.hasArg(OPT_pch_instantiate_templates);
 }
 
 static bool isStrictlyPreprocessorAction(frontend::ActionKind Action) {
Index: clang/include/clang/Driver/CC1Options.td
===
--- clang/include/clang/Driver/CC1Options.td
+++ clang/include/clang/Driver/CC1Options.td
@@ -676,6 +676,8 @@
   HelpText<"Disable inclusion of timestamp in precompiled headers">;
 def building_pch_with_obj : Flag<["-"], "building-pch-with-obj">,
   HelpText<"This compilation is part of building a PCH with corresponding 
object file.">;
+def pch_instantiate_templates : Flag<["-"], "pch-instantiate-templates">,
+  HelpText<"Perform pending template instantiations already while building the 
PCH.">;
 
 def aligned_alloc_unavailable : Flag<["-"], "faligned-alloc-unavailable">,
   HelpText<"Aligned allocation/deallocation functions are unavailable">;
Index: clang/include/clang/Basic/LangOptions.def
===
--- clang/include/clang/Basic/LangOptions.def
+++ clang/include/clang/Basic/LangOptions.def
@@ -161,6 +161,7 @@
 BENIGN_LANGOPT(CompilingPCH, 1, 0, "building a pch")
 BENIGN_LANGOPT(BuildingPCHWithObjectFile, 1, 0, "building a pch which has a 
corresponding object file")
 BENIGN_LANGOPT(CacheGeneratedPCH, 1, 0, "cache generated PCH files in memory")
+BENIGN_LANGOPT(PCHInstantiateTemplates, 1, 0, "performing pending template 
instantiations already while building a pch")
 COMPATIBLE_LANGOPT(ModulesDeclUse, 1, 0, "require declaration of module 
uses")
 BENIGN_LANGOPT(ModulesSearchAll  , 1, 1, "searching even non-imported modules 
to find unresolved references")
 COMPATIBLE_LANGOPT(ModulesStrictDeclUse, 1, 0, "requiring declaration of 
module uses and all headers to be in modules")


Index: clang/lib/Sema/Sema.cpp
===
--- clang/lib/Sema/Sema.cpp
+++ clang/lib/Sema/Sema.cpp
@@ -983,6 +983,12 @@
  LateParsedInstantiations.begin(),
  LateParsedInstantiations.end());
 LateParsedInstantiations.clear();
+
+if(LangOpts.PCHInstantiateTemplates) {
+  llvm::TimeTraceScope TimeScope("PerformPendingInstantiations",
+ StringRef(""));
+  PerformPendingInstantiations();
+}
   }
 
   DiagnoseUnterminatedPragmaPack();
Index: clang/lib/Frontend/CompilerInvocation.cpp
===
--- clang/lib/Frontend/CompilerInvocation.cpp
+++ clang/lib/Frontend/CompilerInvocation.cpp
@@ -3294,6 +3294,7 @@
 
   Opts.CompleteMemberPointers = Args.hasArg(OPT_fcomplete_member_pointers);
   Opts.BuildingPCHWithObjectFile = 

[PATCH] D69778: Make -fmodules-codegen and -fmodules-debuginfo work also with precompiled headers

[Luboš Luňák via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGcbc9d22e49b4: make -fmodules-codegen and -fmodules-debuginfo 
work also with PCHs (authored by llunak).

Changed prior to commit:
  https://reviews.llvm.org/D69778?vs=227634=238115#toc

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D69778/new/

https://reviews.llvm.org/D69778

Files:
  clang/lib/Serialization/ASTReader.cpp
  clang/lib/Serialization/ASTReaderDecl.cpp
  clang/lib/Serialization/ASTWriter.cpp
  clang/lib/Serialization/ASTWriterDecl.cpp
  clang/test/Modules/Inputs/codegen-flags/foo.h
  clang/test/PCH/codegen.cpp

Index: clang/test/PCH/codegen.cpp
===
--- /dev/null
+++ clang/test/PCH/codegen.cpp
@@ -0,0 +1,30 @@
+// This test is the PCH version of Modules/codegen-flags.test . It uses its inputs.
+// The purpose of this test is just verify that the codegen options work with PCH as well.
+// All the codegen functionality should be tested in Modules/.
+
+// RUN: rm -rf %t
+// RUN: mkdir -p %t
+// REQUIRES: x86-registered-target
+
+// RUN: %clang_cc1 -triple=x86_64-linux-gnu -fmodules-codegen -x c++-header -building-pch-with-obj -emit-pch %S/../Modules/Inputs/codegen-flags/foo.h -o %t/foo-cg.pch
+// RUN: %clang_cc1 -triple=x86_64-linux-gnu -fmodules-debuginfo -x c++-header -building-pch-with-obj -emit-pch %S/../Modules/Inputs/codegen-flags/foo.h -o %t/foo-di.pch
+
+// RUN: %clang_cc1 -triple x86_64-linux-gnu -emit-llvm -debug-info-kind=limited -o - %s -include-pch %t/foo-cg.pch -building-pch-with-obj -fmodules-codegen | FileCheck --check-prefix=CG %s
+// RUN: %clang_cc1 -triple x86_64-linux-gnu -emit-llvm -debug-info-kind=limited -o - %s -include-pch %t/foo-di.pch -building-pch-with-obj -fmodules-debuginfo | FileCheck --check-prefix=DI %s
+
+// RUN: %clang_cc1 -triple x86_64-linux-gnu -emit-llvm -debug-info-kind=limited -o - -include-pch %t/foo-cg.pch %S/../Modules/Inputs/codegen-flags/use.cpp | FileCheck --check-prefix=CG-USE %s
+// RUN: %clang_cc1 -triple x86_64-linux-gnu -emit-llvm -debug-info-kind=limited -o - -include-pch %t/foo-di.pch %S/../Modules/Inputs/codegen-flags/use.cpp | FileCheck --check-prefix=DI-USE %s
+
+// CG: define weak_odr void @_Z2f1v
+// CG: DICompileUnit
+// CG-NOT: DICompositeType
+
+// CG-USE: declare void @_Z2f1v
+// CG-USE: DICompileUnit
+// CG-USE: distinct !DICompositeType(tag: DW_TAG_structure_type, name: "foo"
+
+// DI-NOT: define
+// DI: distinct !DICompositeType(tag: DW_TAG_structure_type, name: "foo"
+
+// DI-USE: define linkonce_odr void @_Z2f1v
+// DI-USE: = !DICompositeType(tag: DW_TAG_structure_type, name: "foo", {{.*}}, flags: DIFlagFwdDecl
Index: clang/test/Modules/Inputs/codegen-flags/foo.h
===
--- clang/test/Modules/Inputs/codegen-flags/foo.h
+++ clang/test/Modules/Inputs/codegen-flags/foo.h
@@ -1,4 +1,7 @@
+#ifndef FOO_H
+#define FOO_H
 struct foo {
 };
 inline void f1() {
 }
+#endif
Index: clang/lib/Serialization/ASTWriterDecl.cpp
===
--- clang/lib/Serialization/ASTWriterDecl.cpp
+++ clang/lib/Serialization/ASTWriterDecl.cpp
@@ -1010,15 +1010,16 @@
 
   if (D->getStorageDuration() == SD_Static) {
 bool ModulesCodegen = false;
-if (Writer.WritingModule &&
-!D->getDescribedVarTemplate() && !D->getMemberSpecializationInfo() &&
+if (!D->getDescribedVarTemplate() && !D->getMemberSpecializationInfo() &&
 !isa(D)) {
   // When building a C++ Modules TS module interface unit, a strong
   // definition in the module interface is provided by the compilation of
   // that module interface unit, not by its users. (Inline variables are
   // still emitted in module users.)
   ModulesCodegen =
-  (Writer.WritingModule->Kind == Module::ModuleInterfaceUnit &&
+  (((Writer.WritingModule &&
+ Writer.WritingModule->Kind == Module::ModuleInterfaceUnit) ||
+Writer.Context->getLangOpts().BuildingPCHWithObjectFile) &&
Writer.Context->GetGVALinkageForVariable(D) == GVA_StrongExternal);
 }
 Record.push_back(ModulesCodegen);
@@ -2425,9 +2426,11 @@
 
   assert(FD->doesThisDeclarationHaveABody());
   bool ModulesCodegen = false;
-  if (Writer->WritingModule && !FD->isDependentContext()) {
+  if (!FD->isDependentContext()) {
 Optional Linkage;
-if (Writer->WritingModule->Kind == Module::ModuleInterfaceUnit) {
+if ((Writer->WritingModule &&
+ Writer->WritingModule->Kind == Module::ModuleInterfaceUnit) ||
+Writer->Context->getLangOpts().BuildingPCHWithObjectFile) {
   // When building a C++ Modules TS module interface unit, a strong
   // definition in the module interface is provided by the compilation of
   // that module interface unit, not by its users. (Inline functions are
Index: 

[PATCH] D69779: -fmodules-codegen should not emit extern templates

[Luboš Luňák via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG729530f68fe1: -fmodules-codegen should not emit extern 
templates (authored by llunak).

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D69779/new/

https://reviews.llvm.org/D69779

Files:
  clang/lib/Serialization/ASTWriterDecl.cpp
  clang/test/Modules/codegen-extern-template.cpp
  clang/test/Modules/codegen-extern-template.h
  clang/test/Modules/codegen-extern-template.modulemap


Index: clang/test/Modules/codegen-extern-template.modulemap
===
--- /dev/null
+++ clang/test/Modules/codegen-extern-template.modulemap
@@ -0,0 +1 @@
+module foo { header "codegen-extern-template.h" }
Index: clang/test/Modules/codegen-extern-template.h
===
--- /dev/null
+++ clang/test/Modules/codegen-extern-template.h
@@ -0,0 +1,12 @@
+// header for codegen-extern-template.cpp
+#ifndef CODEGEN_EXTERN_TEMPLATE_H
+#define CODEGEN_EXTERN_TEMPLATE_H
+
+template 
+inline T foo() { return 10; }
+
+extern template int foo();
+
+inline int bar() { return foo(); }
+
+#endif
Index: clang/test/Modules/codegen-extern-template.cpp
===
--- /dev/null
+++ clang/test/Modules/codegen-extern-template.cpp
@@ -0,0 +1,9 @@
+// RUN: %clang_cc1 -triple=x86_64-linux-gnu -fmodules -fmodules-codegen 
-emit-module -fmodule-name=foo %S/codegen-extern-template.modulemap -x c++ -o 
%t.pcm
+// RUN: %clang_cc1 -triple x86_64-linux-gnu -fmodules -fmodule-file=%t.pcm %s 
-emit-llvm -o - | FileCheck %s
+// expected-no-diagnostics
+
+#include "codegen-extern-template.h"
+
+template int foo();
+
+// CHECK: define weak_odr i32 @_Z3fooIiET_v
Index: clang/lib/Serialization/ASTWriterDecl.cpp
===
--- clang/lib/Serialization/ASTWriterDecl.cpp
+++ clang/lib/Serialization/ASTWriterDecl.cpp
@@ -2437,11 +2437,12 @@
 }
 if (Writer->Context->getLangOpts().ModulesCodegen) {
   // Under -fmodules-codegen, codegen is performed for all non-internal,
-  // non-always_inline functions.
+  // non-always_inline functions, unless they are available elsewhere.
   if (!FD->hasAttr()) {
 if (!Linkage)
   Linkage = Writer->Context->GetGVALinkageForFunction(FD);
-ModulesCodegen = *Linkage != GVA_Internal;
+ModulesCodegen =
+*Linkage != GVA_Internal && *Linkage != GVA_AvailableExternally;
   }
 }
   }


Index: clang/test/Modules/codegen-extern-template.modulemap
===
--- /dev/null
+++ clang/test/Modules/codegen-extern-template.modulemap
@@ -0,0 +1 @@
+module foo { header "codegen-extern-template.h" }
Index: clang/test/Modules/codegen-extern-template.h
===
--- /dev/null
+++ clang/test/Modules/codegen-extern-template.h
@@ -0,0 +1,12 @@
+// header for codegen-extern-template.cpp
+#ifndef CODEGEN_EXTERN_TEMPLATE_H
+#define CODEGEN_EXTERN_TEMPLATE_H
+
+template 
+inline T foo() { return 10; }
+
+extern template int foo();
+
+inline int bar() { return foo(); }
+
+#endif
Index: clang/test/Modules/codegen-extern-template.cpp
===
--- /dev/null
+++ clang/test/Modules/codegen-extern-template.cpp
@@ -0,0 +1,9 @@
+// RUN: %clang_cc1 -triple=x86_64-linux-gnu -fmodules -fmodules-codegen -emit-module -fmodule-name=foo %S/codegen-extern-template.modulemap -x c++ -o %t.pcm
+// RUN: %clang_cc1 -triple x86_64-linux-gnu -fmodules -fmodule-file=%t.pcm %s -emit-llvm -o - | FileCheck %s
+// expected-no-diagnostics
+
+#include "codegen-extern-template.h"
+
+template int foo();
+
+// CHECK: define weak_odr i32 @_Z3fooIiET_v
Index: clang/lib/Serialization/ASTWriterDecl.cpp
===
--- clang/lib/Serialization/ASTWriterDecl.cpp
+++ clang/lib/Serialization/ASTWriterDecl.cpp
@@ -2437,11 +2437,12 @@
 }
 if (Writer->Context->getLangOpts().ModulesCodegen) {
   // Under -fmodules-codegen, codegen is performed for all non-internal,
-  // non-always_inline functions.
+  // non-always_inline functions, unless they are available elsewhere.
   if (!FD->hasAttr()) {
 if (!Linkage)
   Linkage = Writer->Context->GetGVALinkageForFunction(FD);
-ModulesCodegen = *Linkage != GVA_Internal;
+ModulesCodegen =
+*Linkage != GVA_Internal && *Linkage != GVA_AvailableExternally;
   }
 }
   }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D69779: -fmodules-codegen should not emit extern templates

[Luboš Luňák via Phabricator via cfe-commits]

llunak updated this revision to Diff 238089.
llunak added a comment.

This version uses a module based on the code posted above.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D69779/new/

https://reviews.llvm.org/D69779

Files:
  clang/lib/Serialization/ASTWriterDecl.cpp
  clang/test/Modules/codegen-extern-template.cpp
  clang/test/Modules/codegen-extern-template.h
  clang/test/Modules/codegen-extern-template.modulemap


Index: clang/test/Modules/codegen-extern-template.modulemap
===
--- /dev/null
+++ clang/test/Modules/codegen-extern-template.modulemap
@@ -0,0 +1 @@
+module foo { header "codegen-extern-template.h" }
Index: clang/test/Modules/codegen-extern-template.h
===
--- /dev/null
+++ clang/test/Modules/codegen-extern-template.h
@@ -0,0 +1,12 @@
+// header for codegen-extern-template.cpp
+#ifndef CODEGEN_EXTERN_TEMPLATE_H
+#define CODEGEN_EXTERN_TEMPLATE_H
+
+template 
+inline T foo() { return 10; }
+
+extern template int foo();
+
+inline int bar() { return foo(); }
+
+#endif
Index: clang/test/Modules/codegen-extern-template.cpp
===
--- /dev/null
+++ clang/test/Modules/codegen-extern-template.cpp
@@ -0,0 +1,9 @@
+// RUN: %clang_cc1 -triple=x86_64-linux-gnu -fmodules -fmodules-codegen 
-emit-module -fmodule-name=foo %S/codegen-extern-template.modulemap -x c++ -o 
%t.pcm
+// RUN: %clang_cc1 -triple x86_64-linux-gnu -fmodules -fmodule-file=%t.pcm %s 
-emit-llvm -o - | FileCheck %s
+// expected-no-diagnostics
+
+#include "codegen-extern-template.h"
+
+template int foo();
+
+// CHECK: define weak_odr i32 @_Z3fooIiET_v
Index: clang/lib/Serialization/ASTWriterDecl.cpp
===
--- clang/lib/Serialization/ASTWriterDecl.cpp
+++ clang/lib/Serialization/ASTWriterDecl.cpp
@@ -2412,11 +2412,12 @@
 }
 if (Writer->Context->getLangOpts().ModulesCodegen) {
   // Under -fmodules-codegen, codegen is performed for all non-internal,
-  // non-always_inline functions.
+  // non-always_inline functions, unless they are available elsewhere.
   if (!FD->hasAttr()) {
 if (!Linkage)
   Linkage = Writer->Context->GetGVALinkageForFunction(FD);
-ModulesCodegen = *Linkage != GVA_Internal;
+ModulesCodegen =
+*Linkage != GVA_Internal && *Linkage != GVA_AvailableExternally;
   }
 }
   }


Index: clang/test/Modules/codegen-extern-template.modulemap
===
--- /dev/null
+++ clang/test/Modules/codegen-extern-template.modulemap
@@ -0,0 +1 @@
+module foo { header "codegen-extern-template.h" }
Index: clang/test/Modules/codegen-extern-template.h
===
--- /dev/null
+++ clang/test/Modules/codegen-extern-template.h
@@ -0,0 +1,12 @@
+// header for codegen-extern-template.cpp
+#ifndef CODEGEN_EXTERN_TEMPLATE_H
+#define CODEGEN_EXTERN_TEMPLATE_H
+
+template 
+inline T foo() { return 10; }
+
+extern template int foo();
+
+inline int bar() { return foo(); }
+
+#endif
Index: clang/test/Modules/codegen-extern-template.cpp
===
--- /dev/null
+++ clang/test/Modules/codegen-extern-template.cpp
@@ -0,0 +1,9 @@
+// RUN: %clang_cc1 -triple=x86_64-linux-gnu -fmodules -fmodules-codegen -emit-module -fmodule-name=foo %S/codegen-extern-template.modulemap -x c++ -o %t.pcm
+// RUN: %clang_cc1 -triple x86_64-linux-gnu -fmodules -fmodule-file=%t.pcm %s -emit-llvm -o - | FileCheck %s
+// expected-no-diagnostics
+
+#include "codegen-extern-template.h"
+
+template int foo();
+
+// CHECK: define weak_odr i32 @_Z3fooIiET_v
Index: clang/lib/Serialization/ASTWriterDecl.cpp
===
--- clang/lib/Serialization/ASTWriterDecl.cpp
+++ clang/lib/Serialization/ASTWriterDecl.cpp
@@ -2412,11 +2412,12 @@
 }
 if (Writer->Context->getLangOpts().ModulesCodegen) {
   // Under -fmodules-codegen, codegen is performed for all non-internal,
-  // non-always_inline functions.
+  // non-always_inline functions, unless they are available elsewhere.
   if (!FD->hasAttr()) {
 if (!Linkage)
   Linkage = Writer->Context->GetGVALinkageForFunction(FD);
-ModulesCodegen = *Linkage != GVA_Internal;
+ModulesCodegen =
+*Linkage != GVA_Internal && *Linkage != GVA_AvailableExternally;
   }
 }
   }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D69779: -fmodules-codegen should not emit extern templates

[Luboš Luňák via Phabricator via cfe-commits]

llunak updated this revision to Diff 238086.
llunak added a comment.

I've updated the test as requested.

However I've noticed that a PCH-based test for this relies on 
https://reviews.llvm.org/D69585 . The fix works (of course), but it requires a 
PCH/module with the instantiation.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D69779/new/

https://reviews.llvm.org/D69779

Files:
  clang/lib/Serialization/ASTWriterDecl.cpp
  clang/test/PCH/codegen-extern-template.cpp
  clang/test/PCH/codegen-extern-template.h


Index: clang/test/PCH/codegen-extern-template.h
===
--- /dev/null
+++ clang/test/PCH/codegen-extern-template.h
@@ -0,0 +1,12 @@
+// header for codegen-extern-template.cpp
+#ifndef CODEGEN_EXTERN_TEMPLATE_H
+#define CODEGEN_EXTERN_TEMPLATE_H
+
+template 
+inline T foo() { return 10; }
+
+extern template int foo();
+
+inline int bar() { return foo(); }
+
+#endif
Index: clang/test/PCH/codegen-extern-template.cpp
===
--- /dev/null
+++ clang/test/PCH/codegen-extern-template.cpp
@@ -0,0 +1,9 @@
+// RUN: %clang_cc1 -triple=x86_64-linux-gnu -x c++-header 
-building-pch-with-obj -pch-instantiate-templates -fmodules-codegen -emit-pch 
%S/codegen-extern-template.h -o %t.pch
+// RUN: %clang_cc1 -triple x86_64-linux-gnu -emit-llvm -o - %s -include-pch 
%t.pch | FileCheck %s
+// expected-no-diagnostics
+
+#include "codegen-extern-template.h"
+
+template int foo();
+
+// CHECK: define weak_odr i32 @_Z3fooIiET_v
Index: clang/lib/Serialization/ASTWriterDecl.cpp
===
--- clang/lib/Serialization/ASTWriterDecl.cpp
+++ clang/lib/Serialization/ASTWriterDecl.cpp
@@ -2412,11 +2412,12 @@
 }
 if (Writer->Context->getLangOpts().ModulesCodegen) {
   // Under -fmodules-codegen, codegen is performed for all non-internal,
-  // non-always_inline functions.
+  // non-always_inline functions, unless they are available elsewhere.
   if (!FD->hasAttr()) {
 if (!Linkage)
   Linkage = Writer->Context->GetGVALinkageForFunction(FD);
-ModulesCodegen = *Linkage != GVA_Internal;
+ModulesCodegen =
+*Linkage != GVA_Internal && *Linkage != GVA_AvailableExternally;
   }
 }
   }


Index: clang/test/PCH/codegen-extern-template.h
===
--- /dev/null
+++ clang/test/PCH/codegen-extern-template.h
@@ -0,0 +1,12 @@
+// header for codegen-extern-template.cpp
+#ifndef CODEGEN_EXTERN_TEMPLATE_H
+#define CODEGEN_EXTERN_TEMPLATE_H
+
+template 
+inline T foo() { return 10; }
+
+extern template int foo();
+
+inline int bar() { return foo(); }
+
+#endif
Index: clang/test/PCH/codegen-extern-template.cpp
===
--- /dev/null
+++ clang/test/PCH/codegen-extern-template.cpp
@@ -0,0 +1,9 @@
+// RUN: %clang_cc1 -triple=x86_64-linux-gnu -x c++-header -building-pch-with-obj -pch-instantiate-templates -fmodules-codegen -emit-pch %S/codegen-extern-template.h -o %t.pch
+// RUN: %clang_cc1 -triple x86_64-linux-gnu -emit-llvm -o - %s -include-pch %t.pch | FileCheck %s
+// expected-no-diagnostics
+
+#include "codegen-extern-template.h"
+
+template int foo();
+
+// CHECK: define weak_odr i32 @_Z3fooIiET_v
Index: clang/lib/Serialization/ASTWriterDecl.cpp
===
--- clang/lib/Serialization/ASTWriterDecl.cpp
+++ clang/lib/Serialization/ASTWriterDecl.cpp
@@ -2412,11 +2412,12 @@
 }
 if (Writer->Context->getLangOpts().ModulesCodegen) {
   // Under -fmodules-codegen, codegen is performed for all non-internal,
-  // non-always_inline functions.
+  // non-always_inline functions, unless they are available elsewhere.
   if (!FD->hasAttr()) {
 if (!Linkage)
   Linkage = Writer->Context->GetGVALinkageForFunction(FD);
-ModulesCodegen = *Linkage != GVA_Internal;
+ModulesCodegen =
+*Linkage != GVA_Internal && *Linkage != GVA_AvailableExternally;
   }
 }
   }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D69585: PerformPendingInstatiations() already in the PCH

[Luboš Luňák via Phabricator via cfe-commits]

llunak marked 2 inline comments as done.
llunak added a comment.

In D69585#1818205 , @rnk wrote:

> I'm interested in making clang do this, but I think this needs significantly 
> more work until this is ready to land. It needs in-tree tests.


What tests specifically? I can add one test that just uses the option and then 
checks the PCH already contains what it should, but besides that this would 
need to repeat all PCH tests with this enabled. Should I just do that?

> I assumed we'd want to hook it up to `clang-cl /Yc` and `/Yu`.

If you mean automatically, then probably not. As said in the description, this 
leads to an error if a PCH'd template has a specialization that's not at least 
mentioned in the PCH. That's not a big deal and it's easy to handle, but it's 
still technically a regression. I myself wouldn't mind and would be fine with 
making this the default, but I assume (plural) you wouldn't.




Comment at: clang/include/clang/Basic/LangOptions.def:163
 BENIGN_LANGOPT(CacheGeneratedPCH, 1, 0, "cache generated PCH files in memory")
+BENIGN_LANGOPT(PCHInstantiateTemplates, 1, 0, "performing pending template 
instantiations already while building a pch")
 COMPATIBLE_LANGOPT(ModulesDeclUse, 1, 0, "require declaration of module 
uses")

rnk wrote:
> I think both sides of a PCH user and creator will need to agree on this, so 
> this isn't a "benign" langopt, is it? This is a regular one, and we should 
> diagnose mismatches between the PCH creation and use.
The flag needs to be used only when creating the PCH. Users don't care, they'll 
run the instantiation too, the flag will just make users do less repeated work.





Comment at: clang/lib/Sema/Sema.cpp:985-986
+
+// FIXME: Instantiating implicit templates already in the PCH breaks some
+// OpenMP-specific code paths, see https://reviews.llvm.org/D69585 .
+if(LangOpts.PCHInstantiateTemplates && !LangOpts.OpenMP) {

rnk wrote:
> This really deserves to be debugged before we land it.
I debugged this more than 2 months ago, that's why the OpenMP code owner is 
added as a reviewer.  The initial diff (that I have no idea how to display 
here) included a suggested fix and the description said this was a WIP and 
included my findings about it. As far as I can tell the only effect that had 
was that this patch was sitting here all the time without a single reaction, so 
if nobody OpenMP related cares then I do not either.




CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D69585/new/

https://reviews.llvm.org/D69585



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D69779: -fmodules-codegen should not emit extern templates

[Luboš Luňák via Phabricator via cfe-commits]

llunak added a comment.

Do you need some more information about the patch? It'd be nice if this could 
make it into 10.0.


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D69779/new/

https://reviews.llvm.org/D69779



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D69585: PerformPendingInstatiations() already in the PCH

[Luboš Luňák via Phabricator via cfe-commits]

llunak added a comment.

Ping.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D69585/new/

https://reviews.llvm.org/D69585



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D69779: -fmodules-codegen should not emit extern templates

[Luboš Luňák via Phabricator via cfe-commits]

llunak added a comment.

In D69779#1801771 , @dblaikie wrote:

> Ah, if I mark the standalone function template 'inline' (the implicit linkage 
> of member functions) then I get the same failure for both. Haven't tested 
> whether the fix is the same fix for both yet.


With my patch there are no errors with your testcase, even with the 'inline' 
added.


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D69779/new/

https://reviews.llvm.org/D69779



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D69778: Make -fmodules-codegen and -fmodules-debuginfo work also with precompiled headers

[Luboš Luňák via Phabricator via cfe-commits]

llunak added a comment.

In D69778#1776472 , @dblaikie wrote:

> I guess one aspect is that -building-pch-with-obj seems like it duplicates 
> the fmodules-codegen concept (both basically are a flag passed during pcm/pch 
> build time that says "I promise to build an object file from this pcm/pch, so 
> rely on that assumption when building other things that depend on the 
> pcm/pch) - if I'd noticed the -building-pch-with-obj going in I would've made 
> the point then.


Is that described somewhere? There's no mention of modules codegen anywhere 
under clang/docs. I was under the impression that modules created the codegen 
part directly in the .pcm in ELF (or whatever) format.

> But yeah, that's out of scope for this patch, but might be something that'd 
> be good to look into to try to merge these features/codepaths more.

Note that AFAICT the -building-pch-with-obj flag is pretty much internal, used 
only by the MSVC-like PCH creation, so presumably this could be still unified 
more without any real harm. And I think I could do the work if it'd be agreed 
that this would be a good thing.

As for this patch itself, could you please also review 
https://reviews.llvm.org/D69585 and https://reviews.llvm.org/D69779 ? I have 
not really tried this patch without the first one, so I'm a bit hesitant to 
merge it alone, and I quickly ran into the problem fixed by the second patch, 
so I'd prefer to merge them all three together if possible.


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D69778/new/

https://reviews.llvm.org/D69778



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D69778: Make -fmodules-codegen and -fmodules-debuginfo work also with precompiled headers

[Luboš Luňák via Phabricator via cfe-commits]

llunak added a comment.

In D69778#1771799 , @rsmith wrote:

> It's a bit weird for this to be controlled by a `-fmodules` flag, but it's 
> only a `-cc1` flag, so I'm OK with that; we can rename it if/when we expose 
> it from the driver.


It's a bit weird, but I didn't want to add -fpch-codegen and then duplicate the 
checks everywhere. And the -building-pch-with-obj part requires a non-trivial 
build setup anyway. But I can create another patch that maps it all to some 
common flag, if wanted.

In D69778#1772125 , @dblaikie wrote:

> I was/am still wondering whether there's a way to coalesce these codepaths 
> between PCH and the existing modular code generation support?


In what way could this be united more? The way I understand it, the code paths 
are pretty much the same, they just need to account for being invoked in 
different contexts. This patch is tiny compared to what it was originally 
before I made it reuse all the codegen code.


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D69778/new/

https://reviews.llvm.org/D69778



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D69779: -fmodules-codegen should not emit extern templates

[Luboš Luňák via Phabricator via cfe-commits]

llunak added a comment.

Ping.


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D69779/new/

https://reviews.llvm.org/D69779



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D69778: Make -fmodules-codegen and -fmodules-debuginfo work also with precompiled headers

[Luboš Luňák via Phabricator via cfe-commits]

llunak added a comment.

Ping.


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D69778/new/

https://reviews.llvm.org/D69778



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D69585: PerformPendingInstatiations() already in the PCH

[Luboš Luňák via Phabricator via cfe-commits]

llunak added a comment.

Ping.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D69585/new/

https://reviews.llvm.org/D69585



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D69585: PerformPendingInstatiations() already in the PCH

[Luboš Luňák via Phabricator via cfe-commits]

llunak updated this revision to Diff 229722.
llunak added a comment.

It turns out that this patch may introduce unwanted changes, specifically it 
can cause err_specialization_after_instantiation if the specialization is done 
in a source file but needed already by code in the PCH. But this seems to be 
rare (I've encountered it only after building the Skia library with 
put-everything-in-the-PCH), is easy to avoid (PCHs often need little tweaks 
anyway) and the performance gain is very much worth it.

Still, this means there should be an option to enable this. Done in this 
version of the patch. I don't know how to handle tests for it, pretty much all 
the tests pass as said above, but it seems a bit silly to duplicate all the 
PCH-related ones to also use the flag.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D69585/new/

https://reviews.llvm.org/D69585

Files:
  clang/include/clang/Basic/LangOptions.def
  clang/include/clang/Driver/CC1Options.td
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/lib/Sema/Sema.cpp


Index: clang/lib/Sema/Sema.cpp
===
--- clang/lib/Sema/Sema.cpp
+++ clang/lib/Sema/Sema.cpp
@@ -981,6 +981,14 @@
  LateParsedInstantiations.begin(),
  LateParsedInstantiations.end());
 LateParsedInstantiations.clear();
+
+// FIXME: Instantiating implicit templates already in the PCH breaks some
+// OpenMP-specific code paths, see https://reviews.llvm.org/D69585 .
+if(LangOpts.PCHInstantiateTemplates && !LangOpts.OpenMP) {
+  llvm::TimeTraceScope TimeScope("PerformPendingInstantiations",
+ StringRef(""));
+  PerformPendingInstantiations();
+}
   }
 
   DiagnoseUnterminatedPragmaPack();
Index: clang/lib/Frontend/CompilerInvocation.cpp
===
--- clang/lib/Frontend/CompilerInvocation.cpp
+++ clang/lib/Frontend/CompilerInvocation.cpp
@@ -3223,6 +3223,7 @@
 
   Opts.CompleteMemberPointers = Args.hasArg(OPT_fcomplete_member_pointers);
   Opts.BuildingPCHWithObjectFile = Args.hasArg(OPT_building_pch_with_obj);
+  Opts.PCHInstantiateTemplates = Args.hasArg(OPT_pch_instantiate_templates);
 }
 
 static bool isStrictlyPreprocessorAction(frontend::ActionKind Action) {
Index: clang/include/clang/Driver/CC1Options.td
===
--- clang/include/clang/Driver/CC1Options.td
+++ clang/include/clang/Driver/CC1Options.td
@@ -672,6 +672,8 @@
   HelpText<"Disable inclusion of timestamp in precompiled headers">;
 def building_pch_with_obj : Flag<["-"], "building-pch-with-obj">,
   HelpText<"This compilation is part of building a PCH with corresponding 
object file.">;
+def pch_instantiate_templates : Flag<["-"], "pch-instantiate-templates">,
+  HelpText<"Perform pending template instantiations already while building the 
PCH.">;
 
 def aligned_alloc_unavailable : Flag<["-"], "faligned-alloc-unavailable">,
   HelpText<"Aligned allocation/deallocation functions are unavailable">;
Index: clang/include/clang/Basic/LangOptions.def
===
--- clang/include/clang/Basic/LangOptions.def
+++ clang/include/clang/Basic/LangOptions.def
@@ -160,6 +160,7 @@
 BENIGN_LANGOPT(CompilingPCH, 1, 0, "building a pch")
 BENIGN_LANGOPT(BuildingPCHWithObjectFile, 1, 0, "building a pch which has a 
corresponding object file")
 BENIGN_LANGOPT(CacheGeneratedPCH, 1, 0, "cache generated PCH files in memory")
+BENIGN_LANGOPT(PCHInstantiateTemplates, 1, 0, "performing pending template 
instantiations already while building a pch")
 COMPATIBLE_LANGOPT(ModulesDeclUse, 1, 0, "require declaration of module 
uses")
 BENIGN_LANGOPT(ModulesSearchAll  , 1, 1, "searching even non-imported modules 
to find unresolved references")
 COMPATIBLE_LANGOPT(ModulesStrictDeclUse, 1, 0, "requiring declaration of 
module uses and all headers to be in modules")


Index: clang/lib/Sema/Sema.cpp
===
--- clang/lib/Sema/Sema.cpp
+++ clang/lib/Sema/Sema.cpp
@@ -981,6 +981,14 @@
  LateParsedInstantiations.begin(),
  LateParsedInstantiations.end());
 LateParsedInstantiations.clear();
+
+// FIXME: Instantiating implicit templates already in the PCH breaks some
+// OpenMP-specific code paths, see https://reviews.llvm.org/D69585 .
+if(LangOpts.PCHInstantiateTemplates && !LangOpts.OpenMP) {
+  llvm::TimeTraceScope TimeScope("PerformPendingInstantiations",
+ StringRef(""));
+  PerformPendingInstantiations();
+}
   }
 
   DiagnoseUnterminatedPragmaPack();
Index: clang/lib/Frontend/CompilerInvocation.cpp
===
--- 

[PATCH] D69779: -fmodules-codegen should not emit extern templates

[Luboš Luňák via Phabricator via cfe-commits]

llunak created this revision.
llunak added a reviewer: dblaikie.
llunak added a project: clang.
Herald added a subscriber: cfe-commits.

See the test for a testcase. If a header contains 'extern template', then the 
template should be provided somewhere by an explicit instantiation, so it is 
not necessary to generate a copy. Worse (at least with PCH 
-building-pch-with-obj), the current code actually leads to an unresolved 
symbol, because the PCH's object file will not actually contain functions from 
such a template (because of the GVA_AvailableExternally?), but the object file 
for the explicit instantiation will not contain them either because it will be 
blocked by the information provided by the PCH. I don't know if the same 
problem exists with modules (nor I know how to check for sure), all tests pass. 
If the problem is not there for modules, I can make the change PCH-specific.


Repository:
  rC Clang

https://reviews.llvm.org/D69779

Files:
  clang/lib/Serialization/ASTWriterDecl.cpp
  clang/test/PCH/codegen-extern-template.cpp
  clang/test/PCH/codegen-extern-template.h


Index: clang/test/PCH/codegen-extern-template.h
===
--- /dev/null
+++ clang/test/PCH/codegen-extern-template.h
@@ -0,0 +1,16 @@
+// header for codegen-extern-template.cpp
+#pragma once
+
+template< typename T >
+struct A
+{
+   T foo() { return 10;}
+};
+
+extern template struct A< int >;
+
+inline
+int bar(A* p)
+{
+return p->foo();
+}
Index: clang/test/PCH/codegen-extern-template.cpp
===
--- /dev/null
+++ clang/test/PCH/codegen-extern-template.cpp
@@ -0,0 +1,21 @@
+// Build the PCH with extern template.
+// RUN: %clang -x c++-header %S/codegen-extern-template.h -o %t.pch -Xclang 
-building-pch-with-obj -Xclang -fmodules-codegen
+// Build the PCH's object file.
+// RUN: %clang -c %s -include-pch %t.pch -Xclang -building-pch-with-obj 
-Xclang -fmodules-codegen -o %t.1.o
+// Build source with explicit template instantiation.
+// RUN: %clang -c %s -DMAIN -include-pch %t.pch -o %t.2.o
+// There should be no unresolved symbol.
+// RUN: %clang %t.1.o %t.2.o
+// expected-no-diagnostics
+
+#include "codegen-extern-template.h"
+
+#ifdef MAIN
+template struct A< int >;
+int main()
+{
+A< int > a;
+return bar();
+}
+
+#endif
Index: clang/lib/Serialization/ASTWriterDecl.cpp
===
--- clang/lib/Serialization/ASTWriterDecl.cpp
+++ clang/lib/Serialization/ASTWriterDecl.cpp
@@ -2412,11 +2412,12 @@
 }
 if (Writer->Context->getLangOpts().ModulesCodegen) {
   // Under -fmodules-codegen, codegen is performed for all non-internal,
-  // non-always_inline functions.
+  // non-always_inline functions, unless they are available elsewhere.
   if (!FD->hasAttr()) {
 if (!Linkage)
   Linkage = Writer->Context->GetGVALinkageForFunction(FD);
-ModulesCodegen = *Linkage != GVA_Internal;
+ModulesCodegen =
+*Linkage != GVA_Internal && *Linkage != GVA_AvailableExternally;
   }
 }
   }


Index: clang/test/PCH/codegen-extern-template.h
===
--- /dev/null
+++ clang/test/PCH/codegen-extern-template.h
@@ -0,0 +1,16 @@
+// header for codegen-extern-template.cpp
+#pragma once
+
+template< typename T >
+struct A
+{
+   T foo() { return 10;}
+};
+
+extern template struct A< int >;
+
+inline
+int bar(A* p)
+{
+return p->foo();
+}
Index: clang/test/PCH/codegen-extern-template.cpp
===
--- /dev/null
+++ clang/test/PCH/codegen-extern-template.cpp
@@ -0,0 +1,21 @@
+// Build the PCH with extern template.
+// RUN: %clang -x c++-header %S/codegen-extern-template.h -o %t.pch -Xclang -building-pch-with-obj -Xclang -fmodules-codegen
+// Build the PCH's object file.
+// RUN: %clang -c %s -include-pch %t.pch -Xclang -building-pch-with-obj -Xclang -fmodules-codegen -o %t.1.o
+// Build source with explicit template instantiation.
+// RUN: %clang -c %s -DMAIN -include-pch %t.pch -o %t.2.o
+// There should be no unresolved symbol.
+// RUN: %clang %t.1.o %t.2.o
+// expected-no-diagnostics
+
+#include "codegen-extern-template.h"
+
+#ifdef MAIN
+template struct A< int >;
+int main()
+{
+A< int > a;
+return bar();
+}
+
+#endif
Index: clang/lib/Serialization/ASTWriterDecl.cpp
===
--- clang/lib/Serialization/ASTWriterDecl.cpp
+++ clang/lib/Serialization/ASTWriterDecl.cpp
@@ -2412,11 +2412,12 @@
 }
 if (Writer->Context->getLangOpts().ModulesCodegen) {
   // Under -fmodules-codegen, codegen is performed for all non-internal,
-  // non-always_inline functions.
+  // non-always_inline functions, unless they are available elsewhere.
   if (!FD->hasAttr()) {
 if (!Linkage)
   Linkage = 

[PATCH] D69778: Make -fmodules-codegen and -fmodules-debuginfo work also with precompiled headers

[Luboš Luňák via Phabricator via cfe-commits]

llunak created this revision.
llunak added a reviewer: dblaikie.
llunak added a project: clang.
Herald added a subscriber: cfe-commits.

This patch allows to build PCH's (with -building-pch-with-obj and the extra .o 
file) with -fmodules-codegen -fmodules-debuginfo to allow emitting shared code 
into the extra .o file (presumably similarly how modules emit that to wherever 
they emit it). This saves up to 20% of build time here. I can build pretty much 
a complete debug build of LibreOffice with this, with just two minor problems 
(to be sorted out later).

The patch is fairly simple, it basically just duplicates -fmodules checks to 
also alternatively check -building-pch-with-obj.


Repository:
  rC Clang

https://reviews.llvm.org/D69778

Files:
  clang/lib/Serialization/ASTReader.cpp
  clang/lib/Serialization/ASTReaderDecl.cpp
  clang/lib/Serialization/ASTWriter.cpp
  clang/lib/Serialization/ASTWriterDecl.cpp
  clang/test/Modules/Inputs/codegen-flags/foo.h
  clang/test/PCH/codegen.cpp

Index: clang/test/PCH/codegen.cpp
===
--- /dev/null
+++ clang/test/PCH/codegen.cpp
@@ -0,0 +1,30 @@
+// This test is the PCH version of Modules/codegen-flags.test . It uses its inputs.
+// The purpose of this test is just verify that the codegen options work with PCH as well.
+// All the codegen functionality should be tested in Modules/.
+
+// RUN: rm -rf %t
+// RUN: mkdir -p %t
+// REQUIRES: x86-registered-target
+
+// RUN: %clang_cc1 -triple=x86_64-linux-gnu -fmodules-codegen -x c++-header -building-pch-with-obj -emit-pch %S/../Modules/Inputs/codegen-flags/foo.h -o %t/foo-cg.pch
+// RUN: %clang_cc1 -triple=x86_64-linux-gnu -fmodules-debuginfo -x c++-header -building-pch-with-obj -emit-pch %S/../Modules/Inputs/codegen-flags/foo.h -o %t/foo-di.pch
+
+// RUN: %clang_cc1 -triple x86_64-linux-gnu -emit-llvm -debug-info-kind=limited -o - %s -include-pch %t/foo-cg.pch -building-pch-with-obj -fmodules-codegen | FileCheck --check-prefix=CG %s
+// RUN: %clang_cc1 -triple x86_64-linux-gnu -emit-llvm -debug-info-kind=limited -o - %s -include-pch %t/foo-di.pch -building-pch-with-obj -fmodules-debuginfo | FileCheck --check-prefix=DI %s
+
+// RUN: %clang_cc1 -triple x86_64-linux-gnu -emit-llvm -debug-info-kind=limited -o - -include-pch %t/foo-cg.pch %S/../Modules/Inputs/codegen-flags/use.cpp | FileCheck --check-prefix=CG-USE %s
+// RUN: %clang_cc1 -triple x86_64-linux-gnu -emit-llvm -debug-info-kind=limited -o - -include-pch %t/foo-di.pch %S/../Modules/Inputs/codegen-flags/use.cpp | FileCheck --check-prefix=DI-USE %s
+
+// CG: define weak_odr void @_Z2f1v
+// CG: DICompileUnit
+// CG-NOT: DICompositeType
+
+// CG-USE: declare void @_Z2f1v
+// CG-USE: DICompileUnit
+// CG-USE: distinct !DICompositeType(tag: DW_TAG_structure_type, name: "foo"
+
+// DI-NOT: define
+// DI: distinct !DICompositeType(tag: DW_TAG_structure_type, name: "foo"
+
+// DI-USE: define linkonce_odr void @_Z2f1v
+// DI-USE: = !DICompositeType(tag: DW_TAG_structure_type, name: "foo", {{.*}}, flags: DIFlagFwdDecl
Index: clang/test/Modules/Inputs/codegen-flags/foo.h
===
--- clang/test/Modules/Inputs/codegen-flags/foo.h
+++ clang/test/Modules/Inputs/codegen-flags/foo.h
@@ -1,3 +1,4 @@
+#pragma once
 struct foo {
 };
 inline void f1() {
Index: clang/lib/Serialization/ASTWriterDecl.cpp
===
--- clang/lib/Serialization/ASTWriterDecl.cpp
+++ clang/lib/Serialization/ASTWriterDecl.cpp
@@ -990,15 +990,16 @@
 
   if (D->getStorageDuration() == SD_Static) {
 bool ModulesCodegen = false;
-if (Writer.WritingModule &&
-!D->getDescribedVarTemplate() && !D->getMemberSpecializationInfo() &&
+if (!D->getDescribedVarTemplate() && !D->getMemberSpecializationInfo() &&
 !isa(D)) {
   // When building a C++ Modules TS module interface unit, a strong
   // definition in the module interface is provided by the compilation of
   // that module interface unit, not by its users. (Inline variables are
   // still emitted in module users.)
   ModulesCodegen =
-  (Writer.WritingModule->Kind == Module::ModuleInterfaceUnit &&
+  (((Writer.WritingModule &&
+ Writer.WritingModule->Kind == Module::ModuleInterfaceUnit) ||
+Writer.Context->getLangOpts().BuildingPCHWithObjectFile) &&
Writer.Context->GetGVALinkageForVariable(D) == GVA_StrongExternal);
 }
 Record.push_back(ModulesCodegen);
@@ -2397,9 +2398,11 @@
 
   assert(FD->doesThisDeclarationHaveABody());
   bool ModulesCodegen = false;
-  if (Writer->WritingModule && !FD->isDependentContext()) {
+  if (!FD->isDependentContext()) {
 Optional Linkage;
-if (Writer->WritingModule->Kind == Module::ModuleInterfaceUnit) {
+if ((Writer->WritingModule &&
+ Writer->WritingModule->Kind == Module::ModuleInterfaceUnit) ||
+

[PATCH] D69750: make -ftime-trace also trace time spent creating debug info

[Luboš Luňák via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG4f2104c5adbc: make -ftime-trace also trace time spent 
creating debug info (authored by llunak).

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D69750/new/

https://reviews.llvm.org/D69750

Files:
  clang/lib/CodeGen/CGDebugInfo.cpp


Index: clang/lib/CodeGen/CGDebugInfo.cpp
===
--- clang/lib/CodeGen/CGDebugInfo.cpp
+++ clang/lib/CodeGen/CGDebugInfo.cpp
@@ -46,6 +46,7 @@
 #include "llvm/Support/FileSystem.h"
 #include "llvm/Support/MD5.h"
 #include "llvm/Support/Path.h"
+#include "llvm/Support/TimeProfiler.h"
 using namespace clang;
 using namespace clang::CodeGen;
 
@@ -2968,6 +2969,13 @@
   if (Ty.isNull())
 return nullptr;
 
+  llvm::TimeTraceScope TimeScope("DebugType", [&]() {
+std::string Name;
+llvm::raw_string_ostream OS(Name);
+Ty.print(OS, getPrintingPolicy());
+return Name;
+  });
+
   // Unwrap the type as needed for debug information.
   Ty = UnwrapTypeForDebugInfo(Ty, CGM.getContext());
 
@@ -3686,6 +3694,15 @@
   if (!D)
 return;
 
+  llvm::TimeTraceScope TimeScope("DebugFunction", [&]() {
+std::string Name;
+llvm::raw_string_ostream OS(Name);
+if (const NamedDecl *ND = dyn_cast(D))
+  ND->getNameForDiagnostic(OS, getPrintingPolicy(),
+   /*Qualified=*/true);
+return Name;
+  });
+
   llvm::DINode::DIFlags Flags = llvm::DINode::FlagZero;
   llvm::DIFile *Unit = getOrCreateFile(Loc);
   bool IsDeclForCallSite = Fn ? true : false;
@@ -4406,6 +4423,14 @@
   if (D->hasAttr())
 return;
 
+  llvm::TimeTraceScope TimeScope("DebugGlobalVariable", [&]() {
+std::string Name;
+llvm::raw_string_ostream OS(Name);
+D->getNameForDiagnostic(OS, getPrintingPolicy(),
+/*Qualified=*/true);
+return Name;
+  });
+
   // If we already created a DIGlobalVariable for this declaration, just attach
   // it to the llvm::GlobalVariable.
   auto Cached = DeclCache.find(D->getCanonicalDecl());
@@ -4466,6 +4491,14 @@
   assert(DebugKind >= codegenoptions::LimitedDebugInfo);
   if (VD->hasAttr())
 return;
+  llvm::TimeTraceScope TimeScope("DebugConstGlobalVariable", [&]() {
+std::string Name;
+llvm::raw_string_ostream OS(Name);
+VD->getNameForDiagnostic(OS, getPrintingPolicy(),
+ /*Qualified=*/true);
+return Name;
+  });
+
   auto Align = getDeclAlignIfRequired(VD, CGM.getContext());
   // Create the descriptor for the variable.
   llvm::DIFile *Unit = getOrCreateFile(VD->getLocation());


Index: clang/lib/CodeGen/CGDebugInfo.cpp
===
--- clang/lib/CodeGen/CGDebugInfo.cpp
+++ clang/lib/CodeGen/CGDebugInfo.cpp
@@ -46,6 +46,7 @@
 #include "llvm/Support/FileSystem.h"
 #include "llvm/Support/MD5.h"
 #include "llvm/Support/Path.h"
+#include "llvm/Support/TimeProfiler.h"
 using namespace clang;
 using namespace clang::CodeGen;
 
@@ -2968,6 +2969,13 @@
   if (Ty.isNull())
 return nullptr;
 
+  llvm::TimeTraceScope TimeScope("DebugType", [&]() {
+std::string Name;
+llvm::raw_string_ostream OS(Name);
+Ty.print(OS, getPrintingPolicy());
+return Name;
+  });
+
   // Unwrap the type as needed for debug information.
   Ty = UnwrapTypeForDebugInfo(Ty, CGM.getContext());
 
@@ -3686,6 +3694,15 @@
   if (!D)
 return;
 
+  llvm::TimeTraceScope TimeScope("DebugFunction", [&]() {
+std::string Name;
+llvm::raw_string_ostream OS(Name);
+if (const NamedDecl *ND = dyn_cast(D))
+  ND->getNameForDiagnostic(OS, getPrintingPolicy(),
+   /*Qualified=*/true);
+return Name;
+  });
+
   llvm::DINode::DIFlags Flags = llvm::DINode::FlagZero;
   llvm::DIFile *Unit = getOrCreateFile(Loc);
   bool IsDeclForCallSite = Fn ? true : false;
@@ -4406,6 +4423,14 @@
   if (D->hasAttr())
 return;
 
+  llvm::TimeTraceScope TimeScope("DebugGlobalVariable", [&]() {
+std::string Name;
+llvm::raw_string_ostream OS(Name);
+D->getNameForDiagnostic(OS, getPrintingPolicy(),
+/*Qualified=*/true);
+return Name;
+  });
+
   // If we already created a DIGlobalVariable for this declaration, just attach
   // it to the llvm::GlobalVariable.
   auto Cached = DeclCache.find(D->getCanonicalDecl());
@@ -4466,6 +4491,14 @@
   assert(DebugKind >= codegenoptions::LimitedDebugInfo);
   if (VD->hasAttr())
 return;
+  llvm::TimeTraceScope TimeScope("DebugConstGlobalVariable", [&]() {
+std::string Name;
+llvm::raw_string_ostream OS(Name);
+VD->getNameForDiagnostic(OS, getPrintingPolicy(),
+ /*Qualified=*/true);
+return Name;
+  });
+
   auto Align = getDeclAlignIfRequired(VD, CGM.getContext());
   // Create the descriptor for the variable.
   llvm::DIFile *Unit = getOrCreateFile(VD->getLocation());

[PATCH] D69750: make -ftime-trace also trace time spent creating debug info

[Luboš Luňák via Phabricator via cfe-commits]

llunak created this revision.
llunak added a reviewer: anton-afanasyev.
llunak added a project: clang.
Herald added subscribers: cfe-commits, aprantl.

In debug builds a noticeable time can be spent generating debug info, so make 
-ftime-trace track that too.


Repository:
  rC Clang

https://reviews.llvm.org/D69750

Files:
  clang/lib/CodeGen/CGDebugInfo.cpp


Index: clang/lib/CodeGen/CGDebugInfo.cpp
===
--- clang/lib/CodeGen/CGDebugInfo.cpp
+++ clang/lib/CodeGen/CGDebugInfo.cpp
@@ -46,6 +46,7 @@
 #include "llvm/Support/FileSystem.h"
 #include "llvm/Support/MD5.h"
 #include "llvm/Support/Path.h"
+#include "llvm/Support/TimeProfiler.h"
 using namespace clang;
 using namespace clang::CodeGen;
 
@@ -2946,6 +2947,13 @@
   if (Ty.isNull())
 return nullptr;
 
+  llvm::TimeTraceScope TimeScope("DebugType", [&]() {
+std::string Name;
+llvm::raw_string_ostream OS(Name);
+Ty.print(OS, getPrintingPolicy());
+return Name;
+  });
+
   // Unwrap the type as needed for debug information.
   Ty = UnwrapTypeForDebugInfo(Ty, CGM.getContext());
 
@@ -3664,6 +3672,15 @@
   if (!D)
 return;
 
+  llvm::TimeTraceScope TimeScope("DebugFunction", [&]() {
+std::string Name;
+llvm::raw_string_ostream OS(Name);
+if (const NamedDecl *ND = dyn_cast(D))
+  ND->getNameForDiagnostic(OS, getPrintingPolicy(),
+   /*Qualified=*/true);
+return Name;
+  });
+
   llvm::DINode::DIFlags Flags = llvm::DINode::FlagZero;
   llvm::DIFile *Unit = getOrCreateFile(Loc);
   bool IsDeclForCallSite = Fn ? true : false;
@@ -4384,6 +4401,14 @@
   if (D->hasAttr())
 return;
 
+  llvm::TimeTraceScope TimeScope("DebugGlobalVariable", [&]() {
+std::string Name;
+llvm::raw_string_ostream OS(Name);
+D->getNameForDiagnostic(OS, getPrintingPolicy(),
+/*Qualified=*/true);
+return Name;
+  });
+
   // If we already created a DIGlobalVariable for this declaration, just attach
   // it to the llvm::GlobalVariable.
   auto Cached = DeclCache.find(D->getCanonicalDecl());
@@ -,6 +4469,14 @@
   assert(DebugKind >= codegenoptions::LimitedDebugInfo);
   if (VD->hasAttr())
 return;
+  llvm::TimeTraceScope TimeScope("DebugConstGlobalVariable", [&]() {
+std::string Name;
+llvm::raw_string_ostream OS(Name);
+VD->getNameForDiagnostic(OS, getPrintingPolicy(),
+ /*Qualified=*/true);
+return Name;
+  });
+
   auto Align = getDeclAlignIfRequired(VD, CGM.getContext());
   // Create the descriptor for the variable.
   llvm::DIFile *Unit = getOrCreateFile(VD->getLocation());


Index: clang/lib/CodeGen/CGDebugInfo.cpp
===
--- clang/lib/CodeGen/CGDebugInfo.cpp
+++ clang/lib/CodeGen/CGDebugInfo.cpp
@@ -46,6 +46,7 @@
 #include "llvm/Support/FileSystem.h"
 #include "llvm/Support/MD5.h"
 #include "llvm/Support/Path.h"
+#include "llvm/Support/TimeProfiler.h"
 using namespace clang;
 using namespace clang::CodeGen;
 
@@ -2946,6 +2947,13 @@
   if (Ty.isNull())
 return nullptr;
 
+  llvm::TimeTraceScope TimeScope("DebugType", [&]() {
+std::string Name;
+llvm::raw_string_ostream OS(Name);
+Ty.print(OS, getPrintingPolicy());
+return Name;
+  });
+
   // Unwrap the type as needed for debug information.
   Ty = UnwrapTypeForDebugInfo(Ty, CGM.getContext());
 
@@ -3664,6 +3672,15 @@
   if (!D)
 return;
 
+  llvm::TimeTraceScope TimeScope("DebugFunction", [&]() {
+std::string Name;
+llvm::raw_string_ostream OS(Name);
+if (const NamedDecl *ND = dyn_cast(D))
+  ND->getNameForDiagnostic(OS, getPrintingPolicy(),
+   /*Qualified=*/true);
+return Name;
+  });
+
   llvm::DINode::DIFlags Flags = llvm::DINode::FlagZero;
   llvm::DIFile *Unit = getOrCreateFile(Loc);
   bool IsDeclForCallSite = Fn ? true : false;
@@ -4384,6 +4401,14 @@
   if (D->hasAttr())
 return;
 
+  llvm::TimeTraceScope TimeScope("DebugGlobalVariable", [&]() {
+std::string Name;
+llvm::raw_string_ostream OS(Name);
+D->getNameForDiagnostic(OS, getPrintingPolicy(),
+/*Qualified=*/true);
+return Name;
+  });
+
   // If we already created a DIGlobalVariable for this declaration, just attach
   // it to the llvm::GlobalVariable.
   auto Cached = DeclCache.find(D->getCanonicalDecl());
@@ -,6 +4469,14 @@
   assert(DebugKind >= codegenoptions::LimitedDebugInfo);
   if (VD->hasAttr())
 return;
+  llvm::TimeTraceScope TimeScope("DebugConstGlobalVariable", [&]() {
+std::string Name;
+llvm::raw_string_ostream OS(Name);
+VD->getNameForDiagnostic(OS, getPrintingPolicy(),
+ /*Qualified=*/true);
+return Name;
+  });
+
   auto Align = getDeclAlignIfRequired(VD, CGM.getContext());
   // Create the descriptor for the variable.
   llvm::DIFile *Unit = getOrCreateFile(VD->getLocation());

[PATCH] D69585: PerformPendingInstatiations() already in the PCH

[Luboš Luňák via Phabricator via cfe-commits]

llunak updated this revision to Diff 227575.
llunak added a comment.

Let's go a different route. This patch fully passes all tests and so should be 
ready to be committed.

It does so by opting out for OpenMP, which can be handled later whenever 
somebody who know about OpenMP looks at it.


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D69585/new/

https://reviews.llvm.org/D69585

Files:
  clang/lib/Sema/Sema.cpp
  clang/test/CodeGenCXX/vla-lambda-capturing.cpp


Index: clang/test/CodeGenCXX/vla-lambda-capturing.cpp
===
--- clang/test/CodeGenCXX/vla-lambda-capturing.cpp
+++ clang/test/CodeGenCXX/vla-lambda-capturing.cpp
@@ -1,6 +1,6 @@
-// RUN: %clang_cc1 %s -std=c++11 -emit-llvm -o - | FileCheck %s
+// RUN: %clang_cc1 %s -std=c++11 -emit-llvm -o - | FileCheck --check-prefixes 
CHECK,CHECK-NOPCH %s
 // RUN: %clang_cc1 %s -std=c++11 -emit-pch -o %t
-// RUN: %clang_cc1 %s -std=c++11 -include-pch %t -emit-llvm -o - | FileCheck %s
+// RUN: %clang_cc1 %s -std=c++11 -include-pch %t -emit-llvm -o - | FileCheck 
--check-prefixes CHECK,CHECK-PCH %s
 
 #ifndef HEADER
 #define HEADER
@@ -111,14 +111,14 @@
 // CHECK: call void @llvm.stackrestore(
 // CHECK: ret void
 
-// CHECK: define linkonce_odr{{.*}} void [[F_INT_LAMBDA]]([[CAP_TYPE2]]*
-// CHECK: [[THIS:%.+]] = load [[CAP_TYPE2]]*, [[CAP_TYPE2]]**
-// CHECK: [[SIZE_REF:%.+]] = getelementptr inbounds [[CAP_TYPE2]], 
[[CAP_TYPE2]]* [[THIS]], i{{.+}} 0, i{{.+}} 0
-// CHECK: [[SIZE:%.+]] = load [[INTPTR_T]], [[INTPTR_T]]* [[SIZE_REF]]
-// CHECK: call i{{.+}}* @llvm.stacksave()
-// CHECK: alloca [[INTPTR_T]], [[INTPTR_T]] [[SIZE]]
-// CHECK: call void @llvm.stackrestore(
-// CHECK: ret void
+// CHECK-NOPCH: define linkonce_odr{{.*}} void [[F_INT_LAMBDA]]([[CAP_TYPE2]]*
+// CHECK-NOPCH: [[THIS:%.+]] = load [[CAP_TYPE2]]*, [[CAP_TYPE2]]**
+// CHECK-NOPCH: [[SIZE_REF:%.+]] = getelementptr inbounds [[CAP_TYPE2]], 
[[CAP_TYPE2]]* [[THIS]], i{{.+}} 0, i{{.+}} 0
+// CHECK-NOPCH: [[SIZE:%.+]] = load [[INTPTR_T]], [[INTPTR_T]]* [[SIZE_REF]]
+// CHECK-NOPCH: call i{{.+}}* @llvm.stacksave()
+// CHECK-NOPCH: alloca [[INTPTR_T]], [[INTPTR_T]] [[SIZE]]
+// CHECK-NOPCH: call void @llvm.stackrestore(
+// CHECK-NOPCH: ret void
 
 // CHECK: define linkonce_odr{{.*}} void [[B_INT_LAMBDA]]([[CAP_TYPE3]]*
 // CHECK: [[SIZE2_REF:%.+]] = getelementptr inbounds [[CAP_TYPE3]], 
[[CAP_TYPE3]]* [[THIS:%.+]], i{{[0-9]+}} 0, i{{[0-9]+}} 1
@@ -168,4 +168,14 @@
 // CHECK: [[MUL:%.+]] = mul {{.*}} i{{[0-9]+}} [[SIZE2]], [[SIZE1]]
 // CHECK: mul {{.*}} i{{[0-9]+}} {{[0-9]+}}, [[MUL]]
 // CHECK: ret void
+
+// CHECK-PCH: define linkonce_odr{{.*}} void [[F_INT_LAMBDA]]([[CAP_TYPE2]]*
+// CHECK-PCH: [[THIS:%.+]] = load [[CAP_TYPE2]]*, [[CAP_TYPE2]]**
+// CHECK-PCH: [[SIZE_REF:%.+]] = getelementptr inbounds [[CAP_TYPE2]], 
[[CAP_TYPE2]]* [[THIS]], i{{.+}} 0, i{{.+}} 0
+// CHECK-PCH: [[SIZE:%.+]] = load [[INTPTR_T]], [[INTPTR_T]]* [[SIZE_REF]]
+// CHECK-PCH: call i{{.+}}* @llvm.stacksave()
+// CHECK-PCH: alloca [[INTPTR_T]], [[INTPTR_T]] [[SIZE]]
+// CHECK-PCH: call void @llvm.stackrestore(
+// CHECK-PCH: ret void
+
 #endif
Index: clang/lib/Sema/Sema.cpp
===
--- clang/lib/Sema/Sema.cpp
+++ clang/lib/Sema/Sema.cpp
@@ -981,6 +981,14 @@
  LateParsedInstantiations.begin(),
  LateParsedInstantiations.end());
 LateParsedInstantiations.clear();
+
+// FIXME: Instantiating implicit templates already in the PCH breaks some
+// OpenMP-specific code paths, see https://reviews.llvm.org/D69585 .
+if (!LangOpts.OpenMP) {
+  llvm::TimeTraceScope TimeScope("PerformPendingInstantiations",
+ StringRef(""));
+  PerformPendingInstantiations();
+}
   }
 
   DiagnoseUnterminatedPragmaPack();


Index: clang/test/CodeGenCXX/vla-lambda-capturing.cpp
===
--- clang/test/CodeGenCXX/vla-lambda-capturing.cpp
+++ clang/test/CodeGenCXX/vla-lambda-capturing.cpp
@@ -1,6 +1,6 @@
-// RUN: %clang_cc1 %s -std=c++11 -emit-llvm -o - | FileCheck %s
+// RUN: %clang_cc1 %s -std=c++11 -emit-llvm -o - | FileCheck --check-prefixes CHECK,CHECK-NOPCH %s
 // RUN: %clang_cc1 %s -std=c++11 -emit-pch -o %t
-// RUN: %clang_cc1 %s -std=c++11 -include-pch %t -emit-llvm -o - | FileCheck %s
+// RUN: %clang_cc1 %s -std=c++11 -include-pch %t -emit-llvm -o - | FileCheck --check-prefixes CHECK,CHECK-PCH %s
 
 #ifndef HEADER
 #define HEADER
@@ -111,14 +111,14 @@
 // CHECK: call void @llvm.stackrestore(
 // CHECK: ret void
 
-// CHECK: define linkonce_odr{{.*}} void [[F_INT_LAMBDA]]([[CAP_TYPE2]]*
-// CHECK: [[THIS:%.+]] = load [[CAP_TYPE2]]*, [[CAP_TYPE2]]**
-// CHECK: [[SIZE_REF:%.+]] = getelementptr inbounds [[CAP_TYPE2]], [[CAP_TYPE2]]* [[THIS]], i{{.+}} 0, i{{.+}} 0
-// CHECK: [[SIZE:%.+]] = load [[INTPTR_T]], 

[PATCH] D64284: (WIP) share template instantiations from PCH in one .o if -building-pch-with-obj

[Luboš Luňák via Phabricator via cfe-commits]

llunak abandoned this revision.
llunak added a comment.

Due to some technical problems with this approach and lack of feedback, I'm 
scratching this one. A new approach is at https://reviews.llvm.org/D69585 .


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D64284/new/

https://reviews.llvm.org/D64284



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D69585: PerformPendingInstatiations() already in the PCH

[Luboš Luňák via Phabricator via cfe-commits]

llunak created this revision.
llunak added reviewers: ABataev, rsmith, dblaikie.
llunak added projects: clang, OpenMP.
Herald added a reviewer: jdoerfert.
Herald added a subscriber: cfe-commits.

This patch makes template instantiations be already performed in the PCH 
instead of it being done in every single file that uses the PCH. I can see 
20-30% build time saved with the few tests I've tried.

The delaying of template instantiations until the PCH is used comes from 
7f76d11dcc9196e1fc9d1308da9ed2330a6b06c2 , which doesn't really give any useful 
reasoning for it, except for extending a unittest, which however now passes 
even with the instantiation moved back into the PCH. Since modules do not delay 
instantiation either, presumably whatever was wrong there has been fixed 
already.

I've built a complete debug build of LibreOffice with the patch and everything 
seems to work fine.

All Clang tests pass fine as well, with the exception of 23 tests, 22 of which 
are in OpenMP:

- OpenMP/declare_target_codegen.cpp fails because some virtuals of template 
classes are not emitted. This is fixed by the second change in the patch. I do 
not understand the purpose of that code (neither I have any clue about OpenMP), 
but it was introduced in d2c1dd5902782fb773c64dbf4e0b529aa4044b05 , which also 
changed this very test, and the change makes the test pass again.
- The remaining 22 tests, CodeGenCXX/vla-lambda-capturing.cpp and 21 in 
OpenMP/, all use the same FileCheck for normal compilation and using the source 
file as PCH input. Instantiating already in the PCH reorders some things, which 
makes the tests fail. Looking at the differences in the generated output, they 
all seem to be functionally equivalent to me, they just do not match exactly 
anymore.

That obviously makes the patch not ready, but I'd like to get feedback on this, 
before spending the time on adjusting the tests (which I expect will be quite a 
hassle[*]). So, is there any problem with this patch, besides adjusting the 
tests? And is there some easier way to handle those 21 OpenMP tests besides 
possibly (up to) doubling the checks in them?

[X] BTW, https://pastebin.com/Dg2L7K27 helped quite a lot with reviewing the 
differences.


Repository:
  rC Clang

https://reviews.llvm.org/D69585

Files:
  clang/lib/Sema/Sema.cpp
  clang/lib/Sema/SemaDeclCXX.cpp


Index: clang/lib/Sema/SemaDeclCXX.cpp
===
--- clang/lib/Sema/SemaDeclCXX.cpp
+++ clang/lib/Sema/SemaDeclCXX.cpp
@@ -15569,7 +15569,7 @@
 return;
   // Do not mark as used if compiling for the device outside of the target
   // region.
-  if (LangOpts.OpenMP && LangOpts.OpenMPIsDevice &&
+  if (TUKind != TU_Prefix && LangOpts.OpenMP && LangOpts.OpenMPIsDevice &&
   !isInOpenMPDeclareTargetContext() &&
   !isInOpenMPTargetExecutionDirective()) {
 if (!DefinitionRequired)
Index: clang/lib/Sema/Sema.cpp
===
--- clang/lib/Sema/Sema.cpp
+++ clang/lib/Sema/Sema.cpp
@@ -981,6 +981,12 @@
  LateParsedInstantiations.begin(),
  LateParsedInstantiations.end());
 LateParsedInstantiations.clear();
+
+{
+  llvm::TimeTraceScope TimeScope("PerformPendingInstantiations",
+ StringRef(""));
+  PerformPendingInstantiations();
+}
   }
 
   DiagnoseUnterminatedPragmaPack();


Index: clang/lib/Sema/SemaDeclCXX.cpp
===
--- clang/lib/Sema/SemaDeclCXX.cpp
+++ clang/lib/Sema/SemaDeclCXX.cpp
@@ -15569,7 +15569,7 @@
 return;
   // Do not mark as used if compiling for the device outside of the target
   // region.
-  if (LangOpts.OpenMP && LangOpts.OpenMPIsDevice &&
+  if (TUKind != TU_Prefix && LangOpts.OpenMP && LangOpts.OpenMPIsDevice &&
   !isInOpenMPDeclareTargetContext() &&
   !isInOpenMPTargetExecutionDirective()) {
 if (!DefinitionRequired)
Index: clang/lib/Sema/Sema.cpp
===
--- clang/lib/Sema/Sema.cpp
+++ clang/lib/Sema/Sema.cpp
@@ -981,6 +981,12 @@
  LateParsedInstantiations.begin(),
  LateParsedInstantiations.end());
 LateParsedInstantiations.clear();
+
+{
+  llvm::TimeTraceScope TimeScope("PerformPendingInstantiations",
+ StringRef(""));
+  PerformPendingInstantiations();
+}
   }
 
   DiagnoseUnterminatedPragmaPack();
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D63979: actually also compile output in tests for -frewrite-includes

[Luboš Luňák via Phabricator via cfe-commits]

This revision was not accepted when it landed; it landed in state "Needs 
Review".
This revision was automatically updated to reflect the committed changes.
Closed by commit rL372250: actually also compile output in tests for 
-frewrite-includes (authored by llunak, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D63979?vs=207226=220725#toc

Repository:
  rL LLVM

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D63979/new/

https://reviews.llvm.org/D63979

Files:
  cfe/trunk/test/Frontend/Inputs/NextIncludes/rewrite-includes9.h
  cfe/trunk/test/Frontend/Inputs/rewrite-includes1.h
  cfe/trunk/test/Frontend/Inputs/rewrite-includes2.h
  cfe/trunk/test/Frontend/Inputs/rewrite-includes3.h
  cfe/trunk/test/Frontend/Inputs/rewrite-includes4.h
  cfe/trunk/test/Frontend/Inputs/rewrite-includes5.h
  cfe/trunk/test/Frontend/Inputs/rewrite-includes6.h
  cfe/trunk/test/Frontend/Inputs/rewrite-includes7.h
  cfe/trunk/test/Frontend/rewrite-includes-cli-include.c
  cfe/trunk/test/Frontend/rewrite-includes-conditions.c
  cfe/trunk/test/Frontend/rewrite-includes.c

Index: cfe/trunk/test/Frontend/rewrite-includes.c
===
--- cfe/trunk/test/Frontend/rewrite-includes.c
+++ cfe/trunk/test/Frontend/rewrite-includes.c
@@ -1,8 +1,9 @@
-// RUN: not %clang_cc1 -verify -E -frewrite-includes -DFIRST -I %S/Inputs -I %S/Inputs/NextIncludes %s -o - | FileCheck -strict-whitespace %s
-// RUN: not %clang_cc1 -verify -E -frewrite-includes -P -DFIRST -I %S/Inputs -I %S/Inputs/NextIncludes %s -o - | FileCheck -check-prefix=CHECKNL -strict-whitespace %s
+// RUN: %clang_cc1 -E -frewrite-includes -DFIRST -I %S/Inputs -I %S/Inputs/NextIncludes %s -o - | FileCheck -strict-whitespace %s
+// RUN: %clang_cc1 -E -frewrite-includes -P -DFIRST -I %S/Inputs -I %S/Inputs/NextIncludes %s -o - | FileCheck -check-prefix=CHECKNL -strict-whitespace %s
+// RUN: %clang_cc1 -E -frewrite-includes -DFIRST -I %S/Inputs -I %S/Inputs/NextIncludes %s -o - | %clang_cc1 -Wall -Wextra -Wconversion -DFIRST -x c -fsyntax-only 2>&1 | FileCheck -check-prefix=COMPILE --implicit-check-not warning: %s
 // STARTCOMPARE
 #define A(a,b) a ## b
-A(1,2)
+A(in,t) a;
 #include "rewrite-includes1.h"
 #ifdef FIRST
 #define HEADER "rewrite-includes3.h"
@@ -21,94 +22,95 @@
 #include "rewrite-includes7.h"
 #include "rewrite-includes8.h"
 #include "rewrite-includes9.h"
+static int unused;
 // ENDCOMPARE
 // CHECK: {{^}}# 1 "{{.*}}rewrite-includes.c"{{$}}
 // CHECK: {{^}}// STARTCOMPARE{{$}}
 // CHECK-NEXT: {{^}}#define A(a,b) a ## b{{$}}
-// CHECK-NEXT: {{^}}A(1,2){{$}}
+// CHECK-NEXT: {{^}}A(in,t) a;{{$}}
 // CHECK-NEXT: {{^}}#if 0 /* expanded by -frewrite-includes */{{$}}
 // CHECK-NEXT: {{^}}#include "rewrite-includes1.h"{{$}}
 // CHECK-NEXT: {{^}}#endif /* expanded by -frewrite-includes */{{$}}
-// CHECK-NEXT: {{^}}# 6 "{{.*}}rewrite-includes.c"{{$}}
+// CHECK-NEXT: {{^}}# 7 "{{.*}}rewrite-includes.c"{{$}}
 // CHECK-NEXT: {{^}}# 1 "{{.*[/\\]Inputs(/|)}}rewrite-includes1.h" 1{{$}}
 // CHECK-NEXT: {{^}}#if 0 /* expanded by -frewrite-includes */{{$}}
 // CHECK-NEXT: {{^}}#pragma clang system_header{{$}}
 // CHECK-NEXT: {{^}}#endif /* expanded by -frewrite-includes */{{$}}
 // CHECK-NEXT: {{^}}# 2 "{{.*[/\\]Inputs(/|)}}rewrite-includes1.h" 3{{$}}
-// CHECK-NEXT: {{^}}included_line1{{$}}
+// CHECK-NEXT: {{^}}int included_line1;{{$}}
 // CHECK-NEXT: {{^}}#if 0 /* expanded by -frewrite-includes */{{$}}
 // CHECK-NEXT: {{^}}#include "rewrite-includes2.h"{{$}}
 // CHECK-NEXT: {{^}}#endif /* expanded by -frewrite-includes */{{$}}
 // CHECK-NEXT: {{^}}# 3 "{{.*[/\\]Inputs(/|)}}rewrite-includes1.h" 3{{$}}
 // CHECK-NEXT: {{^}}# 1 "{{.*[/\\]Inputs(/|)}}rewrite-includes2.h" 1 3{{$}}
-// CHECK-NEXT: {{^}}included_line2{{$}}
+// CHECK-NEXT: {{^}}int included_line2;{{$}}
 // CHECK-NEXT: {{^}}# 4 "{{.*[/\\]Inputs(/|)}}rewrite-includes1.h" 2 3{{$}}
-// CHECK-NEXT: {{^}}# 7 "{{.*}}rewrite-includes.c" 2{{$}}
+// CHECK-NEXT: {{^}}# 8 "{{.*}}rewrite-includes.c" 2{{$}}
 // CHECK-NEXT: {{^}}#ifdef FIRST{{$}}
 // CHECK-NEXT: {{^}}#define HEADER "rewrite-includes3.h"{{$}}
 // CHECK-NEXT: {{^}}#if 0 /* expanded by -frewrite-includes */{{$}}
 // CHECK-NEXT: {{^}}#include HEADER{{$}}
 // CHECK-NEXT: {{^}}#endif /* expanded by -frewrite-includes */{{$}}
-// CHECK-NEXT: {{^}}# 9 "{{.*}}rewrite-includes.c"{{$}}
+// CHECK-NEXT: {{^}}# 10 "{{.*}}rewrite-includes.c"{{$}}
 // CHECK-NEXT: {{^}}# 1 "{{.*[/\\]Inputs(/|)}}rewrite-includes3.h" 1{{$}}
-// CHECK-NEXT: {{^}}included_line3{{$}}
-// CHECK-NEXT: {{^}}# 10 "{{.*}}rewrite-includes.c" 2{{$}}
+// CHECK-NEXT: {{^}}unsigned int included_line3 = -10;{{$}}
+// CHECK-NEXT: {{^}}# 11 "{{.*}}rewrite-includes.c" 2{{$}}
 // CHECK-NEXT: {{^}}#else{{$}}
-// CHECK-NEXT: {{^}}# 11 "{{.*}}rewrite-includes.c"{{$}}
+// CHECK-NEXT: {{^}}# 12 "{{.*}}rewrite-includes.c"{{$}}
 // CHECK-NEXT: {{^}}#if 0 /* 

[PATCH] D63508: make -frewrite-includes also rewrite conditions in #if/#elif

[Luboš Luňák via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL372248: make -frewrite-includes also rewrite conditions in 
#if/#elif (authored by llunak, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D63508?vs=220378=220722#toc

Repository:
  rL LLVM

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D63508/new/

https://reviews.llvm.org/D63508

Files:
  cfe/trunk/lib/Frontend/Rewrite/InclusionRewriter.cpp
  cfe/trunk/test/Frontend/rewrite-includes-conditions.c
  cfe/trunk/test/Frontend/rewrite-includes.c
  cfe/trunk/test/Modules/preprocess-module.cpp

Index: cfe/trunk/lib/Frontend/Rewrite/InclusionRewriter.cpp
===
--- cfe/trunk/lib/Frontend/Rewrite/InclusionRewriter.cpp
+++ cfe/trunk/lib/Frontend/Rewrite/InclusionRewriter.cpp
@@ -49,6 +49,8 @@
   std::map ModuleIncludes;
   /// Tracks where inclusions that enter modules (in a module build) are found.
   std::map ModuleEntryIncludes;
+  /// Tracks where #if and #elif directives get evaluated and whether to true.
+  std::map IfConditions;
   /// Used transitively for building up the FileIncludes mapping over the
   /// various \c PPCallbacks callbacks.
   SourceLocation LastInclusionLocation;
@@ -78,6 +80,10 @@
   StringRef SearchPath, StringRef RelativePath,
   const Module *Imported,
   SrcMgr::CharacteristicKind FileType) override;
+  void If(SourceLocation Loc, SourceRange ConditionRange,
+  ConditionValueKind ConditionValue) override;
+  void Elif(SourceLocation Loc, SourceRange ConditionRange,
+ConditionValueKind ConditionValue, SourceLocation IfLoc) override;
   void WriteLineInfo(StringRef Filename, int Line,
  SrcMgr::CharacteristicKind FileType,
  StringRef Extra = StringRef());
@@ -89,12 +95,10 @@
   void CommentOutDirective(Lexer , const Token ,
const MemoryBuffer , StringRef EOL,
unsigned , int );
-  bool HandleHasInclude(FileID FileId, Lexer ,
-const DirectoryLookup *Lookup, Token ,
-bool );
   const IncludedFile *FindIncludeAtLocation(SourceLocation Loc) const;
   const Module *FindModuleAtLocation(SourceLocation Loc) const;
   const Module *FindEnteredModule(SourceLocation Loc) const;
+  bool IsIfAtLocationTrue(SourceLocation Loc) const;
   StringRef NextIdentifierName(Lexer , Token );
 };
 
@@ -203,6 +207,23 @@
 LastInclusionLocation = HashLoc;
 }
 
+void InclusionRewriter::If(SourceLocation Loc, SourceRange ConditionRange,
+   ConditionValueKind ConditionValue) {
+  auto P = IfConditions.insert(
+  std::make_pair(Loc.getRawEncoding(), ConditionValue == CVK_True));
+  (void)P;
+  assert(P.second && "Unexpected revisitation of the same if directive");
+}
+
+void InclusionRewriter::Elif(SourceLocation Loc, SourceRange ConditionRange,
+ ConditionValueKind ConditionValue,
+ SourceLocation IfLoc) {
+  auto P = IfConditions.insert(
+  std::make_pair(Loc.getRawEncoding(), ConditionValue == CVK_True));
+  (void)P;
+  assert(P.second && "Unexpected revisitation of the same elif directive");
+}
+
 /// Simple lookup for a SourceLocation (specifically one denoting the hash in
 /// an inclusion directive) in the map of inclusion information, FileChanges.
 const InclusionRewriter::IncludedFile *
@@ -233,6 +254,13 @@
   return nullptr;
 }
 
+bool InclusionRewriter::IsIfAtLocationTrue(SourceLocation Loc) const {
+  const auto I = IfConditions.find(Loc.getRawEncoding());
+  if (I != IfConditions.end())
+return I->second;
+  return false;
+}
+
 /// Detect the likely line ending style of \p FromFile by examining the first
 /// newline found within it.
 static StringRef DetectEOL(const MemoryBuffer ) {
@@ -346,80 +374,6 @@
   return StringRef();
 }
 
-// Expand __has_include and __has_include_next if possible. If there's no
-// definitive answer return false.
-bool InclusionRewriter::HandleHasInclude(
-FileID FileId, Lexer , const DirectoryLookup *Lookup, Token ,
-bool ) {
-  // Lex the opening paren.
-  RawLex.LexFromRawLexer(Tok);
-  if (Tok.isNot(tok::l_paren))
-return false;
-
-  RawLex.LexFromRawLexer(Tok);
-
-  SmallString<128> FilenameBuffer;
-  StringRef Filename;
-  // Since the raw lexer doesn't give us angle_literals we have to parse them
-  // ourselves.
-  // FIXME: What to do if the file name is a macro?
-  if (Tok.is(tok::less)) {
-RawLex.LexFromRawLexer(Tok);
-
-FilenameBuffer += '<';
-do {
-  if (Tok.is(tok::eod)) // Sanity check.
-return false;
-
-  if (Tok.is(tok::raw_identifier))
-PP.LookUpIdentifierInfo(Tok);
-
-  // Get the string piece.
-  SmallVector TmpBuffer;
-  

[PATCH] D63508: make -frewrite-includes also rewrite conditions in #if/#elif

[Luboš Luňák via Phabricator via cfe-commits]

llunak added a comment.

In D63508#1671776 , @torarnv wrote:

> So, this will make `-frewrite-includes` do more work, to ensure that it not 
> only covers the "top level" `#include` or `__has_include` case, but also 
> `__has_include` in one or more levels of macros?
>
> Does that effectively means it needs to do full preprocessing?


This patch does not make -frewrite-includes do any more work. It already does a 
partial preprocessing run which evaluates all preprocessor directives. The 
patch just make better use of that information.

> Does it depend on whether `-E` is passed with `-frewrite-includes`?

You cannot pass -frewrite-includes without -E.

> Will this mean that cases like icecream and distcc that use 
> `-frewrite-includes` will have fully preprocessed sources to send to the 
> remote hosts (with correct line/column number when compilation fails), or 
> should they still pass `-D` options during the remote compilation?

The patch does not change anything about the usage. -frewrite-includes still 
only expands all #include directives and that's it.

> How does `ICECC_REMOTE_CPP` work in this case?

The same way as before. You preferably do nothing, which defaults to 
ICECC_REMOTE_CPP=1, which uses -frewrite-includes to send full source to the 
remote, which does full preprocessing and compilation and you get pretty much 
the same result as if compiling locally. Or you may go with ICECC_REMOTE_CPP=0, 
which will not do this and then you'll most likely get warnings caused by 
compiling already preprocessed source. As for ccache, I personally consider 
suggesting explicit passing of -frewrite-includes to the compilation as 
somewhat lame; if ccache requires it for the cpp run, it can silently add it 
under the hood the same way icecream does, or at least it can have it 
configurable the same as other options and not require messing with build 
options. I myself use ccache with CCACHE_DEPEND=1, which completely avoids any 
preprocessor usage.


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D63508/new/

https://reviews.llvm.org/D63508



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D63508: make -frewrite-includes also rewrite conditions in #if/#elif

[Luboš Luňák via Phabricator via cfe-commits]

llunak added a comment.

I've noticed that the Developer Policy says that it's actually allowed to 
commit patches without approval for parts "that you have contributed or 
maintain". Given that I'm the author of -rewrite-includes I take it that it's 
ok if I commit this if there are no further comments.


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D63508/new/

https://reviews.llvm.org/D63508



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D63508: make -frewrite-includes also rewrite conditions in #if/#elif

[Luboš Luňák via Phabricator via cfe-commits]

llunak updated this revision to Diff 220378.
llunak added a comment.

- updated to apply to current trunk
- changed misleading condition in a test


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D63508/new/

https://reviews.llvm.org/D63508

Files:
  clang/lib/Frontend/Rewrite/InclusionRewriter.cpp
  clang/test/Frontend/rewrite-includes-conditions.c
  clang/test/Frontend/rewrite-includes.c
  clang/test/Modules/preprocess-module.cpp

Index: clang/test/Modules/preprocess-module.cpp
===
--- clang/test/Modules/preprocess-module.cpp
+++ clang/test/Modules/preprocess-module.cpp
@@ -51,7 +51,7 @@
 // RUN: %clang_cc1 -fmodules -fmodule-file=%t/file.rewrite.pcm %s -I%t -verify -fno-modules-error-recovery -DFILE_REWRITE -DINCLUDE -I%S/Inputs/preprocess
 //
 // Check that we can preprocess this user of the .pcm file.
-// RUN: %clang_cc1 -fmodules -fmodule-file=%t/file.pcm %s -I%t -E -frewrite-imports -o %t/preprocess-module.ii
+// RUN: %clang_cc1 -fmodules -fmodule-file=%t/file.pcm %s -I%t -E -frewrite-imports -DFILE_REWRITE_FULL -o %t/preprocess-module.ii
 // RUN: %clang_cc1 -fmodules %t/preprocess-module.ii -verify -fno-modules-error-recovery -DFILE_REWRITE_FULL
 //
 // Check that language / header search options are ignored when preprocessing from a .pcm file.
Index: clang/test/Frontend/rewrite-includes.c
===
--- clang/test/Frontend/rewrite-includes.c
+++ clang/test/Frontend/rewrite-includes.c
@@ -110,12 +110,27 @@
 // CHECK-NEXT: {{^}}#endif /* expanded by -frewrite-includes */{{$}}
 // CHECK-NEXT: {{^}}# 22 "{{.*}}rewrite-includes.c"{{$}}
 // CHECK-NEXT: {{^}}# 1 "{{.*[/\\]Inputs(/|)}}rewrite-includes8.h" 1{{$}}
-// CHECK-NEXT: {{^}}#if (0)/*__has_include_next()*/{{$}}
-// CHECK-NEXT: {{^}}#elif (0)/*__has_include()*/{{$}}
+// CHECK-NEXT: {{^}}#if 0 /* disabled by -frewrite-includes */{{$}}
+// CHECK-NEXT: {{^}}#if __has_include_next(){{$}}
+// CHECK-NEXT: {{^}}#endif{{$}}
+// CHECK-NEXT: {{^}}#endif /* disabled by -frewrite-includes */{{$}}
+// CHECK-NEXT: {{^}}#if 0 /* evaluated by -frewrite-includes */{{$}}
+// CHECK-NEXT: {{^}}# 2 "{{.*[/\\]Inputs(/|)}}rewrite-includes8.h"{{$}}
+// CHECK-NEXT: {{^}}#if 0 /* disabled by -frewrite-includes */{{$}}
+// CHECK-NEXT: {{^}}#if 0{{$}}
+// CHECK-NEXT: {{^}}#elif __has_include(){{$}}
+// CHECK-NEXT: {{^}}#endif{{$}}
+// CHECK-NEXT: {{^}}#endif /* disabled by -frewrite-includes */{{$}}
+// CHECK-NEXT: {{^}}#elif 0 /* evaluated by -frewrite-includes */{{$}}
 // CHECK-NEXT: {{^}}# 3 "{{.*[/\\]Inputs(/|)}}rewrite-includes8.h"{{$}}
 // CHECK-NEXT: {{^}}#endif{{$}}
 // CHECK-NEXT: {{^}}# 4 "{{.*[/\\]Inputs(/|)}}rewrite-includes8.h"{{$}}
-// CHECK-NEXT: {{^}}#if !(1)/*__has_include("rewrite-includes8.h")*/{{$}}
+// CHECK-NEXT: {{^}}#if 0 /* disabled by -frewrite-includes */{{$}}
+// CHECK-NEXT: {{^}}#if !__has_include("rewrite-includes8.h"){{$}}
+// CHECK-NEXT: {{^}}#endif{{$}}
+// CHECK-NEXT: {{^}}#endif /* disabled by -frewrite-includes */{{$}}
+// CHECK-NEXT: {{^}}#if 0 /* evaluated by -frewrite-includes */{{$}}
+// CHECK-NEXT: {{^}}# 5 "{{.*[/\\]Inputs(/|)}}rewrite-includes8.h"{{$}}
 // CHECK-NEXT: {{^}}#endif{{$}}
 // CHECK-NEXT: {{^}}# 6 "{{.*[/\\]Inputs(/|)}}rewrite-includes8.h"{{$}}
 // CHECK-NEXT: {{^}}# 23 "{{.*}}rewrite-includes.c" 2{{$}}
@@ -124,7 +139,12 @@
 // CHECK-NEXT: {{^}}#endif /* expanded by -frewrite-includes */{{$}}
 // CHECK-NEXT: {{^}}# 23 "{{.*}}rewrite-includes.c"{{$}}
 // CHECK-NEXT: {{^}}# 1 "{{.*[/\\]Inputs(/|)}}rewrite-includes9.h" 1{{$}}
-// CHECK-NEXT: {{^}}#if (1)/*__has_include_next()*/{{$}}
+// CHECK-NEXT: {{^}}#if 0 /* disabled by -frewrite-includes */{{$}}
+// CHECK-NEXT: {{^}}#if __has_include_next(){{$}}
+// CHECK-NEXT: {{^}}#endif{{$}}
+// CHECK-NEXT: {{^}}#endif /* disabled by -frewrite-includes */{{$}}
+// CHECK-NEXT: {{^}}#if 1 /* evaluated by -frewrite-includes */{{$}}
+// CHECK-NEXT: {{^}}# 2 "{{.*[/\\]Inputs(/|)}}rewrite-includes9.h"{{$}}
 // CHECK-NEXT: {{^}}#if 0 /* expanded by -frewrite-includes */{{$}}
 // CHECK-NEXT: {{^}}#include_next {{$}}
 // CHECK-NEXT: {{^}}#endif /* expanded by -frewrite-includes */{{$}}
@@ -193,15 +213,32 @@
 // CHECKNL-NEXT: {{^}}#if 0 /* expanded by -frewrite-includes */{{$}}
 // CHECKNL-NEXT: {{^}}#include "rewrite-includes8.h"{{$}}
 // CHECKNL-NEXT: {{^}}#endif /* expanded by -frewrite-includes */{{$}}
-// CHECKNL-NEXT: {{^}}#if (0)/*__has_include_next()*/{{$}}
-// CHECKNL-NEXT: {{^}}#elif (0)/*__has_include()*/{{$}}
+// CHECKNL-NEXT: {{^}}#if 0 /* disabled by -frewrite-includes */{{$}}
+// CHECKNL-NEXT: {{^}}#if __has_include_next(){{$}}
 // CHECKNL-NEXT: {{^}}#endif{{$}}
-// CHECKNL-NEXT: {{^}}#if !(1)/*__has_include("rewrite-includes8.h")*/{{$}}
+// CHECKNL-NEXT: {{^}}#endif /* disabled by -frewrite-includes */{{$}}
+// CHECKNL-NEXT: {{^}}#if 0 /* evaluated by -frewrite-includes */{{$}}
+// CHECKNL-NEXT: 

[PATCH] D65371: do not emit -Wunused-macros warnings in -frewrite-includes mode (PR15614)

[Luboš Luňák via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGa507a5ec8f1d: do not emit -Wunused-macros warnings in 
-frewrite-includes mode (PR15614) (authored by llunak).

Changed prior to commit:
  https://reviews.llvm.org/D65371?vs=219370=220370#toc

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D65371/new/

https://reviews.llvm.org/D65371

Files:
  clang/lib/Lex/PPDirectives.cpp
  clang/test/Frontend/rewrite-includes-warnings.c


Index: clang/test/Frontend/rewrite-includes-warnings.c
===
--- clang/test/Frontend/rewrite-includes-warnings.c
+++ clang/test/Frontend/rewrite-includes-warnings.c
@@ -1,4 +1,7 @@
-// RUN: %clang_cc1 -verify -Wall -Wextra -E -frewrite-includes %s
+// RUN: %clang_cc1 -verify -Wall -Wextra -Wunused-macros -E -frewrite-includes 
%s
 // expected-no-diagnostics
 
 #pragma GCC visibility push (default)
+
+#define USED_MACRO 1
+int test() { return USED_MACRO; }
Index: clang/lib/Lex/PPDirectives.cpp
===
--- clang/lib/Lex/PPDirectives.cpp
+++ clang/lib/Lex/PPDirectives.cpp
@@ -2782,7 +2782,8 @@
   // If we need warning for not using the macro, add its location in the
   // warn-because-unused-macro set. If it gets used it will be removed from 
set.
   if (getSourceManager().isInMainFile(MI->getDefinitionLoc()) &&
-  !Diags->isIgnored(diag::pp_macro_not_used, MI->getDefinitionLoc())) {
+  !Diags->isIgnored(diag::pp_macro_not_used, MI->getDefinitionLoc()) &&
+  !MacroExpansionInDirectivesOverride) {
 MI->setIsWarnIfUnused(true);
 WarnUnusedMacroLocs.insert(MI->getDefinitionLoc());
   }


Index: clang/test/Frontend/rewrite-includes-warnings.c
===
--- clang/test/Frontend/rewrite-includes-warnings.c
+++ clang/test/Frontend/rewrite-includes-warnings.c
@@ -1,4 +1,7 @@
-// RUN: %clang_cc1 -verify -Wall -Wextra -E -frewrite-includes %s
+// RUN: %clang_cc1 -verify -Wall -Wextra -Wunused-macros -E -frewrite-includes %s
 // expected-no-diagnostics
 
 #pragma GCC visibility push (default)
+
+#define USED_MACRO 1
+int test() { return USED_MACRO; }
Index: clang/lib/Lex/PPDirectives.cpp
===
--- clang/lib/Lex/PPDirectives.cpp
+++ clang/lib/Lex/PPDirectives.cpp
@@ -2782,7 +2782,8 @@
   // If we need warning for not using the macro, add its location in the
   // warn-because-unused-macro set. If it gets used it will be removed from set.
   if (getSourceManager().isInMainFile(MI->getDefinitionLoc()) &&
-  !Diags->isIgnored(diag::pp_macro_not_used, MI->getDefinitionLoc())) {
+  !Diags->isIgnored(diag::pp_macro_not_used, MI->getDefinitionLoc()) &&
+  !MacroExpansionInDirectivesOverride) {
 MI->setIsWarnIfUnused(true);
 WarnUnusedMacroLocs.insert(MI->getDefinitionLoc());
   }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D65371: do not emit -Wunused-macros warnings in -frewrite-includes mode (PR15614)

[Luboš Luňák via Phabricator via cfe-commits]

llunak added a comment.

In D65371#1659929 , @dblaikie wrote:

> A test case would be good (in the clang/test directory - probably near/in the 
> other tests for -frewrite-includes)


Done.

> And does the same bug occur for other preprocessor-related warnings? Maybe 
> it's not practical to disable them all this way & there should be a different 
> solution? (or maybe we shouldn't fix these and users can pass -w to disable 
> warnings when using -frewrite-includes?)

I've never seen any other warning from -frewrite-includes besides 
-Wunused-macros. Given that I use and more or less maintain Icecream, which 
uses -frewrite-includes for distributed builds, I'd say any other warnings 
there either don't exist or are very rare (which rather makes sense, given that 
-frewrite-includes only does limited macro expansion when analysing the input 
and that's about it). Given that, I'd prefer not to disable warnings globally - 
they are unlikely to show up, if they do, they can hopefully be handled 
individually, but on the other hand maybe Clang one day gets warnings about 
#include or similar that would be a pity to miss when using -rewrite-includes.


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D65371/new/

https://reviews.llvm.org/D65371



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D65371: do not emit -Wunused-macros warnings in -frewrite-includes mode (PR15614)

[Luboš Luňák via Phabricator via cfe-commits]

llunak updated this revision to Diff 219370.
llunak added a comment.

Added a test.


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D65371/new/

https://reviews.llvm.org/D65371

Files:
  clang/lib/Lex/PPDirectives.cpp
  clang/test/Frontend/rewrite-includes-warnings.c


Index: clang/test/Frontend/rewrite-includes-warnings.c
===
--- clang/test/Frontend/rewrite-includes-warnings.c
+++ clang/test/Frontend/rewrite-includes-warnings.c
@@ -1,4 +1,6 @@
-// RUN: %clang_cc1 -verify -Wall -Wextra -E -frewrite-includes %s
+// RUN: %clang_cc1 -verify -Wall -Wextra -Wunused-macros -E -frewrite-includes 
%s
 // expected-no-diagnostics
 
 #pragma GCC visibility push (default)
+
+#define UNUSED
Index: clang/lib/Lex/PPDirectives.cpp
===
--- clang/lib/Lex/PPDirectives.cpp
+++ clang/lib/Lex/PPDirectives.cpp
@@ -2723,7 +2723,8 @@
   // If we need warning for not using the macro, add its location in the
   // warn-because-unused-macro set. If it gets used it will be removed from 
set.
   if (getSourceManager().isInMainFile(MI->getDefinitionLoc()) &&
-  !Diags->isIgnored(diag::pp_macro_not_used, MI->getDefinitionLoc())) {
+  !Diags->isIgnored(diag::pp_macro_not_used, MI->getDefinitionLoc()) &&
+  !MacroExpansionInDirectivesOverride) {
 MI->setIsWarnIfUnused(true);
 WarnUnusedMacroLocs.insert(MI->getDefinitionLoc());
   }


Index: clang/test/Frontend/rewrite-includes-warnings.c
===
--- clang/test/Frontend/rewrite-includes-warnings.c
+++ clang/test/Frontend/rewrite-includes-warnings.c
@@ -1,4 +1,6 @@
-// RUN: %clang_cc1 -verify -Wall -Wextra -E -frewrite-includes %s
+// RUN: %clang_cc1 -verify -Wall -Wextra -Wunused-macros -E -frewrite-includes %s
 // expected-no-diagnostics
 
 #pragma GCC visibility push (default)
+
+#define UNUSED
Index: clang/lib/Lex/PPDirectives.cpp
===
--- clang/lib/Lex/PPDirectives.cpp
+++ clang/lib/Lex/PPDirectives.cpp
@@ -2723,7 +2723,8 @@
   // If we need warning for not using the macro, add its location in the
   // warn-because-unused-macro set. If it gets used it will be removed from set.
   if (getSourceManager().isInMainFile(MI->getDefinitionLoc()) &&
-  !Diags->isIgnored(diag::pp_macro_not_used, MI->getDefinitionLoc())) {
+  !Diags->isIgnored(diag::pp_macro_not_used, MI->getDefinitionLoc()) &&
+  !MacroExpansionInDirectivesOverride) {
 MI->setIsWarnIfUnused(true);
 WarnUnusedMacroLocs.insert(MI->getDefinitionLoc());
   }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D65371: do not emit -Wunused-macros warnings in -frewrite-includes mode (PR15614)

[Luboš Luňák via Phabricator via cfe-commits]

llunak added a comment.

Ping?


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D65371/new/

https://reviews.llvm.org/D65371



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D63508: make -frewrite-includes also rewrite conditions in #if/#elif

[Luboš Luňák via Phabricator via cfe-commits]

llunak added a comment.

Ping?


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D63508/new/

https://reviews.llvm.org/D63508



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D65371: do not emit -Wunused-macros warnings in -frewrite-includes mode (PR15614)

[Luboš Luňák via Phabricator via cfe-commits]

llunak created this revision.
llunak added a reviewer: rsmith.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

This is an updated patch from https://bugs.llvm.org/show_bug.cgi?id=15614. See 
there for testcase etc.


Repository:
  rC Clang

https://reviews.llvm.org/D65371

Files:
  clang/lib/Lex/PPDirectives.cpp


Index: clang/lib/Lex/PPDirectives.cpp
===
--- clang/lib/Lex/PPDirectives.cpp
+++ clang/lib/Lex/PPDirectives.cpp
@@ -2723,7 +2723,8 @@
   // If we need warning for not using the macro, add its location in the
   // warn-because-unused-macro set. If it gets used it will be removed from 
set.
   if (getSourceManager().isInMainFile(MI->getDefinitionLoc()) &&
-  !Diags->isIgnored(diag::pp_macro_not_used, MI->getDefinitionLoc())) {
+  !Diags->isIgnored(diag::pp_macro_not_used, MI->getDefinitionLoc()) &&
+  !MacroExpansionInDirectivesOverride) {
 MI->setIsWarnIfUnused(true);
 WarnUnusedMacroLocs.insert(MI->getDefinitionLoc());
   }


Index: clang/lib/Lex/PPDirectives.cpp
===
--- clang/lib/Lex/PPDirectives.cpp
+++ clang/lib/Lex/PPDirectives.cpp
@@ -2723,7 +2723,8 @@
   // If we need warning for not using the macro, add its location in the
   // warn-because-unused-macro set. If it gets used it will be removed from set.
   if (getSourceManager().isInMainFile(MI->getDefinitionLoc()) &&
-  !Diags->isIgnored(diag::pp_macro_not_used, MI->getDefinitionLoc())) {
+  !Diags->isIgnored(diag::pp_macro_not_used, MI->getDefinitionLoc()) &&
+  !MacroExpansionInDirectivesOverride) {
 MI->setIsWarnIfUnused(true);
 WarnUnusedMacroLocs.insert(MI->getDefinitionLoc());
   }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D63508: make -frewrite-includes also rewrite conditions in #if/#elif

[Luboš Luňák via Phabricator via cfe-commits]

llunak added inline comments.



Comment at: clang/test/Frontend/rewrite-includes-conditions.c:17
+line4
+#elif value2 < value2
+line5

rsmith wrote:
> Did you mean for this to be `value1 < value2` rather than `value2 < value2`?
Yes, not that it'd matter in practice. I'll fix that in the next iteration of 
the patch.




Comment at: clang/test/Frontend/rewrite-includes-conditions.c:54-67
+// CHECK: #if 0 /* disabled by -frewrite-includes */
+// CHECK-NEXT: #if value1 == value2
+// CHECK-NEXT: #endif
+// CHECK-NEXT: #endif /* disabled by -frewrite-includes */
+// CHECK-NEXT: #if 0 /* evaluated by -frewrite-includes */
+// CHECK-NEXT: # 14 "{{.*}}rewrite-includes-conditions.c"
+

rsmith wrote:
> I find this output pretty hard to read -- it's hard to tell what's an 
> original `#if` / `#endif` that's been effectively commented out, and what's 
> an added `#if` / `#endif` that's doing the commenting.
> 
> What do you think about modifying the original line so it's no longer 
> recognized as a preprocessing directive? For instance, instead of:
> 
> ```
> #if 0 /* disabled by -frewrite-includes */
> #if value1 == value2
> #endif
> #endif /* disabled by -frewrite-includes */
> #if 0 /* evaluated by -frewrite-includes */
> # 14 "{{.*}}rewrite-includes-conditions.c"
> line3
> #if 0 /* disabled by -frewrite-includes */
> #if 0
> #elif value1 > value2
> #endif
> #endif /* disabled by -frewrite-includes */
> #elif 0 /* evaluated by -frewrite-includes */
> # 16 "{{.*}}rewrite-includes-conditions.c"
> line4
> #if 0 /* disabled by -frewrite-includes */
> #if 0
> #elif value1 < value2
> #endif
> #endif /* disabled by -frewrite-includes */
> #elif 1 /* evaluated by -frewrite-includes */
> # 18 "{{.*}}rewrite-includes-conditions.c"
> line5
> [...]
> ```
> 
> you might produce
> 
> ```
> #if 0 /* rewritten by -frewrite-includes */
> !#if value1 == value2
> #endif
> #if 0 /* evaluated by -frewrite-includes */
> # 14 "{{.*}}rewrite-includes-conditions.c"
> line3
> #if 0 /* rewritten by -frewrite-includes */
> !#elif value1 > value2
> #endif
> #elif 0 /* evaluated by -frewrite-includes */
> # 16 "{{.*}}rewrite-includes-conditions.c"
> line4
> #if 0 /* rewritten by -frewrite-includes */
> !#elif value1 < value2
> #endif
> #elif 1 /* evaluated by -frewrite-includes */
> # 18 "{{.*}}rewrite-includes-conditions.c"
> line5
> [...]
> ```
> 
> (or whatever transformation you like that prevents recognition of a 
> `#if`/`#elif` within the `#if 0` block).
> 
> Also, maybe we could move the quoted directives inside their respective `#if` 
> / `#elif` block, and only comment them out if the block was entered:
> 
> ```
> #if 0 /* evaluated by -frewrite-includes */
> was: #if value1 == value2
> # 14 "{{.*}}rewrite-includes-conditions.c"
> line3
> #elif 0 /* evaluated by -frewrite-includes */
> was: #elif value1 > value2
> # 16 "{{.*}}rewrite-includes-conditions.c"
> line4
> #elif 1 /* evaluated by -frewrite-includes */
> #if 0
> was: #elif value1 < value2
> #endif
> # 18 "{{.*}}rewrite-includes-conditions.c"
> line5
> [...]
> ```
I think that's a matter of taste/opinion. I find your proposal visually harder 
to read, because although it saves one line, it also is incorrect syntax, it 
visually breaks if-endif nesting and is inconsistent with how 
-frewrite-includes does other rewriting. Moreover I think this will be so 
rarely examined by a human that it doesn't really matter. So unless you insist 
I'd prefer to keep it as it is.

BTW, the other related review I linked above, is it enough to mention it here, 
or should I do something more about it? I'm not sure how to pick reviewers if 
there's not an obvious one.


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D63508/new/

https://reviews.llvm.org/D63508



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D64284: (WIP) share template instantiations from PCH in one .o if -building-pch-with-obj

[Luboš Luňák via Phabricator via cfe-commits]

llunak created this revision.
llunak added reviewers: rsmith, dblaikie, hans.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

This is a WIP patch that can significantly reduce build time by avoiding 
needlessly repeatedly instantiating templates when using PCH.

As described in http://lists.llvm.org/pipermail/cfe-dev/2019-May/062371.html / 
http://llunak.blogspot.com/2019/05/why-precompiled-headers-do-not-improve.html 
, whenever a compilation uses PCH it instantiates every template that's been 
instantiated in the PCH, which can add significant build time. This patch 
allows avoiding that, if -building-pch-with-obj option is used, the templates 
will be instantiated just once in the object file that should accompany the 
PCH, and then it can be skipped during normal compilations.

Testcase: I've built an entire debug build of LibreOffice with this patch and 
it works.

Pending problems/questions:

- The patch currently skips C++ constructors and destructors, because (at least 
with the Itanium ABI) they can be emitted in several variants (complete vs base 
ctor/dtor) and I don't know how to force emitting all variants. Ctors/dtors are 
a significant portion of PCH template instantiations, so without this the 
building time improvement is not as large as it could be.

- The patch currently does not handle function inlining well (=it works only 
with -O0 builds). This optimization cannot be done with template functions that 
should be inlined by the compilation, so the code needs to check whether a 
function would possibly be inlined. The problem is that this depends on 
settings in CodeGenOptions and Sema (understandably) doesn't have an access to 
that. How can I access that in Sema? Does it make sense to try to handle this, 
or is the deciding whether a function will get actually inlined too complex and 
this should rather be skipped for CodeGen::NormalInlining?

- In rate circumstances it is possible to get undefined references because of 
instantiations in the PCH's object file. Specifically this happens if the PCH 
includes a header providing non-exported code from another module (=shared 
library), in that case the PCH may refer to it. I think this is conceptually 
not possible to handle in Clang, either the code should be cleaned up to not 
provide non-exported code to other modules, or it's possible to use linker's 
--gc-sections to drop such instantiations.

- In Sema::InstantiateFunctionDefinition() the code for extern templates still 
instantiates a function if it has getContainedAutoType(), so my code should 
probably also check that. But I'm not sure what that is (is that 'auto foo() { 
return 1; }' ?) or why that would need an instance in every TU.

- Is there a guideline on when to use data members in class Decl vs functions? 
The patch uses DeclIsPendingInstantiationFromPCHWithObjectFile() to check 
whether a FunctionDecl comes from the PCH, which may get called quite often. 
This could be optimized away by storing a flag in class Decl.


Repository:
  rC Clang

https://reviews.llvm.org/D64284

Files:
  clang/include/clang/AST/ExternalASTSource.h
  clang/include/clang/Sema/MultiplexExternalSemaSource.h
  clang/include/clang/Serialization/ASTReader.h
  clang/lib/AST/ASTContext.cpp
  clang/lib/Sema/MultiplexExternalSemaSource.cpp
  clang/lib/Sema/Sema.cpp
  clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
  clang/lib/Serialization/ASTReader.cpp

Index: clang/lib/Serialization/ASTReader.cpp
===
--- clang/lib/Serialization/ASTReader.cpp
+++ clang/lib/Serialization/ASTReader.cpp
@@ -8293,14 +8293,14 @@
 
 void ASTReader::ReadPendingInstantiations(
SmallVectorImpl> ) {
-  for (unsigned Idx = 0, N = PendingInstantiations.size(); Idx < N;) {
+  for (unsigned Idx = PendingInstantiationsReadCount, N = PendingInstantiations.size(); Idx < N;) {
 ValueDecl *D = cast(GetDecl(PendingInstantiations[Idx++]));
 SourceLocation Loc
   = SourceLocation::getFromRawEncoding(PendingInstantiations[Idx++]);
 
 Pending.push_back(std::make_pair(D, Loc));
   }
-  PendingInstantiations.clear();
+  PendingInstantiationsReadCount = PendingInstantiations.size();
 }
 
 void ASTReader::ReadLateParsedTemplates(
@@ -8522,6 +8522,17 @@
   return MF && MF->PCHHasObjectFile;
 }
 
+bool ASTReader::DeclIsPendingInstantiationFromPCHWithObjectFile(const Decl *D) {
+  if( !DeclIsFromPCHWithObjectFile(D))
+return false;
+  for (unsigned Idx = 0, N = PendingInstantiations.size(); Idx < N;) {
+if( D == cast(GetDecl(PendingInstantiations[Idx++])))
+return true;
+Idx++;
+  }
+  return false;
+}
+
 ModuleFile *ASTReader::getLocalModuleFile(ModuleFile , unsigned ID) {
   if (ID & 1) {
 // It's a module, look it up by submodule ID.
Index: clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
===
--- clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
+++ 

[PATCH] D63508: make -frewrite-includes also rewrite conditions in #if/#elif

[Luboš Luňák via Phabricator via cfe-commits]

llunak updated this revision to Diff 207229.
llunak added a comment.

Updated the patch again, commenting out just a part of #if didn't work either, 
so it seems there's no good way to comment out unwanted #if. This patch 
surrounds rewritten #if conditions by extra #if 0 #endif block to disable them, 
moreover it makes the conditions be part of an empty #if #endif block, so they 
do not break nesting.

I've also created https://reviews.llvm.org/D63979 to verify that the results in 
fact do compile.


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D63508/new/

https://reviews.llvm.org/D63508

Files:
  clang/lib/Frontend/Rewrite/InclusionRewriter.cpp
  clang/test/Frontend/rewrite-includes-conditions.c
  clang/test/Frontend/rewrite-includes.c
  clang/test/Modules/preprocess-module.cpp

Index: clang/test/Modules/preprocess-module.cpp
===
--- clang/test/Modules/preprocess-module.cpp
+++ clang/test/Modules/preprocess-module.cpp
@@ -51,7 +51,7 @@
 // RUN: %clang_cc1 -fmodules -fmodule-file=%t/file.rewrite.pcm %s -I%t -verify -fno-modules-error-recovery -DFILE_REWRITE -DINCLUDE -I%S/Inputs/preprocess
 //
 // Check that we can preprocess this user of the .pcm file.
-// RUN: %clang_cc1 -fmodules -fmodule-file=%t/file.pcm %s -I%t -E -frewrite-imports -o %t/preprocess-module.ii
+// RUN: %clang_cc1 -fmodules -fmodule-file=%t/file.pcm %s -I%t -E -frewrite-imports -DFILE_REWRITE_FULL -o %t/preprocess-module.ii
 // RUN: %clang_cc1 -fmodules %t/preprocess-module.ii -verify -fno-modules-error-recovery -DFILE_REWRITE_FULL
 //
 // Check that language / header search options are ignored when preprocessing from a .pcm file.
Index: clang/test/Frontend/rewrite-includes.c
===
--- clang/test/Frontend/rewrite-includes.c
+++ clang/test/Frontend/rewrite-includes.c
@@ -110,12 +110,27 @@
 // CHECK-NEXT: {{^}}#endif /* expanded by -frewrite-includes */{{$}}
 // CHECK-NEXT: {{^}}# 22 "{{.*}}rewrite-includes.c"{{$}}
 // CHECK-NEXT: {{^}}# 1 "{{.*[/\\]Inputs(/|)}}rewrite-includes8.h" 1{{$}}
-// CHECK-NEXT: {{^}}#if (0)/*__has_include_next()*/{{$}}
-// CHECK-NEXT: {{^}}#elif (0)/*__has_include()*/{{$}}
+// CHECK-NEXT: {{^}}#if 0 /* disabled by -frewrite-includes */{{$}}
+// CHECK-NEXT: {{^}}#if __has_include_next(){{$}}
+// CHECK-NEXT: {{^}}#endif{{$}}
+// CHECK-NEXT: {{^}}#endif /* disabled by -frewrite-includes */{{$}}
+// CHECK-NEXT: {{^}}#if 0 /* evaluated by -frewrite-includes */{{$}}
+// CHECK-NEXT: {{^}}# 2 "{{.*[/\\]Inputs(/|)}}rewrite-includes8.h"{{$}}
+// CHECK-NEXT: {{^}}#if 0 /* disabled by -frewrite-includes */{{$}}
+// CHECK-NEXT: {{^}}#if 0{{$}}
+// CHECK-NEXT: {{^}}#elif __has_include(){{$}}
+// CHECK-NEXT: {{^}}#endif{{$}}
+// CHECK-NEXT: {{^}}#endif /* disabled by -frewrite-includes */{{$}}
+// CHECK-NEXT: {{^}}#elif 0 /* evaluated by -frewrite-includes */{{$}}
 // CHECK-NEXT: {{^}}# 3 "{{.*[/\\]Inputs(/|)}}rewrite-includes8.h"{{$}}
 // CHECK-NEXT: {{^}}#endif{{$}}
 // CHECK-NEXT: {{^}}# 4 "{{.*[/\\]Inputs(/|)}}rewrite-includes8.h"{{$}}
-// CHECK-NEXT: {{^}}#if !(1)/*__has_include("rewrite-includes8.h")*/{{$}}
+// CHECK-NEXT: {{^}}#if 0 /* disabled by -frewrite-includes */{{$}}
+// CHECK-NEXT: {{^}}#if !__has_include("rewrite-includes8.h"){{$}}
+// CHECK-NEXT: {{^}}#endif{{$}}
+// CHECK-NEXT: {{^}}#endif /* disabled by -frewrite-includes */{{$}}
+// CHECK-NEXT: {{^}}#if 0 /* evaluated by -frewrite-includes */{{$}}
+// CHECK-NEXT: {{^}}# 5 "{{.*[/\\]Inputs(/|)}}rewrite-includes8.h"{{$}}
 // CHECK-NEXT: {{^}}#endif{{$}}
 // CHECK-NEXT: {{^}}# 6 "{{.*[/\\]Inputs(/|)}}rewrite-includes8.h"{{$}}
 // CHECK-NEXT: {{^}}# 23 "{{.*}}rewrite-includes.c" 2{{$}}
@@ -124,7 +139,12 @@
 // CHECK-NEXT: {{^}}#endif /* expanded by -frewrite-includes */{{$}}
 // CHECK-NEXT: {{^}}# 23 "{{.*}}rewrite-includes.c"{{$}}
 // CHECK-NEXT: {{^}}# 1 "{{.*[/\\]Inputs(/|)}}rewrite-includes9.h" 1{{$}}
-// CHECK-NEXT: {{^}}#if (1)/*__has_include_next()*/{{$}}
+// CHECK-NEXT: {{^}}#if 0 /* disabled by -frewrite-includes */{{$}}
+// CHECK-NEXT: {{^}}#if __has_include_next(){{$}}
+// CHECK-NEXT: {{^}}#endif{{$}}
+// CHECK-NEXT: {{^}}#endif /* disabled by -frewrite-includes */{{$}}
+// CHECK-NEXT: {{^}}#if 1 /* evaluated by -frewrite-includes */{{$}}
+// CHECK-NEXT: {{^}}# 2 "{{.*[/\\]Inputs(/|)}}rewrite-includes9.h"{{$}}
 // CHECK-NEXT: {{^}}#if 0 /* expanded by -frewrite-includes */{{$}}
 // CHECK-NEXT: {{^}}#include_next {{$}}
 // CHECK-NEXT: {{^}}#endif /* expanded by -frewrite-includes */{{$}}
@@ -193,15 +213,32 @@
 // CHECKNL-NEXT: {{^}}#if 0 /* expanded by -frewrite-includes */{{$}}
 // CHECKNL-NEXT: {{^}}#include "rewrite-includes8.h"{{$}}
 // CHECKNL-NEXT: {{^}}#endif /* expanded by -frewrite-includes */{{$}}
-// CHECKNL-NEXT: {{^}}#if (0)/*__has_include_next()*/{{$}}
-// CHECKNL-NEXT: {{^}}#elif (0)/*__has_include()*/{{$}}
+// CHECKNL-NEXT: {{^}}#if 0 /* disabled