llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-clang Author: Vassil Vassilev (vgvassilev) <details> <summary>Changes</summary> 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 clone the module and pass it down. With some effort it is possible to teach CodeGen to ask the CodeGenModule for its current module and that would have an overall positive impact on CodeGen improving the encapsulation of various parts but that's not resilient to future regression. This patch takes a more conservative approach and clones the llvm::Module before passing it to the Jit. That's also not bullet proof because we have to guarantee that CodeGen does not write on the blueprint. At that stage that seems more consistent to what clang-repl already does to map each partial translation unit to a new Module. This change will fixes a long-standing invalid memory access reported by valgrind when we enable the TBAA optimization passes. It also unblock progress on llvm/llvm-project#<!-- -->84758. --- Full diff: https://github.com/llvm/llvm-project/pull/89031.diff 1 Files Affected: - (modified) clang/lib/Interpreter/IncrementalExecutor.cpp (+10-1) ``````````diff diff --git a/clang/lib/Interpreter/IncrementalExecutor.cpp b/clang/lib/Interpreter/IncrementalExecutor.cpp index 6f036107c14a9c..e87f43f077f379 100644 --- a/clang/lib/Interpreter/IncrementalExecutor.cpp +++ b/clang/lib/Interpreter/IncrementalExecutor.cpp @@ -28,6 +28,7 @@ #include "llvm/IR/Module.h" #include "llvm/Support/ManagedStatic.h" #include "llvm/Support/TargetSelect.h" +#include "llvm/Transforms/Utils/Cloning.h" // Force linking some of the runtimes that helps attaching to a debugger. LLVM_ATTRIBUTE_USED void linkComponents() { @@ -73,7 +74,15 @@ llvm::Error IncrementalExecutor::addModule(PartialTranslationUnit &PTU) { Jit->getMainJITDylib().createResourceTracker(); ResourceTrackers[&PTU] = RT; - return Jit->addIRModule(RT, {std::move(PTU.TheModule), TSCtx}); + // 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 clone the module and pass it down. + std::unique_ptr<llvm::Module> ModuleClone = llvm::CloneModule(*PTU.TheModule); + + return Jit->addIRModule(RT, {std::move(ModuleClone), TSCtx}); } llvm::Error IncrementalExecutor::removeModule(PartialTranslationUnit &PTU) { `````````` </details> https://github.com/llvm/llvm-project/pull/89031 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
