[PATCH] D56015: Allow new comdat symbols to be added as a result of LTO.

Fri Dec 21 13:34:08 PST 2018

xur added a comment.

In D56015#1339541 <https://reviews.llvm.org/D56015#1339541>, @pcc wrote:

> > Why LTO optimization passes should not add new comdat variables? 
> >  I don't see a reason for this.
>
> This is necessary so that all symbol resolution can be done in the linker before LTO, which as a general principle is what the resolution-based LTO API tries to do (IIRC when the gold plugin was changed to use the linker's symbol resolutions that ended up fixing a lot of bugs, and the same principle was used to design the new LTO API). Resolving comdats again after LTO wouldn't entirely solve the problem, I believe: I expect that you'd still hit a duplicate symbol error if the linker is passed bitcode files together with a regular object file with the PGO instrumentation information.

Is't the final link of the native objects will also do the resolution for the symbols. If the lto api keeps the comdat property of the symbols, they will be work fine when linking with regular object with the same comdat symbol.

In the absence of this patch, we also need final link to do the comdat resolution if we mixed bitcode with native object, right?

As you mentioned gold, gold works with this scheme. I've been using gold as the linker in my patch for a few months.

I think using a comdat variable is a convenient way to sync the actions b/w different compilation units.

> 
> 
>> For my use case, I create a comdat variable to encoce PGO instrumentation information (format, version etc) in each module.
>>  I rely linker to pick one definition. This pass is now in lto backend.
> 
> Can you add the comdat in a pass that runs at compile time?

The instrumentation transformation and creating of the this symbol are all in one pass. The structure will be weird if I split them (not to mention the handling of options)

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

https://reviews.llvm.org/D56015