llvmbot wrote:

<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-clang-codegen

Author: Jeaye Wilkerson (jeaye)

<details>
<summary>Changes</summary>

This is an updated version of @<!-- -->vgvassilev's PR from last year here: 
https://github.com/llvm/llvm-project/pull/94166

In short, it includes:

1. The fix for a blocking issue where `clang::Interpreter` cannot resolve 
symbols defined in a PCH
2. A test to prove this is working
3. A new hidden flag for `clang-repl` so that `llvm-lit` can match the host JIT 
triple between the PCH and `clang-repl`; previously, they may differ in some 
cases
4. Everything based on the latest LLVM main

---
Full diff: https://github.com/llvm/llvm-project/pull/157359.diff


10 Files Affected:

- (modified) clang/include/clang/CodeGen/ModuleBuilder.h (+6) 
- (modified) clang/lib/CodeGen/BackendConsumer.h (-5) 
- (modified) clang/lib/CodeGen/CodeGenAction.cpp (+6-7) 
- (modified) clang/lib/CodeGen/ModuleBuilder.cpp (+9-1) 
- (modified) clang/lib/Interpreter/IncrementalAction.cpp (+2-1) 
- (modified) clang/lib/Interpreter/IncrementalParser.cpp (+4) 
- (modified) clang/lib/Interpreter/Interpreter.cpp (+4-3) 
- (added) clang/test/Interpreter/execute-pch.cpp (+23) 
- (modified) clang/test/lit.cfg.py (+4) 
- (modified) clang/tools/clang-repl/ClangRepl.cpp (+7) 


``````````diff
diff --git a/clang/include/clang/CodeGen/ModuleBuilder.h 
b/clang/include/clang/CodeGen/ModuleBuilder.h
index 59b9840d02e08..f1b8229edd362 100644
--- a/clang/include/clang/CodeGen/ModuleBuilder.h
+++ b/clang/include/clang/CodeGen/ModuleBuilder.h
@@ -52,6 +52,12 @@ namespace CodeGen {
 class CodeGenerator : public ASTConsumer {
   virtual void anchor();
 
+protected:
+  /// True if we've finished generating IR. This prevents us from generating
+  /// additional LLVM IR after emitting output in HandleTranslationUnit. This
+  /// can happen when Clang plugins trigger additional AST deserialization.
+  bool IRGenFinished = false;
+
 public:
   /// Return an opaque reference to the CodeGenModule object, which can
   /// be used in various secondary APIs.  It is valid as long as the
diff --git a/clang/lib/CodeGen/BackendConsumer.h 
b/clang/lib/CodeGen/BackendConsumer.h
index ad3adfca36785..b7bbb81074836 100644
--- a/clang/lib/CodeGen/BackendConsumer.h
+++ b/clang/lib/CodeGen/BackendConsumer.h
@@ -40,11 +40,6 @@ class BackendConsumer : public ASTConsumer {
   llvm::Timer LLVMIRGeneration;
   unsigned LLVMIRGenerationRefCount = 0;
 
-  /// True if we've finished generating IR. This prevents us from generating
-  /// additional LLVM IR after emitting output in HandleTranslationUnit. This
-  /// can happen when Clang plugins trigger additional AST deserialization.
-  bool IRGenFinished = false;
-
   bool TimerIsEnabled = false;
 
   BackendAction Action;
diff --git a/clang/lib/CodeGen/CodeGenAction.cpp 
b/clang/lib/CodeGen/CodeGenAction.cpp
index dc54c97eeae8e..db38351118f26 100644
--- a/clang/lib/CodeGen/CodeGenAction.cpp
+++ b/clang/lib/CodeGen/CodeGenAction.cpp
@@ -189,9 +189,7 @@ void 
BackendConsumer::HandleInlineFunctionDefinition(FunctionDecl *D) {
 }
 
 void BackendConsumer::HandleInterestingDecl(DeclGroupRef D) {
-  // Ignore interesting decls from the AST reader after IRGen is finished.
-  if (!IRGenFinished)
-    HandleTopLevelDecl(D);
+  HandleTopLevelDecl(D);
 }
 
 // Links each entry in LinkModules into our module. Returns true on error.
@@ -240,10 +238,11 @@ void BackendConsumer::HandleTranslationUnit(ASTContext 
&C) {
 
     Gen->HandleTranslationUnit(C);
 
-    if (TimerIsEnabled && !--LLVMIRGenerationRefCount)
-      LLVMIRGeneration.yieldTo(CI.getFrontendTimer());
-
-    IRGenFinished = true;
+    if (TimerIsEnabled) {
+      LLVMIRGenerationRefCount -= 1;
+      if (LLVMIRGenerationRefCount == 0)
+        LLVMIRGeneration.stopTimer();
+    }
   }
 
   // Silently ignore if we weren't initialized for some reason.
diff --git a/clang/lib/CodeGen/ModuleBuilder.cpp 
b/clang/lib/CodeGen/ModuleBuilder.cpp
index 8c1fee8c974f1..3eb0595864c6c 100644
--- a/clang/lib/CodeGen/ModuleBuilder.cpp
+++ b/clang/lib/CodeGen/ModuleBuilder.cpp
@@ -138,6 +138,8 @@ namespace {
       assert(!M && "Replacing existing Module?");
       M.reset(new llvm::Module(ExpandModuleName(ModuleName, CodeGenOpts), C));
 
+      IRGenFinished = false;
+
       std::unique_ptr<CodeGenModule> OldBuilder = std::move(Builder);
 
       Initialize(*Ctx);
@@ -179,6 +181,10 @@ namespace {
     }
 
     bool HandleTopLevelDecl(DeclGroupRef DG) override {
+      // Ignore interesting decls from the AST reader after IRGen is finished.
+      if (IRGenFinished)
+        return true; // We can't CodeGen more but pass to other consumers.
+
       // FIXME: Why not return false and abort parsing?
       if (Diags.hasUnrecoverableErrorOccurred())
         return true;
@@ -282,6 +288,7 @@ namespace {
     }
 
     void HandleTranslationUnit(ASTContext &Ctx) override {
+
       // Release the Builder when there is no error.
       if (!Diags.hasUnrecoverableErrorOccurred() && Builder)
         Builder->Release();
@@ -292,8 +299,9 @@ namespace {
         if (Builder)
           Builder->clear();
         M.reset();
-        return;
       }
+
+      IRGenFinished = true;
     }
 
     void AssignInheritanceModel(CXXRecordDecl *RD) override {
diff --git a/clang/lib/Interpreter/IncrementalAction.cpp 
b/clang/lib/Interpreter/IncrementalAction.cpp
index 4d1bc4c59e851..3d489fce54bc6 100644
--- a/clang/lib/Interpreter/IncrementalAction.cpp
+++ b/clang/lib/Interpreter/IncrementalAction.cpp
@@ -106,7 +106,8 @@ std::unique_ptr<llvm::Module> 
IncrementalAction::GenModule() {
     // around we created an empty module to make CodeGen happy. We should make
     // sure it always stays empty.
     assert(((!CachedInCodeGenModule ||
-             !CI.getPreprocessorOpts().Includes.empty()) ||
+             !CI.getPreprocessorOpts().Includes.empty() ||
+             !CI.getPreprocessorOpts().ImplicitPCHInclude.empty()) ||
             (CachedInCodeGenModule->empty() &&
              CachedInCodeGenModule->global_empty() &&
              CachedInCodeGenModule->alias_empty() &&
diff --git a/clang/lib/Interpreter/IncrementalParser.cpp 
b/clang/lib/Interpreter/IncrementalParser.cpp
index 32d1663fbe1a9..bf08911e23533 100644
--- a/clang/lib/Interpreter/IncrementalParser.cpp
+++ b/clang/lib/Interpreter/IncrementalParser.cpp
@@ -37,6 +37,10 @@ IncrementalParser::IncrementalParser(CompilerInstance 
&Instance,
   llvm::ErrorAsOutParameter EAO(&Err);
   Consumer = &S.getASTConsumer();
   P.reset(new Parser(S.getPreprocessor(), S, /*SkipBodies=*/false));
+
+  if (ExternalASTSource *External = S.getASTContext().getExternalSource())
+    External->StartTranslationUnit(Consumer);
+
   P->Initialize();
 }
 
diff --git a/clang/lib/Interpreter/Interpreter.cpp 
b/clang/lib/Interpreter/Interpreter.cpp
index 043e0c1e5754e..8944937f19115 100644
--- a/clang/lib/Interpreter/Interpreter.cpp
+++ b/clang/lib/Interpreter/Interpreter.cpp
@@ -276,9 +276,10 @@ Interpreter::Interpreter(std::unique_ptr<CompilerInstance> 
Instance,
 
   if (Act->getCodeGen()) {
     Act->CacheCodeGenModule();
-    // The initial PTU is filled by `-include` or by CUDA includes
-    // automatically.
-    if (!CI->getPreprocessorOpts().Includes.empty()) {
+    // The initial PTU is filled by `-include`/`-include-pch` or by CUDA
+    // includes automatically.
+    if (!CI->getPreprocessorOpts().Includes.empty() ||
+        !CI->getPreprocessorOpts().ImplicitPCHInclude.empty()) {
       // We can't really directly pass the CachedInCodeGenModule to the Jit
       // because it will steal it, causing dangling references as explained in
       // Interpreter::Execute
diff --git a/clang/test/Interpreter/execute-pch.cpp 
b/clang/test/Interpreter/execute-pch.cpp
new file mode 100644
index 0000000000000..1ee8221463977
--- /dev/null
+++ b/clang/test/Interpreter/execute-pch.cpp
@@ -0,0 +1,23 @@
+// REQUIRES: host-supports-jit
+// UNSUPPORTED: system-aix
+//
+// RUN: rm -rf %t
+// RUN: mkdir -p %t
+// RUN: split-file %s %t
+//
+// RUN: %clang -fmax-type-align=16 -Xclang -pic-level -Xclang 2 -Xclang 
-fdeprecated-macro -fno-stack-protector -Xclang -fwrapv -fPIC -Xclang -fblocks 
-Xclang -fskip-odr-check-in-gmf -fexceptions -fcxx-exceptions -fgnuc-version=0 
-target %host-jit-triple -Xclang -fblocks -Xclang -fmax-type-align=8 -Xclang 
-fincremental-extensions -Xclang -emit-pch -x c++-header -o %t/include.pch 
%t/include.hpp
+//
+// RUN: cat %t/main.cpp \
+// RUN:     | clang-repl -Xcc -fgnuc-version=0 -Xcc -fno-stack-protector -Xcc 
-fwrapv -Xcc -fPIC -Xcc -fblocks -Xcc -fskip-odr-check-in-gmf -Xcc 
-fmax-type-align=8 -Xcc -include-pch -Xcc %t/include.pch \
+// RUN:     | FileCheck %s
+
+//--- include.hpp
+
+int f_pch() { return 5; }
+
+//--- main.cpp
+
+extern "C" int printf(const char *, ...);
+printf("f_pch = %d\n", f_pch());
+
+// CHECK: f_pch = 5
diff --git a/clang/test/lit.cfg.py b/clang/test/lit.cfg.py
index d34319131ab6d..bec7f70517471 100644
--- a/clang/test/lit.cfg.py
+++ b/clang/test/lit.cfg.py
@@ -191,6 +191,10 @@ def have_host_clang_repl_cuda():
 
     if have_host_clang_repl_cuda():
         config.available_features.add('host-supports-cuda')
+    hosttriple = run_clang_repl("--host-jit-triple")
+    config.substitutions.append(
+        ("%host-jit-triple",  hosttriple.strip())
+    )
 
     if have_host_out_of_process_jit_feature_support():
         config.available_features.add("host-supports-out-of-process-jit")
diff --git a/clang/tools/clang-repl/ClangRepl.cpp 
b/clang/tools/clang-repl/ClangRepl.cpp
index 1d508816d7047..c7879422cd7df 100644
--- a/clang/tools/clang-repl/ClangRepl.cpp
+++ b/clang/tools/clang-repl/ClangRepl.cpp
@@ -85,6 +85,8 @@ static llvm::cl::list<std::string>
               llvm::cl::CommaSeparated);
 static llvm::cl::opt<bool> OptHostSupportsJit("host-supports-jit",
                                               llvm::cl::Hidden);
+static llvm::cl::opt<bool> OptHostJitTriple("host-jit-triple",
+                                            llvm::cl::Hidden);
 static llvm::cl::list<std::string> OptInputs(llvm::cl::Positional,
                                              llvm::cl::desc("[code to run]"));
 
@@ -279,6 +281,11 @@ int main(int argc, const char **argv) {
       llvm::outs() << "false\n";
     }
     return 0;
+  } else if (OptHostJitTriple) {
+    auto J = ExitOnErr(llvm::orc::LLJITBuilder().create());
+    auto T = J->getTargetTriple();
+    llvm::outs() << T.normalize() << '\n';
+    return 0;
   }
 
   clang::IncrementalCompilerBuilder CB;

``````````

</details>


https://github.com/llvm/llvm-project/pull/157359
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to