[PATCH] D61742: [Driver][Windows] Add dependent lib argument for profile instr generate

Tue May 28 06:46:22 PDT 2019

russell.gallop added a subscriber: ruiu.
russell.gallop added a comment.

> Hello, this embeds an absolute path into the generated .obj file, which means the output now is no longer deterministic (since it depends on the absolute path to clang_rt.profile-x86_64.lib).

Yes, it embeds the absolute path. Note that -fsanitize=undefined has done that since r241225 (in 2015).

> This means the output will be different on different machines, which breaks things like caching in distributed builds and whatnot.

I assume that your distributed build sandboxes tool paths so this is deterministic from one users point of view, just not deterministic across users with tools installed in different locations so it defeats the cache. Yes, that's not ideal.

> I can't see an obvious way to save this patch either, since you won't know at compile time what the CWD will be at link time.

I assume you're considering relative paths here. Yes, I don't think that would work nicely.

> As-is, this breaks Chromium's deterministic builds. ("Only" the coverage builds, and they don't yet check for determinism, which is why it took us a while to notice.)
>  Suggestions?

If this was made into a library name instead of a path (e.g. --dependent-lib=clang_rt.profile-x86_64.lib) then it would solve the problem of different paths on different nodes, but then you would have to tell the linker where to find this library.

That is *almost* as bad as having to specify the full path to the library to the linker in the first place! It's not quite as bad as you don't have to specify the architecture and you only need to specify it once regardless of whether you're using profiling or sanitizers. If lld-link added the clang library path itself, then the library name would be all that was needed from the compiler side.

Rui, would it be possible for lld-link to search the path to the clang libraries (e.g. C:\Program Files\LLVM\lib\clang\8.0.0\lib\windows) by default? Would this have any undesirable side effects?

Repository:
  rC Clang

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

https://reviews.llvm.org/D61742