[PATCH] D76802: [InstrProfiling] Use !associated metadata for counters, data and values

Mon Jan 25 11:51:22 PST 2021

MaskRay added a comment.

> The problem I can see is that -fuse-ld=lld is usually only passed in linker flags (see for example https://source.chromium.org/chromium/chromium/src/+/master:build/config/compiler/BUILD.gn;l=314), but unless LTO is being used, we'd need to know which linker is going to be used during compilation, that is we'd need it to be passed as a compiler flag.

Even for LTO, -fuse-ld=lld should have minimum behavior difference when using LLVMgold.so.

> Since binutils 2.36 was released yesterday, we could also just consider emitting SHF_LINKER_ORDER unconditionally which would avoid this issue altogether. The only concern there is if someone tried to use new Clang with older binutils, but it sounds like with sanitizers we've ignored that concern it wasn't a problem, so maybe it won't be a problem here either?

It might cause friction to PGO+non-LLD users... I think it might be premature for LLVM 13.0.0. 14.0.0 may be fine.

You may add tests to `linkage.ll`  (see D84723 <https://reviews.llvm.org/D84723>).

With the current path, `opt -counter-associated-metadata=1 < linkage.ll -mtriple=x86_64-unknown-linux -instrprof -S | llc -filetype=obj` crashes due to an empty metadata node.

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

https://reviews.llvm.org/D76802