Skip to content

Conversation

anutosh491
Copy link
Member

@anutosh491 anutosh491 commented Apr 26, 2025

Read discussion #136404 (comment) and the following comments for context

Motivation

  1. IncrementalAction is designed to keep Frontend statealive across inputs. As per the docstring: “IncrementalAction ensures it keeps its underlying action's objects alive as long as the IncrementalParser needs them.”
  2. To align responsibilities with that contract, the parser layer (host: IncrementalParser, device: IncrementalCUDADeviceParser) should manage PTU registration and module generation, while the interpreter orchestrates at a higher level.

What this PR does

  1. Moves CodeGen surfaces behind IncrementalAction:
    GenModule(), getCodeGen(), and the cached “first CodeGen module” now live in IncrementalAction.

  2. Moves PTU ownership to the parser layer:
    Adds IncrementalParser::RegisterPTU(…) (and device counterpart)

  3. Add device-side registration in IncrementalCUDADeviceParser.

  4. Remove Interpreter::{getCodeGen, GenModule, RegisterPTU}.

@llvmbot llvmbot added clang Clang issues not falling into any other category clang:frontend Language frontend issues, e.g. anything involving "Sema" labels Apr 26, 2025
@llvmbot
Copy link
Member

llvmbot commented Apr 26, 2025

@llvm/pr-subscribers-clang

Author: Anutosh Bhat (anutosh491)

Changes

Read discussion #136404 (comment) and the following comments for context


Patch is 28.83 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/137458.diff

10 Files Affected:

  • (modified) clang/include/clang/Interpreter/Interpreter.h (-11)
  • (modified) clang/lib/Interpreter/CMakeLists.txt (+1)
  • (modified) clang/lib/Interpreter/DeviceOffload.cpp (+3-3)
  • (modified) clang/lib/Interpreter/DeviceOffload.h (+3-3)
  • (added) clang/lib/Interpreter/IncrementalAction.cpp (+110)
  • (added) clang/lib/Interpreter/IncrementalAction.h (+68)
  • (modified) clang/lib/Interpreter/IncrementalParser.cpp (+65-3)
  • (modified) clang/lib/Interpreter/IncrementalParser.h (+29-2)
  • (modified) clang/lib/Interpreter/Interpreter.cpp (+14-184)
  • (modified) clang/lib/Interpreter/InterpreterValuePrinter.cpp (+1-1)
diff --git a/clang/include/clang/Interpreter/Interpreter.h b/clang/include/clang/Interpreter/Interpreter.h
index 56213f88b9e30..145ed409b7807 100644
--- a/clang/include/clang/Interpreter/Interpreter.h
+++ b/clang/include/clang/Interpreter/Interpreter.h
@@ -36,7 +36,6 @@ class ThreadSafeContext;
 namespace clang {
 
 class CompilerInstance;
-class CodeGenerator;
 class CXXRecordDecl;
 class Decl;
 class IncrementalExecutor;
@@ -109,10 +108,6 @@ class Interpreter {
   // printing happens, it's in an invalid state.
   Value LastValue;
 
-  /// When CodeGen is created the first llvm::Module gets cached in many places
-  /// and we must keep it alive.
-  std::unique_ptr<llvm::Module> CachedInCodeGenModule;
-
   /// Compiler instance performing the incremental compilation.
   std::unique_ptr<CompilerInstance> CI;
 
@@ -179,12 +174,6 @@ class Interpreter {
   llvm::Expected<Expr *> ExtractValueFromExpr(Expr *E);
   llvm::Expected<llvm::orc::ExecutorAddr> CompileDtorCall(CXXRecordDecl *CXXRD);
 
-  CodeGenerator *getCodeGen(IncrementalAction *Action = nullptr) const;
-  std::unique_ptr<llvm::Module> GenModule(IncrementalAction *Action = nullptr);
-  PartialTranslationUnit &RegisterPTU(TranslationUnitDecl *TU,
-                                      std::unique_ptr<llvm::Module> M = {},
-                                      IncrementalAction *Action = nullptr);
-
   // A cache for the compiled destructors used to for de-allocation of managed
   // clang::Values.
   llvm::DenseMap<CXXRecordDecl *, llvm::orc::ExecutorAddr> Dtors;
diff --git a/clang/lib/Interpreter/CMakeLists.txt b/clang/lib/Interpreter/CMakeLists.txt
index bf70cdfbee01e..79b8369371db6 100644
--- a/clang/lib/Interpreter/CMakeLists.txt
+++ b/clang/lib/Interpreter/CMakeLists.txt
@@ -22,6 +22,7 @@ endif()
 add_clang_library(clangInterpreter
   DeviceOffload.cpp
   CodeCompletion.cpp
+  IncrementalAction.cpp
   IncrementalExecutor.cpp
   IncrementalParser.cpp
   Interpreter.cpp
diff --git a/clang/lib/Interpreter/DeviceOffload.cpp b/clang/lib/Interpreter/DeviceOffload.cpp
index 7d0125403ea52..5415697d24daf 100644
--- a/clang/lib/Interpreter/DeviceOffload.cpp
+++ b/clang/lib/Interpreter/DeviceOffload.cpp
@@ -26,10 +26,10 @@ namespace clang {
 
 IncrementalCUDADeviceParser::IncrementalCUDADeviceParser(
     std::unique_ptr<CompilerInstance> DeviceInstance,
-    CompilerInstance &HostInstance,
+    CompilerInstance &HostInstance, IncrementalAction *DeviceAct,
     llvm::IntrusiveRefCntPtr<llvm::vfs::InMemoryFileSystem> FS,
-    llvm::Error &Err, const std::list<PartialTranslationUnit> &PTUs)
-    : IncrementalParser(*DeviceInstance, Err), PTUs(PTUs), VFS(FS),
+    llvm::Error &Err, std::list<PartialTranslationUnit> &PTUs)
+    : IncrementalParser(*DeviceInstance, DeviceAct, Err, PTUs), VFS(FS),
       CodeGenOpts(HostInstance.getCodeGenOpts()),
       TargetOpts(DeviceInstance->getTargetOpts()) {
   if (Err)
diff --git a/clang/lib/Interpreter/DeviceOffload.h b/clang/lib/Interpreter/DeviceOffload.h
index 43645033c4840..e4cfd26aa4871 100644
--- a/clang/lib/Interpreter/DeviceOffload.h
+++ b/clang/lib/Interpreter/DeviceOffload.h
@@ -22,16 +22,16 @@ struct PartialTranslationUnit;
 class CompilerInstance;
 class CodeGenOptions;
 class TargetOptions;
+class IncrementalAction;
 
 class IncrementalCUDADeviceParser : public IncrementalParser {
-  const std::list<PartialTranslationUnit> &PTUs;
 
 public:
   IncrementalCUDADeviceParser(
       std::unique_ptr<CompilerInstance> DeviceInstance,
-      CompilerInstance &HostInstance,
+      CompilerInstance &HostInstance, IncrementalAction *DeviceAct,
       llvm::IntrusiveRefCntPtr<llvm::vfs::InMemoryFileSystem> VFS,
-      llvm::Error &Err, const std::list<PartialTranslationUnit> &PTUs);
+      llvm::Error &Err, std::list<PartialTranslationUnit> &PTUs);
 
   // Generate PTX for the last PTU.
   llvm::Expected<llvm::StringRef> GeneratePTX();
diff --git a/clang/lib/Interpreter/IncrementalAction.cpp b/clang/lib/Interpreter/IncrementalAction.cpp
new file mode 100644
index 0000000000000..683925859c645
--- /dev/null
+++ b/clang/lib/Interpreter/IncrementalAction.cpp
@@ -0,0 +1,110 @@
+//===--- IncrementalAction.h - Incremental Frontend Action -*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#include "IncrementalAction.h"
+
+#include "clang/AST/ASTConsumer.h"
+#include "clang/CodeGen/CodeGenAction.h"
+#include "clang/Frontend/CompilerInstance.h"
+#include "clang/Frontend/FrontendOptions.h"
+#include "clang/FrontendTool/Utils.h"
+#include "clang/Interpreter/Interpreter.h"
+#include "clang/Sema/Sema.h"
+#include "llvm/Support/Error.h"
+#include "llvm/Support/ErrorHandling.h"
+
+namespace clang {
+IncrementalAction::IncrementalAction(CompilerInstance &CI,
+                                     llvm::LLVMContext &LLVMCtx,
+                                     llvm::Error &Err, Interpreter &I,
+                                     std::unique_ptr<ASTConsumer> Consumer)
+    : WrapperFrontendAction([&]() {
+        llvm::ErrorAsOutParameter EAO(&Err);
+        std::unique_ptr<FrontendAction> Act;
+        switch (CI.getFrontendOpts().ProgramAction) {
+        default:
+          Err = llvm::createStringError(
+              std::errc::state_not_recoverable,
+              "Driver initialization failed. "
+              "Incremental mode for action %d is not supported",
+              CI.getFrontendOpts().ProgramAction);
+          return Act;
+        case frontend::ASTDump:
+        case frontend::ASTPrint:
+        case frontend::ParseSyntaxOnly:
+          Act = CreateFrontendAction(CI);
+          break;
+        case frontend::PluginAction:
+        case frontend::EmitAssembly:
+        case frontend::EmitBC:
+        case frontend::EmitObj:
+        case frontend::PrintPreprocessedInput:
+        case frontend::EmitLLVMOnly:
+          Act.reset(new EmitLLVMOnlyAction(&LLVMCtx));
+          break;
+        }
+        return Act;
+      }()),
+      Interp(I), Consumer(std::move(Consumer)) {}
+
+std::unique_ptr<ASTConsumer>
+IncrementalAction::CreateASTConsumer(CompilerInstance &CI, StringRef InFile) {
+  std::unique_ptr<ASTConsumer> C =
+      WrapperFrontendAction::CreateASTConsumer(CI, InFile);
+
+  if (Consumer) {
+    std::vector<std::unique_ptr<ASTConsumer>> Cs;
+    Cs.push_back(std::move(Consumer));
+    Cs.push_back(std::move(C));
+    return std::make_unique<MultiplexConsumer>(std::move(Cs));
+  }
+
+  return std::make_unique<InProcessPrintingASTConsumer>(std::move(C), Interp);
+}
+
+void IncrementalAction::ExecuteAction() {
+  WrapperFrontendAction::ExecuteAction();
+  getCompilerInstance().getSema().CurContext = nullptr;
+}
+
+void IncrementalAction::EndSourceFile() {
+  if (IsTerminating && getWrapped())
+    WrapperFrontendAction::EndSourceFile();
+}
+
+void IncrementalAction::FinalizeAction() {
+  assert(!IsTerminating && "Already finalized!");
+  IsTerminating = true;
+  EndSourceFile();
+}
+
+InProcessPrintingASTConsumer::InProcessPrintingASTConsumer(
+    std::unique_ptr<ASTConsumer> C, Interpreter &I)
+    : MultiplexConsumer(std::move(C)), Interp(I) {}
+
+bool InProcessPrintingASTConsumer::HandleTopLevelDecl(DeclGroupRef DGR) {
+  if (DGR.isNull())
+    return true;
+
+  for (Decl *D : DGR)
+    if (auto *TLSD = llvm::dyn_cast<TopLevelStmtDecl>(D))
+      if (TLSD && TLSD->isSemiMissing()) {
+        auto ExprOrErr =
+            Interp.ExtractValueFromExpr(cast<Expr>(TLSD->getStmt()));
+        if (llvm::Error E = ExprOrErr.takeError()) {
+          llvm::logAllUnhandledErrors(std::move(E), llvm::errs(),
+                                      "Value printing failed: ");
+          return false; // abort parsing
+        }
+        TLSD->setStmt(*ExprOrErr);
+      }
+
+  return MultiplexConsumer::HandleTopLevelDecl(DGR);
+}
+
+} // namespace clang
\ No newline at end of file
diff --git a/clang/lib/Interpreter/IncrementalAction.h b/clang/lib/Interpreter/IncrementalAction.h
new file mode 100644
index 0000000000000..08d090a0513fe
--- /dev/null
+++ b/clang/lib/Interpreter/IncrementalAction.h
@@ -0,0 +1,68 @@
+//===--- IncrementalAction.h - Incremental Frontend Action -*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLVM_CLANG_INTERPRETER_INCREMENTALACTION_H
+#define LLVM_CLANG_INTERPRETER_INCREMENTALACTION_H
+
+#include "clang/Frontend/FrontendActions.h"
+#include "clang/Frontend/MultiplexConsumer.h"
+
+namespace clang {
+
+class Interpreter;
+
+/// A custom action enabling the incremental processing functionality.
+///
+/// The usual \p FrontendAction expects one call to ExecuteAction and once it
+/// sees a call to \p EndSourceFile it deletes some of the important objects
+/// such as \p Preprocessor and \p Sema assuming no further input will come.
+///
+/// \p IncrementalAction ensures it keep its underlying action's objects alive
+/// as long as the \p IncrementalParser needs them.
+///
+class IncrementalAction : public WrapperFrontendAction {
+private:
+  bool IsTerminating = false;
+  Interpreter &Interp;
+  std::unique_ptr<ASTConsumer> Consumer;
+
+public:
+  IncrementalAction(CompilerInstance &CI, llvm::LLVMContext &LLVMCtx,
+                    llvm::Error &Err, Interpreter &I,
+                    std::unique_ptr<ASTConsumer> Consumer = nullptr);
+
+  FrontendAction *getWrapped() const { return WrappedAction.get(); }
+
+  TranslationUnitKind getTranslationUnitKind() override {
+    return TU_Incremental;
+  }
+
+  std::unique_ptr<ASTConsumer> CreateASTConsumer(CompilerInstance &CI,
+                                                 StringRef InFile) override;
+
+  void ExecuteAction() override;
+
+  // Do not terminate after processing the input. This allows us to keep various
+  // clang objects alive and to incrementally grow the current TU.
+  void EndSourceFile() override;
+
+  void FinalizeAction();
+};
+
+class InProcessPrintingASTConsumer final : public MultiplexConsumer {
+  Interpreter &Interp;
+
+public:
+  InProcessPrintingASTConsumer(std::unique_ptr<ASTConsumer> C, Interpreter &I);
+
+  bool HandleTopLevelDecl(DeclGroupRef DGR) override;
+};
+
+} // end namespace clang
+
+#endif // LLVM_CLANG_INTERPRETER_INCREMENTALACTION_H
diff --git a/clang/lib/Interpreter/IncrementalParser.cpp b/clang/lib/Interpreter/IncrementalParser.cpp
index 41d6304bd5f65..9691673be06dd 100644
--- a/clang/lib/Interpreter/IncrementalParser.cpp
+++ b/clang/lib/Interpreter/IncrementalParser.cpp
@@ -11,27 +11,36 @@
 //===----------------------------------------------------------------------===//
 
 #include "IncrementalParser.h"
+#include "IncrementalAction.h"
 
 #include "clang/AST/DeclContextInternals.h"
+#include "clang/CodeGen/CodeGenAction.h"
+#include "clang/CodeGen/ModuleBuilder.h"
 #include "clang/Frontend/CompilerInstance.h"
+#include "clang/Frontend/FrontendAction.h"
 #include "clang/Interpreter/PartialTranslationUnit.h"
+#include "clang/Lex/PreprocessorOptions.h"
 #include "clang/Parse/Parser.h"
 #include "clang/Sema/Sema.h"
+#include "llvm/IR/Module.h"
 #include "llvm/Support/CrashRecoveryContext.h"
 #include "llvm/Support/Error.h"
 
 #include <sstream>
 
+#define DEBUG_TYPE "clang-repl"
+
 namespace clang {
 
 // IncrementalParser::IncrementalParser() {}
 
 IncrementalParser::IncrementalParser(CompilerInstance &Instance,
-                                     llvm::Error &Err)
-    : S(Instance.getSema()) {
+                                     IncrementalAction *Act, llvm::Error &Err,
+                                     std::list<PartialTranslationUnit> &PTUs)
+    : S(Instance.getSema()), CI(Instance), Act(Act), PTUs(PTUs) {
   llvm::ErrorAsOutParameter EAO(&Err);
   Consumer = &S.getASTConsumer();
-  P.reset(new Parser(S.getPreprocessor(), S, /*SkipBodies=*/false));
+  P = std::make_unique<Parser>(S.getPreprocessor(), S, /*SkipBodies=*/false);
   P->Initialize();
 }
 
@@ -185,4 +194,57 @@ void IncrementalParser::CleanUpPTU(TranslationUnitDecl *MostRecentTU) {
   }
 }
 
+PartialTranslationUnit &
+IncrementalParser::RegisterPTU(TranslationUnitDecl *TU,
+                               std::unique_ptr<llvm::Module> M /*={}*/) {
+  PTUs.emplace_back(PartialTranslationUnit());
+  PartialTranslationUnit &LastPTU = PTUs.back();
+  LastPTU.TUPart = TU;
+
+  if (!M)
+    M = GenModule();
+
+  assert((!getCodeGen() || M) && "Must have a llvm::Module at this point");
+
+  LastPTU.TheModule = std::move(M);
+  LLVM_DEBUG(llvm::dbgs() << "compile-ptu " << PTUs.size() - 1
+                          << ": [TU=" << LastPTU.TUPart);
+  if (LastPTU.TheModule)
+    LLVM_DEBUG(llvm::dbgs() << ", M=" << LastPTU.TheModule.get() << " ("
+                            << LastPTU.TheModule->getName() << ")");
+  LLVM_DEBUG(llvm::dbgs() << "]\n");
+  return LastPTU;
+}
+
+std::unique_ptr<llvm::Module> IncrementalParser::GenModule() {
+  static unsigned ID = 0;
+  if (CodeGenerator *CG = getCodeGen()) {
+    // Clang's CodeGen is designed to work with a single llvm::Module. In many
+    // cases for convenience various CodeGen parts have a reference to the
+    // llvm::Module (TheModule or Module) which does not change when a new
+    // module is pushed. However, the execution engine wants to take ownership
+    // of the module which does not map well to CodeGen's design. To work this
+    // around we created an empty module to make CodeGen happy. We should make
+    // sure it always stays empty.
+    assert(((!CachedInCodeGenModule ||
+             !CI.getPreprocessorOpts().Includes.empty()) ||
+            (CachedInCodeGenModule->empty() &&
+             CachedInCodeGenModule->global_empty() &&
+             CachedInCodeGenModule->alias_empty() &&
+             CachedInCodeGenModule->ifunc_empty())) &&
+           "CodeGen wrote to a readonly module");
+    std::unique_ptr<llvm::Module> M(CG->ReleaseModule());
+    CG->StartModule("incr_module_" + std::to_string(ID++), M->getContext());
+    return M;
+  }
+  return nullptr;
+}
+
+CodeGenerator *IncrementalParser::getCodeGen() const {
+  FrontendAction *WrappedAct = Act->getWrapped();
+  if (!WrappedAct->hasIRSupport())
+    return nullptr;
+  return static_cast<CodeGenAction *>(WrappedAct)->getCodeGenerator();
+}
+
 } // end namespace clang
diff --git a/clang/lib/Interpreter/IncrementalParser.h b/clang/lib/Interpreter/IncrementalParser.h
index 4fdde749a2e75..c3d7cd615d9b0 100644
--- a/clang/lib/Interpreter/IncrementalParser.h
+++ b/clang/lib/Interpreter/IncrementalParser.h
@@ -19,6 +19,10 @@
 #include <list>
 #include <memory>
 
+namespace llvm {
+class Module;
+}
+
 namespace clang {
 class ASTConsumer;
 class CodeGenerator;
@@ -26,6 +30,8 @@ class CompilerInstance;
 class Parser;
 class Sema;
 class TranslationUnitDecl;
+class IncrementalAction;
+struct PartialTranslationUnit;
 
 /// Provides support for incremental compilation. Keeps track of the state
 /// changes between the subsequent incremental input.
@@ -44,12 +50,23 @@ class IncrementalParser {
   /// Counts the number of direct user input lines that have been parsed.
   unsigned InputCount = 0;
 
-  // IncrementalParser();
+  /// The CompilerInstance used during parsing.
+  CompilerInstance &CI;
+
+  /// The FrontendAction used during incremental parsing.
+  IncrementalAction *Act = nullptr;
+
+  std::list<PartialTranslationUnit> &PTUs;
 
 public:
-  IncrementalParser(CompilerInstance &Instance, llvm::Error &Err);
+  IncrementalParser(CompilerInstance &Instance, IncrementalAction *Act,
+                    llvm::Error &Err, std::list<PartialTranslationUnit> &PTUs);
   virtual ~IncrementalParser();
 
+  /// When CodeGen is created the first llvm::Module gets cached in many places
+  /// and we must keep it alive.
+  std::unique_ptr<llvm::Module> CachedInCodeGenModule;
+
   /// Parses incremental input by creating an in-memory file.
   ///\returns a \c PartialTranslationUnit which holds information about the
   /// \c TranslationUnitDecl.
@@ -57,6 +74,16 @@ class IncrementalParser {
 
   void CleanUpPTU(TranslationUnitDecl *MostRecentTU);
 
+  /// Access the current code generator.
+  CodeGenerator *getCodeGen() const;
+
+  /// Generate an LLVM module for the most recent parsed input.
+  std::unique_ptr<llvm::Module> GenModule();
+
+  /// Register a PTU produced by Parse.
+  PartialTranslationUnit &RegisterPTU(TranslationUnitDecl *TU,
+                                      std::unique_ptr<llvm::Module> M = {});
+
 private:
   llvm::Expected<TranslationUnitDecl *> ParseOrWrapTopLevelDecl();
 };
diff --git a/clang/lib/Interpreter/Interpreter.cpp b/clang/lib/Interpreter/Interpreter.cpp
index e2950e171a77e..deb3b7655eb27 100644
--- a/clang/lib/Interpreter/Interpreter.cpp
+++ b/clang/lib/Interpreter/Interpreter.cpp
@@ -12,6 +12,7 @@
 //===----------------------------------------------------------------------===//
 
 #include "DeviceOffload.h"
+#include "IncrementalAction.h"
 #include "IncrementalExecutor.h"
 #include "IncrementalParser.h"
 #include "InterpreterUtils.h"
@@ -28,7 +29,6 @@
 #include "clang/Basic/DiagnosticSema.h"
 #include "clang/Basic/TargetInfo.h"
 #include "clang/CodeGen/CodeGenAction.h"
-#include "clang/CodeGen/ModuleBuilder.h"
 #include "clang/CodeGen/ObjectFilePCHContainerWriter.h"
 #include "clang/Driver/Compilation.h"
 #include "clang/Driver/Driver.h"
@@ -248,120 +248,6 @@ IncrementalCompilerBuilder::CreateCudaHost() {
   return IncrementalCompilerBuilder::createCuda(false);
 }
 
-class InProcessPrintingASTConsumer final : public MultiplexConsumer {
-  Interpreter &Interp;
-
-public:
-  InProcessPrintingASTConsumer(std::unique_ptr<ASTConsumer> C, Interpreter &I)
-      : MultiplexConsumer(std::move(C)), Interp(I) {}
-  bool HandleTopLevelDecl(DeclGroupRef DGR) override final {
-    if (DGR.isNull())
-      return true;
-
-    for (Decl *D : DGR)
-      if (auto *TLSD = llvm::dyn_cast<TopLevelStmtDecl>(D))
-        if (TLSD && TLSD->isSemiMissing()) {
-          auto ExprOrErr =
-              Interp.ExtractValueFromExpr(cast<Expr>(TLSD->getStmt()));
-          if (llvm::Error E = ExprOrErr.takeError()) {
-            llvm::logAllUnhandledErrors(std::move(E), llvm::errs(),
-                                        "Value printing failed: ");
-            return false; // abort parsing
-          }
-          TLSD->setStmt(*ExprOrErr);
-        }
-
-    return MultiplexConsumer::HandleTopLevelDecl(DGR);
-  }
-};
-
-/// A custom action enabling the incremental processing functionality.
-///
-/// The usual \p FrontendAction expects one call to ExecuteAction and once it
-/// sees a call to \p EndSourceFile it deletes some of the important objects
-/// such as \p Preprocessor and \p Sema assuming no further input will come.
-///
-/// \p IncrementalAction ensures it keep its underlying action's objects alive
-/// as long as the \p IncrementalParser needs them.
-///
-class IncrementalAction : public WrapperFrontendAction {
-private:
-  bool IsTerminating = false;
-  Interpreter &Interp;
-  std::unique_ptr<ASTConsumer> Consumer;
-
-public:
-  IncrementalAction(CompilerInstance &CI, llvm::LLVMContext &LLVMCtx,
-                    llvm::Error &Err, Interpreter &I,
-                    std::unique_ptr<ASTConsumer> Consumer = nullptr)
-      : WrapperFrontendAction([&]() {
-          llvm::ErrorAsOutParameter EAO(&Err);
-          std::unique_ptr<FrontendAction> Act;
-          switch (CI.getFrontendOpts().ProgramAction) {
-          default:
-            Err = llvm::createStringError(
-                std::errc::state_not_recoverable,
-                "Driver initialization failed. "
-                "Incremental mode for action %d is not supported",
-                CI.getFrontendOpts().ProgramAction);
-            return Act;
-          case frontend::ASTDump:
-          case frontend::ASTPrint:
-          case frontend::ParseSyntaxOnly:
-            Act = CreateFrontendAction(CI);
-            break;
-          case frontend::PluginAction:
-          case frontend::EmitAssembly:
-          case frontend::EmitBC:
-          case frontend::EmitObj:
-          case fr...
[truncated]

Copy link

github-actions bot commented May 5, 2025

✅ With the latest revision this PR passed the C/C++ code formatter.

@vgvassilev
Copy link
Contributor

It seems like the direction is reasonable. Could you update the PR description and the commit message to capture the intent of this PR?

@anutosh491
Copy link
Member Author

anutosh491 commented Aug 26, 2025

This works as expected now !

anutosh491@vv-nuc:/build/anutosh491/llvm-project/build2/bin$ ./clang-repl --cuda
clang-repl> extern "C" int printf(const char*, ...);
clang-repl> template <typename T> __device__ inline T sum(T a, T b) { return a + b; }
clang-repl> __global__ void test_kernel(int* value) { *value = sum(40, 2); }
clang-repl> int var;
clang-repl> int* devptr = nullptr;
clang-repl> printf("cudaMalloc: %d\n", cudaMalloc((void **) &devptr, sizeof(int)));
cudaMalloc: 0
clang-repl> test_kernel<<<1,1>>>(devptr);
clang-repl> printf("CUDA Error: %d\n", cudaGetLastError());
CUDA Error: 0
clang-repl> printf("cudaMemcpy: %d\n", cudaMemcpy(&var, devptr, sizeof(int), cudaMemcpyDeviceToHost));
cudaMemcpy: 0
clang-repl> printf("Value: %d\n", var);
Value: 42
clang-repl> 

Should be ready now.

@anutosh491
Copy link
Member Author

It seems like the direction is reasonable. Could you update the PR description and the commit message to capture the intent of this PR?

Thank you. I have updated the PR description and we can update the commit message to convey the same while squashing & merging I suppose.

Copy link
Contributor

@vgvassilev vgvassilev left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! Please update the commit message accordingly on merge.

@anutosh491 anutosh491 merged commit 2ab4c28 into llvm:main Aug 26, 2025
9 checks passed
@anutosh491 anutosh491 deleted the cuda2 branch August 26, 2025 09:44
@anutosh491
Copy link
Member Author

Thanks for the review :)

private:
bool IsTerminating = false;
Interpreter &Interp;
CompilerInstance &CI;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The CI member seems to be unused in builds without asserts?

../../clang/lib/Interpreter/IncrementalAction.h:37:21: error: private field 'CI' is not used [-Werror,-Wunused-private-field]
   37 |   CompilerInstance &CI;
      |                     ^
1 error generated.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ahh thanks for reporting this. I've proposed a fix here #155400

anutosh491 added a commit that referenced this pull request Aug 27, 2025
…on-assert/assert builds (#155400)

See
#137458 (comment)

Context: So the CompilerInstance CI being used with the Incremental
Action are tightly coupled in any case. Which means
the  CI put to use while constructing the IncrementalAction
```
  Act = TSCtx->withContextDo([&](llvm::LLVMContext *Ctx) {
    return std::make_unique<IncrementalAction>(*CI, *Ctx, ErrOut, *this,
                                               std::move(Consumer));
  });                      
```

Is also the CI through which the we call `ExecuteAction` on `Act`
```
CI->ExecuteAction(*Act);
```

So we need to use CI as a member variable in IncrementalAction for
assert builds for

https://github.com/llvm/llvm-project/blob/bddac5eda9b7591e05ccdc86a5e86c592085f318/clang/lib/Interpreter/IncrementalAction.cpp#L97-L108

The same can be put to use for `CreateASTConsumer` too as all of these
are referring to the same CI
```
std::unique_ptr<ASTConsumer>
IncrementalAction::CreateASTConsumer(CompilerInstance & /*CI*/, StringRef InFile) {
  std::unique_ptr<ASTConsumer> C =
      WrapperFrontendAction::CreateASTConsumer(this->CI, InFile);
      
```
llvm-sync bot pushed a commit to arm/arm-toolchain that referenced this pull request Aug 27, 2025
…o use for non-assert/assert builds (#155400)

See
llvm/llvm-project#137458 (comment)

Context: So the CompilerInstance CI being used with the Incremental
Action are tightly coupled in any case. Which means
the  CI put to use while constructing the IncrementalAction
```
  Act = TSCtx->withContextDo([&](llvm::LLVMContext *Ctx) {
    return std::make_unique<IncrementalAction>(*CI, *Ctx, ErrOut, *this,
                                               std::move(Consumer));
  });
```

Is also the CI through which the we call `ExecuteAction` on `Act`
```
CI->ExecuteAction(*Act);
```

So we need to use CI as a member variable in IncrementalAction for
assert builds for

https://github.com/llvm/llvm-project/blob/bddac5eda9b7591e05ccdc86a5e86c592085f318/clang/lib/Interpreter/IncrementalAction.cpp#L97-L108

The same can be put to use for `CreateASTConsumer` too as all of these
are referring to the same CI
```
std::unique_ptr<ASTConsumer>
IncrementalAction::CreateASTConsumer(CompilerInstance & /*CI*/, StringRef InFile) {
  std::unique_ptr<ASTConsumer> C =
      WrapperFrontendAction::CreateASTConsumer(this->CI, InFile);

```
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
clang:frontend Language frontend issues, e.g. anything involving "Sema" clang Clang issues not falling into any other category
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants