[PATCH] D15390: [ThinLTO] Launch importing backends in parallel threads from gold plugin

[Rafael Espíndola via llvm-commits]

> ================
> Comment at: tools/gold/gold-plugin.cpp:940
> @@ +939,3 @@
> +  std::unique_ptr<llvm::Module> RenamedModule =
> +      renameModuleForThinLTO(M, &CombinedIndex);
> +  if (!RenamedModule)
> ----------------
> rafael wrote:
>> Don't we want something that uses gold's symbol resolution?
>>
>> renameModuleForThinLTO will copy even stuff that gold has marked as preempted, no?
> Unfortunately all the ThinLTO promotion logic and renaming support is in the ModuleLinker, so I couldn't just use IRMover::move with the Keep list.
>
> Perhaps the ModuleLinker should have a mode where all it does is the promotion handling for exporting modules before calling IRMover::move. I.e. I think this would just need to do a version of processGlobalsForThinLTO where locals in a supplied ValuesToLink list (initiallized from the Keep list in gold) would be promoted if necessary. Similar to the existing processGlobalsForThinLTO but only for things already in the supplied ValuesToLink.
>
> Does that sound right?

Not sure. Thinking a bit more about it I think I am missing the big picture.

I was at least under the impression that we could:

* Run the IRMover more or less like we do in normal LTO, but instead
of moving to a merged module, each task gets one file and moves it
into an empty module.
* Run a transformation that updates name and visibility in place.
* For each module we want to cherry pick something, FunctionImport brings it in.


> ================
> Comment at: tools/gold/gold-plugin.cpp:971
> @@ +970,3 @@
> +  if (!options::obj_path.empty())
> +    Filename = options::obj_path;
> +  else if (options::TheOutputType == options::OT_SAVE_TEMPS)
> ----------------
> rafael wrote:
>> This seems incompatible with threading or even multiple outputs, no? It looks like every thread will try to use the same output file name.
>>
>> I would suggest producing an error for now.
> The openOutputFile helper below will append a unique thread ID (should probably change this to TaskID...). Will add comment to that effect. Or do you still think it is better to error?

No, but please add a test :-)

Cheers,
Rafael