[PATCH] D130229: [ELF] Add --thinlto-index= and --remapping-file=

[Fangrui Song via Phabricator via llvm-commits]

MaskRay added a comment.

In D130229#3669642 <https://reviews.llvm.org/D130229#3669642>, @tejohnson wrote:

> In D130229#3669613 <https://reviews.llvm.org/D130229#3669613>, @MaskRay wrote:
>
>> In D130229#3668618 <https://reviews.llvm.org/D130229#3668618>, @tejohnson wrote:
>>
>>> I suppose the rationale for doing things this way is that you can keep the input files in the same ordering on the indexing link and the final native link, to avoid changing the relative order between files that were bitcode and files that were native already during the indexing link.
>>
>> Yes, and also to avoid `-nostdlib`.
>>
>>> However, I'm a little confused as to how this will work for symbol resolution. The main reason for doing things the way we were was to ensure the same objects were selected for linking out of static libraries (that use --start-lib/--end-lib), since symbols may be referenced differently after importing. That's why the native objects corresponding to the bitcode files are listed in a flat list in the thinlto-index-only=params file (i.e. not using --start-lib/--end-lib). I'm not sure how the new solution prevents those issues.
>>>
>>> E.g. If I change your example to use --start-lib/--end-lib:
>>>
>>>   # ThinLTO indexing
>>>   clang -flto=thin -fuse-ld=lld -Wl,--thinlto-index=out/exe.map -Wl,--thinlto-prefix-replace='out;lto' \
>>>     -Wl,--thinlto-object-suffix-replace='.indexing.o;.o' out/a.o --start-lib out/b.indexing.o out/c.indexing.o --end-lib
>>>
>>> Is the final link also like that?:
>>
>> Yes for all input files. The final link removes `-flto=thin -Wl,--thinlto-index=out/exe.map` and adds `-Wl,--remapping-file=out/exe.map`
>>
>> `--thinlto-index-only=` reorders bitcode files and native object files, therefore may change symbol resolution. As a compensation, all backend compiled bitcode files drop --start-lib/--end-lib.
>> For `--thinlto-index=`, there is no reordering. I am trying to think of a case where dropping `--start-lib` is a necessity but so far unable to come up with one.
>
> See discussion and examples linked from https://reviews.llvm.org/D22677

I'll analyze the example later. But I haven't seen evidence that this --thinlto-index= and --remapping-file= won't work.

I have written a script to convert Bazel `-Wl,-plugin-opt=thinlto-` options to `-Wl,--thinlto-index=/tmp/c/lto-final.map` and tried the approach with several internal executables.
They do work with a small change that the remapping file ignores all un-extracted minimalized bitcode files.

>> If there is a case, we probably have to give --remapping-file additional semantics that a specified file ignores the surrounding `--start-lib`. This makes  --remapping-file less generic, but we can probably add the third column to indicate whether --start-lib should be ignored.
>
> Can we just have the file generated by thinlto-index-only= also include native objects?
>
>> ! In D130229#3669926 <https://reviews.llvm.org/D130229#3669926>, @weiwang wrote:
>>
>>> ! In D130229#3669826 <https://reviews.llvm.org/D130229#3669826>, @tejohnson wrote:
>>>
>>>> ! In D130229#3669754 <https://reviews.llvm.org/D130229#3669754>, @weiwang wrote:
>>>>
>>>>> ! In D130229#3669642 <https://reviews.llvm.org/D130229#3669642>, @tejohnson wrote:
>>>>>
>>>>> If there is a case, we probably have to give --remapping-file additional semantics that a specified file ignores the surrounding `--start-lib`. This makes  --remapping-file less generic, but we can probably add the third column to indicate whether --start-lib should be ignored.
>>>>
>>>> Can we just have the file generated by thinlto-index-only= also include native objects?
>>>
>>> I agree that making `thinlto-index-only=` to produce the entire input list, both prebuilt native and bitcode-compiled native, is a less disruptive change to still keep the current cmdline options while making it more general. I think we can probably extend the the index file output to also include `--start-lib/--end-lib` grouping to that the final link can just use it as is.
>>
>> Actually we specifically don't want to do that (include the selected objects in --start-lib/--end-lib in the index output), we already know which files the linker chose to include from the archive groupings and we don't want to redo that logic which could theoretically come to a different conclusion.
>
> Right. Linker already fetched those lazy objects, both native and bitcode, when building symbol table during thinlink, so they are not lazy anymore. The index file would include those needed for final linking.

Actually including all native objects will be more intrusive.
We need to consider archives and shared objects as well as ELF relocatable object files and bitcode files.
Archives have the --whole-archive state and shared objects have the --as-needed state.
For a shared object, its soname is dependent on whether it is specified in the -l form.
These state all have to be serialized.

As my first comment says, I have thought about this but I haven't convinced myself that this is a necessity.

Last, right now -nostdlib just controls some -l and startup files. I am slightly concerned whether a toolchain may use -nostdlib to do something differently and make the alternative --thinlto-index-only= (including all native files) more cumbersome to use.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130229