[PATCH] D97432: [SanitizerCoverage] Clarify llvm.used/llvm.compiler.used and fix unmatched metadata sections on Windows

[Fangrui Song via Phabricator via llvm-commits]

MaskRay marked 2 inline comments as done.
MaskRay added inline comments.


================
Comment at: llvm/lib/Transforms/Instrumentation/SanitizerCoverage.cpp:688
+    GlobalsToAppendToUsed.push_back(Array);
+    GlobalsToAppendToCompilerUsed.push_back(Array);
+  }
----------------
morehouse wrote:
> Don't we still need compiler used?  https://llvm.org/docs/LangRef.html#associated-metadata
> 
> Also even without this change they are only marked "used" on Mac currently, IIRC due to GC issues with their linker.
Yes, I realized that llvm.compiler.used is needed to prevent agggressive optimizations from GlobalOpt/ConstantMerge.

I clarified why `llvm.used` is used - and use it on a case which may be an existing bug on Wondows.


================
Comment at: llvm/lib/Transforms/Instrumentation/SanitizerCoverage.cpp:690
+  }
 
   return Array;
----------------
morehouse wrote:
> What happened to the associated metadata that's added here at ToT?
I think `!associated` cannot be used when inlining is possible. See D97430.

This means even interposable linkages (which are normally not inlinable) can be inlined with LTO, so `!associated` cannot be used at all.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97432