[PATCH] D86142: [LLD] Search archives for non-tentative defintions.

Mon Aug 24 17:20:20 PDT 2020

jyknight added a comment.

In D86142#2234882 <https://reviews.llvm.org/D86142#2234882>, @MaskRay wrote:

> In D86142#2234472 <https://reviews.llvm.org/D86142#2234472>, @sfertile wrote:
>
>> In D86142#2225679 <https://reviews.llvm.org/D86142#2225679>, @psmith wrote:
>>
>>> Given that BFD supports this behaviour but Gold and LLD do not I suspect that the risk of breaking existing programs is small, especially as LLD tends to be used with more recent programs where -fno-common is the default. However I think we should be cautious and perhaps enable under a command line option, especially as it looks like this is a convention rather than part of a specification. If anyone has better references, then I'm happy to retract.
>>
>> I agree the new behavior needs an option to enable it and disable it. Initially I considered having the Fortran driver pass the option to LLD, but after discussing it offline with @rzurob I think it might be best to enable the behavior by default. The consumer of these libraries aren't necessarily Fortran programs, and as you mentioned its likely to be lower risk because we are adopting a behavior already implemented in ld.bfd.
>
> There are 4 combinations: (ArchiveFile,LazyObjFile) x (ELF, BitcodeFile). LazyObjFile is for --start-lib/--end-lib. Do you think whether it is worth making LazyObjFile or BitcodeFile consistent with ArchiveFile x ELF behavior? If we add an option (say, `--fortran-linking`, the Dec 1999 binutils thread suggested but did not adopt), we will have better justification that we don't make LazyObjFile or BitcodeFile consistent.

IMO, it is important to make it consistent.

> I do worry a bit about the time complexity of the regular-overriding-common behavior, which may be a problem for my internal users (Bazel: --start-lib; we use very few `ArchiveFile`s) and external users (mostly `ArchiveFile`s).

I think there should not be a problem with time complexity, because there is only extra work to do in uncommon situations:

1. Only if you see a common symbol, do you need to try to find additional lazy definitions of the same symbol name. This will be very rare for C code now, due to -fno-common being the default.
2. Only if the common symbol's name is present in another archive symbol-table do you need to load the archive member to check whether that second definition is also common or not. This will be even more rare, since it will only trigger, in C code, when you have an actually-incorrect use (e.g. "int x" in a header instead of "extern int x").

> If we use an option `--fortran-linking`, we can mostly get away with the time complexity problem, because most users will not pass the option and will not observe any potential performance problems. The time complexity problem can be solved by looking up symbols in batch but the overall complexity may be larger.

I think it's unnecessary to have an option, due to the above. C users should not generally observe performance problems with this feature, because it won't trigger for them.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86142