[PATCH] D65543: [Windows] Autolink with basenames and add libdir to libpath

Thu Oct 3 18:26:27 PDT 2019

rnk added a comment.

In D65543#1691819 <https://reviews.llvm.org/D65543#1691819>, @mstorsjo wrote:

> Another slightly related thread, regarding libs from the clang resource dir and how they are specified to the linker (regarding the builtins library): https://reviews.llvm.org/D51440

I support that effort. :)

In D65543#1687974 <https://reviews.llvm.org/D65543#1687974>, @russell.gallop wrote:

> This does indeed break PGO self-host with lld-link (applied on top of r373200):
>
>   <...>\bin\lld-link.exe /nologo utils\not\CMakeFiles\not.dir\not.cpp.obj utils\not\CMakeFiles\not.dir\__\__\resources\windows_version_resource.rc.res /out:bin\not.exe /implib:lib\not.lib /pdb:bin\not.pdb /version:0.0 /machine:x64 -fuse-ld=lld /STACK:10000000 /INCREMENTAL:NO /subsystem:console lib\LLVMSupport.lib psapi.lib shell32.lib ole32.lib uuid.lib advapi32.lib delayimp.lib -delayload:shell32.dll -delayload:ole32.dll lib\LLVMDemangle.lib kernel32.lib user32.lib gdi32.lib winspool.lib shell32.lib ole32.lib oleaut32.lib uuid.lib comdlg32.lib advapi32.lib /MANIFEST /MANIFESTFILE:bin\not.exe.manifest
>   lld-link: warning: ignoring unknown argument '-fuse-ld=lld'
>   lld-link: error: could not open 'clang_rt.profile-x86_64.lib': no such file or directory
>
>
> So at the very least this change would need that path adding.

I added a fragment to HandleLLVMOptions.cmake to ask clang for its resource directory and add it to the search path when compiling with instrumentation or sanitizers. I think it's not so bad, and we don't have to write down the version.

> I agree with Nico that having to add a path dependent on the LLVM version sounds like a pain.
> 
> Is it possible for the compiler to embed a /libpath as well as the dependent lib? That goes back to having a path embedded, though you could override it if required so could be an improvement over things at the moment.

Nope, I tried it, got this:

  t.obj : warning LNK4229: invalid directive '/libpath:foo' encountered; ignored

> It sounds to me like:
> 
>> ...folks have to explicitly pass /libpath:\path\to\clang\lib\clang\$changing_version\lib\windows to the linker, which to me is a pretty poor experience
> 
> and
> 
>> I'd like to get away from having paths embedded in the object if possible.
> 
> are hard to reconcile.

True.

> It may be possible to have the Windows installer add the path to LIB environment variable but that would rely on having run the installer, and could cause problems if you have multiple versions of LLVM around.

Well, and I wouldn't want to follow Microsoft down the route of increasing dependence on the environment.

In D65543#1687338 <https://reviews.llvm.org/D65543#1687338>, @thakis wrote:

> If you strongly feel that this is the right direction, go for it, you're code owner here :)

I think I do feel strongly about it, but I don't want to be too pushy. :)

> To me, this feels like a regression. This change has no benefit that I can see (at least none that anybody's asked for that I'm aware of – then again I read llvm's bugzilla less than you do), and the drawback that folks have to explicitly pass `/libpath:\path\to\clang\lib\clang\$changing_version\lib\windows` to the linker, which to me is a pretty poor experience. I'd prefer if we used this qualified path for all runtime libs, so that users would never have to use this libpath. (We currently use a qualified path for libprofile but not asan and the like iirc.)

libgcc follows a similar model, FWIW, it lives in a compiler-version-dependent path. The difference just happens to be that, on this particular platform, there's no compiler driver running the link to paper over this detail for the user.

LLVM has a lot of runtime libraries these days:

- profile (pgo & coverage)
- asan, ubsan, msan, tsan, sancov
- libfuzzer
- openmp
- libc++
- parallel stl

It doesn't seem so bad to document that, if you want to use LLVM instrumentation tools, you have to add one library search path. So, speaking of documentation, we don't really have good holistic instructions for how to use clang-cl. Where is the most obvious place to put this? The main one I know about and haven't done yet is the release notes.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D65543