[PATCH] D80406: [LLD][ThinLTO] A switch to allow compilation of only one module.

[Teresa Johnson via Phabricator via llvm-commits]

tejohnson added a comment.

Thanks for adding this, it looks really useful.

Can you beef up the testing? I.e. ensure that the save temps output files for just the specified module get created. Also please add a test to make sure this interacts well with distributed thinlto build options, i.e. -thinlto-index-only (e.g. that it creates the expected output files just for the specified module).



================
Comment at: lld/test/ELF/lto/thinlto-single-module.ll:1
+; REQUIRES: x86
+; RUN: rm -fr %t && mkdir %t && cd %t
----------------
Some comments in this file as to what and how it is testing would be good. I'm don't quite understand how it is testing that the single module option is working.


================
Comment at: llvm/include/llvm/LTO/Config.h:134
+  /// A single thinLTO module to compile.
+  std::string ThinLTOSingleModule;
+
----------------
hoyFB wrote:
> MaskRay wrote:
> > If this can be extended to regular LTO, `ThinLTO*` is not appropriate.
> > 
> > `std::vector<std::string> ModulesToCompile;`
> > 
> > Needs a test specifying the option multiple times.
> That's a good point. So far it's unclear to me what compiling a single module means in mono LTO mode. E.g, should cross-module inlining be allowed which would end up generating object code for other modules? If so, figuring out what inlinees should be compiled need additional work. I'd like to see a strong need first before addressing that, since thinLTO is currently our primary scenario. On the other hand, single module is naturally well-defined in thinLTO mode, and with this new switch we'll end up having exactly same code generated for a module as we would get with a full compilation.
> 
> Allowing multiple modules to be compiled with this switch is an interesting idea!
@MaskRay I'm also not sure how this would work in regular LTO mode. In ThinLTO mode the compilation of the selected module will not be affected by this option - it will still import and optimize the exact same way it normally would for that module. For regularLTO how do you isolate the compilation of one module without changing the way it is optimized? This seems very ThinLTO specific to me.


================
Comment at: llvm/include/llvm/LTO/LTO.h:341
+    ModuleMapType ModuleMap;
+    Optional<ModuleMapType> ModulesToCompile;
     DenseMap<GlobalValue::GUID, StringRef> PrevailingModuleForGUID;
----------------
Please add comment (I realize this structure is missing comments, but think we should start adding them for new fields at least).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80406