[PATCH] D34444: Teach codegen to work in incremental processing mode.
v.g.vassilev abandoned this revision. v.g.vassilev added a comment. Herald added a project: All. This landed slightly modified here: https://github.com/llvm/llvm-project/commit/4d54e543abd5d0a8b0a321f8c292252f4895693a CHANGES SINCE LAST ACTION https://reviews.llvm.org/D3/new/ https://reviews.llvm.org/D3 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D34444: Teach codegen to work in incremental processing mode.
v.g.vassilev added a comment. A version of this landed in r311843. I am keeping in mind this discussion and I'd like to follow up with @rjmccall once I open the more major review item (libInterpreter). https://reviews.llvm.org/D3 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D34444: Teach codegen to work in incremental processing mode.
v.g.vassilev updated this revision to Diff 112813.
v.g.vassilev added a comment.
Type& name -> Type
https://reviews.llvm.org/D3
Files:
include/clang/CodeGen/ModuleBuilder.h
lib/CodeGen/ModuleBuilder.cpp
Index: lib/CodeGen/ModuleBuilder.cpp
===
--- lib/CodeGen/ModuleBuilder.cpp
+++ lib/CodeGen/ModuleBuilder.cpp
@@ -119,6 +119,14 @@
return Builder->GetAddrOfGlobal(global,
ForDefinition_t(isForDefinition));
}
+llvm::Module *StartModule(llvm::StringRef ModuleName, llvm::LLVMContext ,
+ const CodeGenOptions ) {
+ assert(!M && "Replacing existing Module?");
+ M.reset(new llvm::Module(ModuleName, C));
+ Initialize(*Ctx);
+ return M.get();
+}
+
void Initialize(ASTContext ) override {
Ctx =
@@ -317,6 +325,11 @@
->GetAddrOfGlobal(global, isForDefinition);
}
+llvm::Module *CodeGenerator::StartModule(llvm::StringRef ModuleName,
+ llvm::LLVMContext ) {
+ return static_cast(this)->StartModule(ModuleName, C);
+}
+
CodeGenerator *clang::CreateLLVMCodeGen(
DiagnosticsEngine , llvm::StringRef ModuleName,
const HeaderSearchOptions ,
Index: include/clang/CodeGen/ModuleBuilder.h
===
--- include/clang/CodeGen/ModuleBuilder.h
+++ include/clang/CodeGen/ModuleBuilder.h
@@ -84,6 +84,11 @@
/// code generator will schedule the entity for emission if a
/// definition has been registered with this code generator.
llvm::Constant *GetAddrOfGlobal(GlobalDecl decl, bool isForDefinition);
+
+ /// Create a new \c llvm::Module after calling HandleTranslationUnit. This
+ /// enable codegen in interactive processing environments.
+ llvm::Module* StartModule(llvm::StringRef ModuleName, llvm::LLVMContext ,
+const CodeGenOptions );
};
/// CreateLLVMCodeGen - Create a CodeGenerator instance.
Index: lib/CodeGen/ModuleBuilder.cpp
===
--- lib/CodeGen/ModuleBuilder.cpp
+++ lib/CodeGen/ModuleBuilder.cpp
@@ -119,6 +119,14 @@
return Builder->GetAddrOfGlobal(global, ForDefinition_t(isForDefinition));
}
+llvm::Module *StartModule(llvm::StringRef ModuleName, llvm::LLVMContext ,
+ const CodeGenOptions ) {
+ assert(!M && "Replacing existing Module?");
+ M.reset(new llvm::Module(ModuleName, C));
+ Initialize(*Ctx);
+ return M.get();
+}
+
void Initialize(ASTContext ) override {
Ctx =
@@ -317,6 +325,11 @@
->GetAddrOfGlobal(global, isForDefinition);
}
+llvm::Module *CodeGenerator::StartModule(llvm::StringRef ModuleName,
+ llvm::LLVMContext ) {
+ return static_cast(this)->StartModule(ModuleName, C);
+}
+
CodeGenerator *clang::CreateLLVMCodeGen(
DiagnosticsEngine , llvm::StringRef ModuleName,
const HeaderSearchOptions ,
Index: include/clang/CodeGen/ModuleBuilder.h
===
--- include/clang/CodeGen/ModuleBuilder.h
+++ include/clang/CodeGen/ModuleBuilder.h
@@ -84,6 +84,11 @@
/// code generator will schedule the entity for emission if a
/// definition has been registered with this code generator.
llvm::Constant *GetAddrOfGlobal(GlobalDecl decl, bool isForDefinition);
+
+ /// Create a new \c llvm::Module after calling HandleTranslationUnit. This
+ /// enable codegen in interactive processing environments.
+ llvm::Module* StartModule(llvm::StringRef ModuleName, llvm::LLVMContext ,
+const CodeGenOptions );
};
/// CreateLLVMCodeGen - Create a CodeGenerator instance.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D34444: Teach codegen to work in incremental processing mode.
v.g.vassilev updated this revision to Diff 112812.
v.g.vassilev marked an inline comment as done.
v.g.vassilev added a comment.
std::string& -> llvm::StringRef.
https://reviews.llvm.org/D3
Files:
include/clang/CodeGen/ModuleBuilder.h
lib/CodeGen/ModuleBuilder.cpp
Index: lib/CodeGen/ModuleBuilder.cpp
===
--- lib/CodeGen/ModuleBuilder.cpp
+++ lib/CodeGen/ModuleBuilder.cpp
@@ -119,6 +119,14 @@
return Builder->GetAddrOfGlobal(global,
ForDefinition_t(isForDefinition));
}
+llvm::Module *StartModule(llvm::StringRef ModuleName, llvm::LLVMContext& C,
+ const CodeGenOptions& CGO) {
+ assert(!M && "Replacing existing Module?");
+ M.reset(new llvm::Module(ModuleName, C));
+ Initialize(*Ctx);
+ return M.get();
+}
+
void Initialize(ASTContext ) override {
Ctx =
@@ -317,6 +325,11 @@
->GetAddrOfGlobal(global, isForDefinition);
}
+llvm::Module *CodeGenerator::StartModule(llvm::StringRef ModuleName,
+ llvm::LLVMContext& C) {
+ return static_cast(this)->StartModule(ModuleName, C);
+}
+
CodeGenerator *clang::CreateLLVMCodeGen(
DiagnosticsEngine , llvm::StringRef ModuleName,
const HeaderSearchOptions ,
Index: include/clang/CodeGen/ModuleBuilder.h
===
--- include/clang/CodeGen/ModuleBuilder.h
+++ include/clang/CodeGen/ModuleBuilder.h
@@ -84,6 +84,11 @@
/// code generator will schedule the entity for emission if a
/// definition has been registered with this code generator.
llvm::Constant *GetAddrOfGlobal(GlobalDecl decl, bool isForDefinition);
+
+ /// Create a new \c llvm::Module after calling HandleTranslationUnit. This
+ /// enable codegen in interactive processing environments.
+ llvm::Module* StartModule(llvm::StringRef ModuleName, llvm::LLVMContext& C,
+const CodeGenOptions& CGO);
};
/// CreateLLVMCodeGen - Create a CodeGenerator instance.
Index: lib/CodeGen/ModuleBuilder.cpp
===
--- lib/CodeGen/ModuleBuilder.cpp
+++ lib/CodeGen/ModuleBuilder.cpp
@@ -119,6 +119,14 @@
return Builder->GetAddrOfGlobal(global, ForDefinition_t(isForDefinition));
}
+llvm::Module *StartModule(llvm::StringRef ModuleName, llvm::LLVMContext& C,
+ const CodeGenOptions& CGO) {
+ assert(!M && "Replacing existing Module?");
+ M.reset(new llvm::Module(ModuleName, C));
+ Initialize(*Ctx);
+ return M.get();
+}
+
void Initialize(ASTContext ) override {
Ctx =
@@ -317,6 +325,11 @@
->GetAddrOfGlobal(global, isForDefinition);
}
+llvm::Module *CodeGenerator::StartModule(llvm::StringRef ModuleName,
+ llvm::LLVMContext& C) {
+ return static_cast(this)->StartModule(ModuleName, C);
+}
+
CodeGenerator *clang::CreateLLVMCodeGen(
DiagnosticsEngine , llvm::StringRef ModuleName,
const HeaderSearchOptions ,
Index: include/clang/CodeGen/ModuleBuilder.h
===
--- include/clang/CodeGen/ModuleBuilder.h
+++ include/clang/CodeGen/ModuleBuilder.h
@@ -84,6 +84,11 @@
/// code generator will schedule the entity for emission if a
/// definition has been registered with this code generator.
llvm::Constant *GetAddrOfGlobal(GlobalDecl decl, bool isForDefinition);
+
+ /// Create a new \c llvm::Module after calling HandleTranslationUnit. This
+ /// enable codegen in interactive processing environments.
+ llvm::Module* StartModule(llvm::StringRef ModuleName, llvm::LLVMContext& C,
+const CodeGenOptions& CGO);
};
/// CreateLLVMCodeGen - Create a CodeGenerator instance.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D34444: Teach codegen to work in incremental processing mode.
rjmccall added a comment. In https://reviews.llvm.org/D3#818047, @v.g.vassilev wrote: > >> I am open to changing this code as well. That should probably be another > >> review. > > > > I agree. Are you comfortable with blocking this review until that lands? > > It seems like it would significantly change this. > > I am afraid that will slow down (if not suspend) our efforts to upstream our > local patches. This patch is pretty fundamental for cling and if we change it > now, I will have to go back and rework our implementation. I'd be much more > comfortable in reworking it once we run on vanilla clang (then efforts seems > easier to justify on our end). > > It seems to me that despite being suboptimal, it is not very intrusive and it > would effect only on our use case. I can keep track of such patches and come > back to you for advice how to do them best. Would that make sense? My concern is that efforts to upstream patches are doomed to get bogged down anyway, and in the meantime we'll have however much more untestable code. But we have some of that anyway, so it's at least not novel. I'm willing to accept it. John. Repository: rL LLVM https://reviews.llvm.org/D3 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D34444: Teach codegen to work in incremental processing mode.
v.g.vassilev added a comment. In https://reviews.llvm.org/D3#818030, @rjmccall wrote: > In https://reviews.llvm.org/D3#812836, @v.g.vassilev wrote: > > > In https://reviews.llvm.org/D3#812418, @rjmccall wrote: > > > > > In https://reviews.llvm.org/D3#795175, @v.g.vassilev wrote: > > > > > > > @rjmccall, thanks for the prompt and thorough reply. > > > > > > > > In https://reviews.llvm.org/D3#793311, @rjmccall wrote: > > > > > > > > > Okay. In that case, I see two problems, one major and one > > > > > potentially major. > > > > > > > > > > > > > > > > > > > > This is a very accurate diagnosis which took us 5 years to discover > > > > on an empirical basis ;) > > > > > > > > > You could've asked at any time. :) > > > > > > True. I am not really sure I knew what to ask, though ;) > > > We're open to general "I'm trying to do this and having problems" questions > on the mailing lists. You probably would've needed to know to CC me > specifically, though; sadly, I can't keep up with all the lists I need to. Good to know. Thanks! I will come back to you once I get rid of our O(100) clang patches to discuss what would be the best way of supporting incremental compilation. >>> That's quite brittle, because that code is only executed in a code path >>> that only you are using, and you're not adding any tests. I would greatly >>> prefer a change to IRGen's core assumptions, as suggested. >> >> I am open to changing this code as well. That should probably be another >> review. > > I agree. Are you comfortable with blocking this review until that lands? It > seems like it would significantly change this. I am afraid that will slow down (if not suspend) our efforts to upstream our local patches. This patch is pretty fundamental for cling and if we change it now, I will have to go back and rework our implementation. I'd be much more comfortable in reworking it once we run on vanilla clang (then efforts seems easier to justify on our end). It seems to me that despite being suboptimal, it is not very intrusive and it would effect only on our use case. I can keep track of such patches and come back to you for advice how to do them best. Would that make sense? > > >>> I feel it is important that there be a way to inform an ASTConsumer that no >>> further requests will be made of it, something other than calling its >>> destructor. I would like you to make sure that the ASTConsumer interface >>> supports that and that that call is not made too soon in your alternate >>> processing mode. >> >> Do you have a preference of a name of this new interface? > > Maybe just "finish"? > > John. Repository: rL LLVM https://reviews.llvm.org/D3 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D34444: Teach codegen to work in incremental processing mode.
rjmccall added a comment. In https://reviews.llvm.org/D3#812836, @v.g.vassilev wrote: > In https://reviews.llvm.org/D3#812418, @rjmccall wrote: > > > In https://reviews.llvm.org/D3#795175, @v.g.vassilev wrote: > > > > > @rjmccall, thanks for the prompt and thorough reply. > > > > > > In https://reviews.llvm.org/D3#793311, @rjmccall wrote: > > > > > > > Okay. In that case, I see two problems, one major and one potentially > > > > major. > > > > > > > > > > > > > > > This is a very accurate diagnosis which took us 5 years to discover on > > > an empirical basis ;) > > > > > > You could've asked at any time. :) > > > True. I am not really sure I knew what to ask, though ;) We're open to general "I'm trying to do this and having problems" questions on the mailing lists. You probably would've needed to know to CC me specifically, though; sadly, I can't keep up with all the lists I need to. >> That's quite brittle, because that code is only executed in a code path that >> only you are using, and you're not adding any tests. I would greatly prefer >> a change to IRGen's core assumptions, as suggested. > > I am open to changing this code as well. That should probably be another > review. I agree. Are you comfortable with blocking this review until that lands? It seems like it would significantly change this. >> I feel it is important that there be a way to inform an ASTConsumer that no >> further requests will be made of it, something other than calling its >> destructor. I would like you to make sure that the ASTConsumer interface >> supports that and that that call is not made too soon in your alternate >> processing mode. > > Do you have a preference of a name of this new interface? Maybe just "finish"? John. Repository: rL LLVM https://reviews.llvm.org/D3 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D34444: Teach codegen to work in incremental processing mode.
v.g.vassilev added a comment. In https://reviews.llvm.org/D3#812418, @rjmccall wrote: > In https://reviews.llvm.org/D3#795175, @v.g.vassilev wrote: > > > @rjmccall, thanks for the prompt and thorough reply. > > > > In https://reviews.llvm.org/D3#793311, @rjmccall wrote: > > > > > Okay. In that case, I see two problems, one major and one potentially > > > major. > > > > > > > > > > This is a very accurate diagnosis which took us 5 years to discover on an > > empirical basis ;) > > > You could've asked at any time. :) True. I am not really sure I knew what to ask, though ;) > > >>> The major problem is that, as Richard alludes to, you need to make sure you >>> emit any on-demand definitions that Sema registered with CodeGen during the >>> initial CGM's lifetime but used in later CGMs. >> >> >> >> We bring the CGM state to the subsequent CGMs. See >> https://github.com/vgvassilev/clang/blob/cling-patches/lib/CodeGen/ModuleBuilder.cpp#L138-L160 > > That's quite brittle, because that code is only executed in a code path that > only you are using, and you're not adding any tests. I would greatly prefer > a change to IRGen's core assumptions, as suggested. I am open to changing this code as well. That should probably be another review. > > >>> The potentially major problem is that it is not possible in general to >>> automatically break up a single translation unit into multiple translation >>> units, for two reasons. The big reason is that there is no way to >>> correctly handle entities with non-external linkage that are referenced >>> from two parts of the translation unit without implicitly finding some way >>> to promote them to external linkage, which might open a huge can of worms >>> if you have multiple "outer" translation units. >> >> >> >> We do not have an multiple 'outer' translation units. We have just one >> ever growing TU (which probably invalidates my previous statement that we >> have a distinct TUs) which we send to the RuntimeDyLD allowing only JIT to >> resolve symbols from it. We aid the JIT when resolving symbols with >> internal linkage by changing all internal linkage to external (We haven't >> seen issues with that approach). > > Ah, okay. Yes, that is a completely different translation model from having > distinct TUs. > > IRGen will generally happily emit references to undefined internal objects; > instead of hacking the linkage, you could just clean that up as a post-pass. > Although hacking the linkage as post-pass is reasonable, too. In either > case, you can recognize uses of internal-linkage objects that haven't been > defined yet and report that back to the user. > >>> The lesser reason is that the prefix of a valid translation unit is not >>> necessarily a valid translation unit: for example, a static or inline >>> function can be defined at an arbitrary within the translation unit, i.e. >>> not necessarily before its first use. But if your use case somehow defines >>> away these problems, this might be fine. >> >> >> >> If we end up with a module containing no definition of a symbol and such >> is required, then we complain. So indeed we are defining away this issue. > > Ok. > >>> As a minor request, if you are going to make HandleTranslationUnit no >>> longer the final call on CodeGenerator, please either add a new method that >>> *is* a guaranteed final call or add a new method that does the whole "end a >>> previous part of the translation unit and start a new one" step. >> >> >> >> We can have this but it would be a copy of `HandleTranslationUnit`. The >> `StartModule` interface is the antagonist routine to `ReleaseModule`. If you >> prefer we could merge `StartModule` into `ReleaseModule` adding a flag (or >> reading the value of `isIncrementalProcessingEnabled`). > > I feel it is important that there be a way to inform an ASTConsumer that no > further requests will be made of it, something other than calling its > destructor. I would like you to make sure that the ASTConsumer interface > supports that and that that call is not made too soon in your alternate > processing mode. Do you have a preference of a name of this new interface? Repository: rL LLVM https://reviews.llvm.org/D3 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D34444: Teach codegen to work in incremental processing mode.
rjmccall added a comment. In https://reviews.llvm.org/D3#795175, @v.g.vassilev wrote: > @rjmccall, thanks for the prompt and thorough reply. > > In https://reviews.llvm.org/D3#793311, @rjmccall wrote: > > > Okay. In that case, I see two problems, one major and one potentially > > major. > > > > > This is a very accurate diagnosis which took us 5 years to discover on an > empirical basis ;) You could've asked at any time. :) >> The major problem is that, as Richard alludes to, you need to make sure you >> emit any on-demand definitions that Sema registered with CodeGen during the >> initial CGM's lifetime but used in later CGMs. > > > > We bring the CGM state to the subsequent CGMs. See > https://github.com/vgvassilev/clang/blob/cling-patches/lib/CodeGen/ModuleBuilder.cpp#L138-L160 That's quite brittle, because that code is only executed in a code path that only you are using, and you're not adding any tests. I would greatly prefer a change to IRGen's core assumptions, as suggested. >> The potentially major problem is that it is not possible in general to >> automatically break up a single translation unit into multiple translation >> units, for two reasons. The big reason is that there is no way to correctly >> handle entities with non-external linkage that are referenced from two parts >> of the translation unit without implicitly finding some way to promote them >> to external linkage, which might open a huge can of worms if you have >> multiple "outer" translation units. > > > > We do not have an multiple 'outer' translation units. We have just one ever > growing TU (which probably invalidates my previous statement that we have a > distinct TUs) which we send to the RuntimeDyLD allowing only JIT to resolve > symbols from it. We aid the JIT when resolving symbols with internal linkage > by changing all internal linkage to external (We haven't seen issues with > that approach). Ah, okay. Yes, that is a completely different translation model from having distinct TUs. IRGen will generally happily emit references to undefined internal objects; instead of hacking the linkage, you could just clean that up as a post-pass. Although hacking the linkage as post-pass is reasonable, too. In either case, you can recognize uses of internal-linkage objects that haven't been defined yet and report that back to the user. >> The lesser reason is that the prefix of a valid translation unit is not >> necessarily a valid translation unit: for example, a static or inline >> function can be defined at an arbitrary within the translation unit, i.e. >> not necessarily before its first use. But if your use case somehow defines >> away these problems, this might be fine. > > > > If we end up with a module containing no definition of a symbol and such is > required, then we complain. So indeed we are defining away this issue. Ok. >> As a minor request, if you are going to make HandleTranslationUnit no longer >> the final call on CodeGenerator, please either add a new method that *is* a >> guaranteed final call or add a new method that does the whole "end a >> previous part of the translation unit and start a new one" step. > > > > We can have this but it would be a copy of `HandleTranslationUnit`. The > `StartModule` interface is the antagonist routine to `ReleaseModule`. If you > prefer we could merge `StartModule` into `ReleaseModule` adding a flag (or > reading the value of `isIncrementalProcessingEnabled`). I feel it is important that there be a way to inform an ASTConsumer that no further requests will be made of it, something other than calling its destructor. I would like you to make sure that the ASTConsumer interface supports that and that that call is not made too soon in your alternate processing mode. Repository: rL LLVM https://reviews.llvm.org/D3 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D34444: Teach codegen to work in incremental processing mode.
v.g.vassilev added a comment. ping... Repository: rL LLVM https://reviews.llvm.org/D3 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D34444: Teach codegen to work in incremental processing mode.
v.g.vassilev added a subscriber: karies. v.g.vassilev added a comment. @rjmccall, thanks for the prompt and thorough reply. In https://reviews.llvm.org/D3#793311, @rjmccall wrote: > Okay. In that case, I see two problems, one major and one potentially major. This is a very accurate diagnosis which took us 5 years to discover on an empirical basis ;) > The major problem is that, as Richard alludes to, you need to make sure you > emit any on-demand definitions that Sema registered with CodeGen during the > initial CGM's lifetime but used in later CGMs. We bring the CGM state to the subsequent CGMs. See https://github.com/vgvassilev/clang/blob/cling-patches/lib/CodeGen/ModuleBuilder.cpp#L138-L160 > The potentially major problem is that it is not possible in general to > automatically break up a single translation unit into multiple translation > units, for two reasons. The big reason is that there is no way to correctly > handle entities with non-external linkage that are referenced from two parts > of the translation unit without implicitly finding some way to promote them > to external linkage, which might open a huge can of worms if you have > multiple "outer" translation units. We do not have an multiple 'outer' translation units. We have just one ever growing TU (which probably invalidates my previous statement that we have a distinct TUs) which we send to the RuntimeDyLD allowing only JIT to resolve symbols from it. We aid the JIT when resolving symbols with internal linkage by changing all internal linkage to external (We haven't seen issues with that approach). > The lesser reason is that the prefix of a valid translation unit is not > necessarily a valid translation unit: for example, a static or inline > function can be defined at an arbitrary within the translation unit, i.e. not > necessarily before its first use. But if your use case somehow defines away > these problems, this might be fine. If we end up with a module containing no definition of a symbol and such is required, then we complain. So indeed we are defining away this issue. > As a minor request, if you are going to make HandleTranslationUnit no longer > the final call on CodeGenerator, please either add a new method that *is* a > guaranteed final call or add a new method that does the whole "end a previous > part of the translation unit and start a new one" step. We can have this but it would be a copy of `HandleTranslationUnit`. The `StartModule` interface is the antagonist routine to `ReleaseModule`. If you prefer we could merge `StartModule` into `ReleaseModule` adding a flag (or reading the value of `isIncrementalProcessingEnabled`). (Thanks @karies for helping!) Repository: rL LLVM https://reviews.llvm.org/D3 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D34444: Teach codegen to work in incremental processing mode.
rjmccall added a comment.
Okay. In that case, I see two problems, one major and one potentially major.
The major problem is that, as Richard alludes to, you need to make sure you
emit any on-demand definitions that Sema registered with CodeGen during the
initial CGM's lifetime but used in later CGMs. Probably the easiest way of
doing that is to eliminate CodeGenModule's dependence on having on-demand
definitions pre-registered with it: that is, if it emits a reference to a
declaration whose definition already exists in the AST, CodeGenModule should
use that definition even if e.g. HandleTopLevelDecl has not been called.
Calling HandleTopLevelDecl would only be necessary if the definition is
discovered/instantiated after code is generated for the use. I see little
advantage to the current rule, and doing this would also be a nice
quality-of-life improvement for various out-of-tree projects (including Swift,
but IIRC there are several others) which deserialize references to a complete
AST, and which currently have to walk the definition and recursively register
any definitions used within.
The potentially major problem is that it is not possible in general to
automatically break up a single translation unit into multiple translation
units, for two reasons. The big reason is that there is no way to correctly
handle entities with non-external linkage that are referenced from two parts of
the translation unit without implicitly finding some way to promote them to
external linkage, which might open a huge can of worms if you have multiple
"outer" translation units. The lesser reason is that the prefix of a valid
translation unit is not necessarily a valid translation unit: for example, a
static or inline function can be defined at an arbitrary within the translation
unit, i.e. not necessarily before its first use. But if your use case somehow
defines away these problems, this might be fine.
As a minor request, if you are going to make HandleTranslationUnit no longer
the final call on CodeGenerator, please either add a new method that *is* a
guaranteed final call or add a new method that does the whole "end a previous
part of the translation unit and start a new one" step.
Comment at: lib/CodeGen/ModuleBuilder.cpp:328
+llvm::Module *CodeGenerator::StartModule(const std::string& ModuleName,
+ llvm::LLVMContext& C) {
Why does this one take a const std::string & instead of a StringRef?
Repository:
rL LLVM
https://reviews.llvm.org/D3
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D34444: Teach codegen to work in incremental processing mode.
v.g.vassilev added a comment. We use them as separate translation units due to the fact that we need to survive multiple calls to `HandleEndOfTranslationUnit`. This 'finalizes' the module, and we cannot write to it anymore. Even though we could write to it (this was the case when we were using the old JIT), with the orc infrastructure we generate machine code for the whole module. Repository: rL LLVM https://reviews.llvm.org/D3 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D34444: Teach codegen to work in incremental processing mode.
rjmccall added a comment. What's the relationship between these llvm::Modules that you want to generate? Are you imagining them as separate translation units, or are the subsequent modules more like addenda to the original? Repository: rL LLVM https://reviews.llvm.org/D3 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D34444: Teach codegen to work in incremental processing mode.
rsmith added a comment. This makes sense to me, but I'd like John's input on whether this is a reasonable facility for CodeGen to have, and whether this is sufficient for (for example) inline function definitions to be emitted at the right times into the right `Module`s. (The tests for this appear to be in https://reviews.llvm.org/D34059.) Repository: rL LLVM https://reviews.llvm.org/D3 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D34444: Teach codegen to work in incremental processing mode.
v.g.vassilev updated this revision to Diff 103565.
v.g.vassilev added a comment.
Bring back the initial state of this patch. I accidentally uploaded a wrong
patch here, instead of its dependee...
https://reviews.llvm.org/D3
Files:
include/clang/CodeGen/ModuleBuilder.h
lib/CodeGen/ModuleBuilder.cpp
Index: lib/CodeGen/ModuleBuilder.cpp
===
--- lib/CodeGen/ModuleBuilder.cpp
+++ lib/CodeGen/ModuleBuilder.cpp
@@ -119,6 +119,14 @@
return Builder->GetAddrOfGlobal(global,
ForDefinition_t(isForDefinition));
}
+llvm::Module *StartModule(llvm::StringRef ModuleName, llvm::LLVMContext& C,
+ const CodeGenOptions& CGO) {
+ assert(!M && "Replacing existing Module?");
+ M.reset(new llvm::Module(ModuleName, C));
+ Initialize(*Ctx);
+ return M.get();
+}
+
void Initialize(ASTContext ) override {
Ctx =
@@ -317,6 +325,11 @@
->GetAddrOfGlobal(global, isForDefinition);
}
+llvm::Module *CodeGenerator::StartModule(const std::string& ModuleName,
+ llvm::LLVMContext& C) {
+ return static_cast(this)->StartModule(ModuleName, C);
+}
+
CodeGenerator *clang::CreateLLVMCodeGen(
DiagnosticsEngine , llvm::StringRef ModuleName,
const HeaderSearchOptions ,
Index: include/clang/CodeGen/ModuleBuilder.h
===
--- include/clang/CodeGen/ModuleBuilder.h
+++ include/clang/CodeGen/ModuleBuilder.h
@@ -84,6 +84,11 @@
/// code generator will schedule the entity for emission if a
/// definition has been registered with this code generator.
llvm::Constant *GetAddrOfGlobal(GlobalDecl decl, bool isForDefinition);
+
+ /// Create a new \c llvm::Module after calling HandleTranslationUnit. This
+ /// enable codegen in interactive processing environments.
+ llvm::Module* StartModule(llvm::StringRef ModuleName, llvm::LLVMContext& C,
+const CodeGenOptions& CGO);
};
/// CreateLLVMCodeGen - Create a CodeGenerator instance.
Index: lib/CodeGen/ModuleBuilder.cpp
===
--- lib/CodeGen/ModuleBuilder.cpp
+++ lib/CodeGen/ModuleBuilder.cpp
@@ -119,6 +119,14 @@
return Builder->GetAddrOfGlobal(global, ForDefinition_t(isForDefinition));
}
+llvm::Module *StartModule(llvm::StringRef ModuleName, llvm::LLVMContext& C,
+ const CodeGenOptions& CGO) {
+ assert(!M && "Replacing existing Module?");
+ M.reset(new llvm::Module(ModuleName, C));
+ Initialize(*Ctx);
+ return M.get();
+}
+
void Initialize(ASTContext ) override {
Ctx =
@@ -317,6 +325,11 @@
->GetAddrOfGlobal(global, isForDefinition);
}
+llvm::Module *CodeGenerator::StartModule(const std::string& ModuleName,
+ llvm::LLVMContext& C) {
+ return static_cast(this)->StartModule(ModuleName, C);
+}
+
CodeGenerator *clang::CreateLLVMCodeGen(
DiagnosticsEngine , llvm::StringRef ModuleName,
const HeaderSearchOptions ,
Index: include/clang/CodeGen/ModuleBuilder.h
===
--- include/clang/CodeGen/ModuleBuilder.h
+++ include/clang/CodeGen/ModuleBuilder.h
@@ -84,6 +84,11 @@
/// code generator will schedule the entity for emission if a
/// definition has been registered with this code generator.
llvm::Constant *GetAddrOfGlobal(GlobalDecl decl, bool isForDefinition);
+
+ /// Create a new \c llvm::Module after calling HandleTranslationUnit. This
+ /// enable codegen in interactive processing environments.
+ llvm::Module* StartModule(llvm::StringRef ModuleName, llvm::LLVMContext& C,
+const CodeGenOptions& CGO);
};
/// CreateLLVMCodeGen - Create a CodeGenerator instance.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D34444: Teach codegen to work in incremental processing mode.
v.g.vassilev updated this revision to Diff 103560.
v.g.vassilev added a comment.
Herald added a subscriber: mgorny.
Add a test.
Patch by Axel Naumann!
https://reviews.llvm.org/D3
Files:
lib/CodeGen/CGDeclCXX.cpp
unittests/CodeGen/CMakeLists.txt
unittests/CodeGen/IncrementalProcessingTest.cpp
Index: unittests/CodeGen/IncrementalProcessingTest.cpp
===
--- /dev/null
+++ unittests/CodeGen/IncrementalProcessingTest.cpp
@@ -0,0 +1,177 @@
+//===- unittests/CodeGen/BufferSourceTest.cpp - MemoryBuffer source tests -===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+
+#include "clang/AST/ASTConsumer.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/AST/RecursiveASTVisitor.h"
+#include "clang/Basic/TargetInfo.h"
+#include "clang/CodeGen/ModuleBuilder.h"
+#include "clang/Frontend/CompilerInstance.h"
+#include "clang/Lex/Preprocessor.h"
+#include "clang/Parse/Parser.h"
+#include "clang/Sema/Sema.h"
+#include "llvm/ADT/Triple.h"
+#include "llvm/IR/LLVMContext.h"
+#include "llvm/IR/Module.h"
+#include "llvm/Support/Host.h"
+#include "llvm/Support/MemoryBuffer.h"
+#include "gtest/gtest.h"
+
+#include
+
+using namespace llvm;
+using namespace clang;
+
+namespace {
+
+// Incremental processing produces several modules, all using the same "main
+// file". Make sure CodeGen can cope with that, e.g. for static initializers.
+const char TestProgram1[] =
+"extern \"C\" int funcForProg1() { return 17; }\n"
+"struct EmitCXXGlobalInitFunc1 {\n"
+" EmitCXXGlobalInitFunc1() {}\n"
+"} test1;";
+
+const char TestProgram2[] =
+"extern \"C\" int funcForProg2() { return 42; }\n"
+"struct EmitCXXGlobalInitFunc2 {\n"
+" EmitCXXGlobalInitFunc2() {}\n"
+"} test2;";
+
+
+/// An incremental version of ParseAST().
+static std::unique_ptr
+IncrementalParseAST(CompilerInstance& CI, Parser& P,
+CodeGenerator& CG, const char* code) {
+ static int counter = 0;
+ struct IncreaseCounterOnRet {
+~IncreaseCounterOnRet() {
+ ++counter;
+}
+ } ICOR;
+
+ Sema& S = CI.getSema();
+ clang::SourceManager = S.getSourceManager();
+ if (!code) {
+// Main file
+SM.setMainFileID(SM.createFileID(
+llvm::MemoryBuffer::getMemBuffer(""), clang::SrcMgr::C_User));
+
+S.getPreprocessor().EnterMainSourceFile();
+P.Initialize();
+ } else {
+FileID FID = SM.createFileID(
+llvm::MemoryBuffer::getMemBuffer(code), clang::SrcMgr::C_User);
+SourceLocation MainStartLoc = SM.getLocForStartOfFile(SM.getMainFileID());
+SourceLocation InclLoc = MainStartLoc.getLocWithOffset(counter);
+S.getPreprocessor().EnterSourceFile(FID, 0, InclLoc);
+ }
+
+ ExternalASTSource *External = S.getASTContext().getExternalSource();
+ if (External)
+External->StartTranslationUnit();
+
+ Parser::DeclGroupPtrTy ADecl;
+ for (bool AtEOF = P.ParseFirstTopLevelDecl(ADecl); !AtEOF;
+ AtEOF = P.ParseTopLevelDecl(ADecl)) {
+// If we got a null return and something *was* parsed, ignore it. This
+// is due to a top-level semicolon, an action override, or a parse error
+// skipping something.
+if (ADecl && !CG.HandleTopLevelDecl(ADecl.get()))
+ return false;
+ }
+
+ // Process any TopLevelDecls generated by #pragma weak.
+ for (Decl *D : S.WeakTopLevelDecls())
+CG.HandleTopLevelDecl(DeclGroupRef(D));
+
+ CG.HandleTranslationUnit(S.getASTContext());
+
+ std::unique_ptr M(CG.ReleaseModule());
+ // Switch to next module.
+ CG.StartModule("incremental-module-" + std::to_string(counter),
+ M->getContext(), CI.getCodeGenOpts());
+ return M;
+}
+
+const Function* getGlobalInit(llvm::Module& M) {
+ for (const auto& Func: M)
+if (Func.hasName() && Func.getName().startswith("_GLOBAL__sub_I_"))
+ return
+
+ return nullptr;
+}
+
+TEST(IncrementalProcessing, EmitCXXGlobalInitFunc) {
+LLVMContext Context;
+CompilerInstance compiler;
+
+compiler.createDiagnostics();
+compiler.getLangOpts().CPlusPlus = 1;
+compiler.getLangOpts().CPlusPlus11 = 1;
+
+compiler.getTargetOpts().Triple = llvm::Triple::normalize(
+llvm::sys::getProcessTriple());
+compiler.setTarget(clang::TargetInfo::CreateTargetInfo(
+ compiler.getDiagnostics(),
+ std::make_shared(
+compiler.getTargetOpts(;
+
+compiler.createFileManager();
+compiler.createSourceManager(compiler.getFileManager());
+compiler.createPreprocessor(clang::TU_Prefix);
+compiler.getPreprocessor().enableIncrementalProcessing();
+
+compiler.createASTContext();
+
+CodeGenerator* CG =
+CreateLLVMCodeGen(
+compiler.getDiagnostics(),
+"main-module",
+
[PATCH] D34444: Teach codegen to work in incremental processing mode.
v.g.vassilev updated this revision to Diff 103360.
v.g.vassilev added a comment.
Fix compilation issue.
https://reviews.llvm.org/D3
Files:
include/clang/CodeGen/ModuleBuilder.h
lib/CodeGen/ModuleBuilder.cpp
Index: lib/CodeGen/ModuleBuilder.cpp
===
--- lib/CodeGen/ModuleBuilder.cpp
+++ lib/CodeGen/ModuleBuilder.cpp
@@ -119,6 +119,14 @@
return Builder->GetAddrOfGlobal(global,
ForDefinition_t(isForDefinition));
}
+llvm::Module *StartModule(llvm::StringRef ModuleName, llvm::LLVMContext& C,
+ const CodeGenOptions& CGO) {
+ assert(!M && "Replacing existing Module?");
+ M.reset(new llvm::Module(ModuleName, C));
+ Initialize(*Ctx);
+ return M.get();
+}
+
void Initialize(ASTContext ) override {
Ctx =
@@ -317,6 +325,12 @@
->GetAddrOfGlobal(global, isForDefinition);
}
+llvm::Module *CodeGenerator::StartModule(llvm::StringRef ModuleName,
+ llvm::LLVMContext ,
+ const CodeGenOptions ) {
+ return static_cast(this)->StartModule(ModuleName, C,
CGO);
+}
+
CodeGenerator *clang::CreateLLVMCodeGen(
DiagnosticsEngine , llvm::StringRef ModuleName,
const HeaderSearchOptions ,
Index: include/clang/CodeGen/ModuleBuilder.h
===
--- include/clang/CodeGen/ModuleBuilder.h
+++ include/clang/CodeGen/ModuleBuilder.h
@@ -84,6 +84,11 @@
/// code generator will schedule the entity for emission if a
/// definition has been registered with this code generator.
llvm::Constant *GetAddrOfGlobal(GlobalDecl decl, bool isForDefinition);
+
+ /// Create a new \c llvm::Module after calling HandleTranslationUnit. This
+ /// enable codegen in interactive processing environments.
+ llvm::Module* StartModule(llvm::StringRef ModuleName, llvm::LLVMContext ,
+const CodeGenOptions );
};
/// CreateLLVMCodeGen - Create a CodeGenerator instance.
Index: lib/CodeGen/ModuleBuilder.cpp
===
--- lib/CodeGen/ModuleBuilder.cpp
+++ lib/CodeGen/ModuleBuilder.cpp
@@ -119,6 +119,14 @@
return Builder->GetAddrOfGlobal(global, ForDefinition_t(isForDefinition));
}
+llvm::Module *StartModule(llvm::StringRef ModuleName, llvm::LLVMContext& C,
+ const CodeGenOptions& CGO) {
+ assert(!M && "Replacing existing Module?");
+ M.reset(new llvm::Module(ModuleName, C));
+ Initialize(*Ctx);
+ return M.get();
+}
+
void Initialize(ASTContext ) override {
Ctx =
@@ -317,6 +325,12 @@
->GetAddrOfGlobal(global, isForDefinition);
}
+llvm::Module *CodeGenerator::StartModule(llvm::StringRef ModuleName,
+ llvm::LLVMContext ,
+ const CodeGenOptions ) {
+ return static_cast(this)->StartModule(ModuleName, C, CGO);
+}
+
CodeGenerator *clang::CreateLLVMCodeGen(
DiagnosticsEngine , llvm::StringRef ModuleName,
const HeaderSearchOptions ,
Index: include/clang/CodeGen/ModuleBuilder.h
===
--- include/clang/CodeGen/ModuleBuilder.h
+++ include/clang/CodeGen/ModuleBuilder.h
@@ -84,6 +84,11 @@
/// code generator will schedule the entity for emission if a
/// definition has been registered with this code generator.
llvm::Constant *GetAddrOfGlobal(GlobalDecl decl, bool isForDefinition);
+
+ /// Create a new \c llvm::Module after calling HandleTranslationUnit. This
+ /// enable codegen in interactive processing environments.
+ llvm::Module* StartModule(llvm::StringRef ModuleName, llvm::LLVMContext ,
+const CodeGenOptions );
};
/// CreateLLVMCodeGen - Create a CodeGenerator instance.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D34444: Teach codegen to work in incremental processing mode.
v.g.vassilev created this revision.
When `isIncrementalProcessingEnabled` is on we might call multiple times
`HandleEndOfTranslationUnit`. This would complete the `llvm::Module` CodeGen is
writing to.
This patch allows the clients to start a new `llvm::Module`, allowing CodeGen
to continue writing.
This should give the necessary facilities to write a unittest for
https://reviews.llvm.org/D34059.
Repository:
rL LLVM
https://reviews.llvm.org/D3
Files:
include/clang/CodeGen/ModuleBuilder.h
lib/CodeGen/ModuleBuilder.cpp
Index: lib/CodeGen/ModuleBuilder.cpp
===
--- lib/CodeGen/ModuleBuilder.cpp
+++ lib/CodeGen/ModuleBuilder.cpp
@@ -119,6 +119,14 @@
return Builder->GetAddrOfGlobal(global,
ForDefinition_t(isForDefinition));
}
+llvm::Module *StartModule(llvm::StringRef ModuleName, llvm::LLVMContext& C,
+ const CodeGenOptions& CGO) {
+ assert(!M && "Replacing existing Module?");
+ M.reset(new llvm::Module(ModuleName, C));
+ Initialize(*Ctx);
+ return M.get();
+}
+
void Initialize(ASTContext ) override {
Ctx =
@@ -317,6 +325,11 @@
->GetAddrOfGlobal(global, isForDefinition);
}
+llvm::Module *CodeGenerator::StartModule(const std::string& ModuleName,
+ llvm::LLVMContext& C) {
+ return static_cast(this)->StartModule(ModuleName, C);
+}
+
CodeGenerator *clang::CreateLLVMCodeGen(
DiagnosticsEngine , llvm::StringRef ModuleName,
const HeaderSearchOptions ,
Index: include/clang/CodeGen/ModuleBuilder.h
===
--- include/clang/CodeGen/ModuleBuilder.h
+++ include/clang/CodeGen/ModuleBuilder.h
@@ -84,6 +84,11 @@
/// code generator will schedule the entity for emission if a
/// definition has been registered with this code generator.
llvm::Constant *GetAddrOfGlobal(GlobalDecl decl, bool isForDefinition);
+
+ /// Create a new \c llvm::Module after calling HandleTranslationUnit. This
+ /// enable codegen in interactive processing environments.
+ llvm::Module* StartModule(llvm::StringRef ModuleName, llvm::LLVMContext& C,
+const CodeGenOptions& CGO);
};
/// CreateLLVMCodeGen - Create a CodeGenerator instance.
Index: lib/CodeGen/ModuleBuilder.cpp
===
--- lib/CodeGen/ModuleBuilder.cpp
+++ lib/CodeGen/ModuleBuilder.cpp
@@ -119,6 +119,14 @@
return Builder->GetAddrOfGlobal(global, ForDefinition_t(isForDefinition));
}
+llvm::Module *StartModule(llvm::StringRef ModuleName, llvm::LLVMContext& C,
+ const CodeGenOptions& CGO) {
+ assert(!M && "Replacing existing Module?");
+ M.reset(new llvm::Module(ModuleName, C));
+ Initialize(*Ctx);
+ return M.get();
+}
+
void Initialize(ASTContext ) override {
Ctx =
@@ -317,6 +325,11 @@
->GetAddrOfGlobal(global, isForDefinition);
}
+llvm::Module *CodeGenerator::StartModule(const std::string& ModuleName,
+ llvm::LLVMContext& C) {
+ return static_cast(this)->StartModule(ModuleName, C);
+}
+
CodeGenerator *clang::CreateLLVMCodeGen(
DiagnosticsEngine , llvm::StringRef ModuleName,
const HeaderSearchOptions ,
Index: include/clang/CodeGen/ModuleBuilder.h
===
--- include/clang/CodeGen/ModuleBuilder.h
+++ include/clang/CodeGen/ModuleBuilder.h
@@ -84,6 +84,11 @@
/// code generator will schedule the entity for emission if a
/// definition has been registered with this code generator.
llvm::Constant *GetAddrOfGlobal(GlobalDecl decl, bool isForDefinition);
+
+ /// Create a new \c llvm::Module after calling HandleTranslationUnit. This
+ /// enable codegen in interactive processing environments.
+ llvm::Module* StartModule(llvm::StringRef ModuleName, llvm::LLVMContext& C,
+const CodeGenOptions& CGO);
};
/// CreateLLVMCodeGen - Create a CodeGenerator instance.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
