[PATCH] D127751: [MergeFunctions] Preserve symbols used llvm.used/llvm.compiler.used

[Amanieu d'Antras via Phabricator via llvm-commits]

Amanieu added inline comments.


================
Comment at: llvm/lib/Transforms/IPO/MergeFunctions.cpp:841
+      // usually from inline asm.
+      if (G->hasGlobalUnnamedAddr() && !Used.contains(G)) {
         // G might have been a key in our GlobalNumberState, and it's illegal
----------------
efriedma wrote:
> I'm a bit suspicious of the hasGlobalUnnamedAddr() check here.  hasGlobalUnnamedAddr() generally implies it's legal to merge two constants/functions, but we don't actually figure out if we're going to perform an actual merge at this point; that doesn't happen until we try to discard it, or call writeThunkOrAlias.  (unnamed_addr only allows rewriting all the uses of a variable/function at once, not just some of them.  Otherwise you end up with weird inconsistencies where a value isn't equal to itself.)
I think the code is valid as it is:
- If hasGlobalUnnamedAddr() == true then we replace all uses of the function with replaceAllUsesWith.
- Otherwise we only replace direct caller, which is equivalent to inlining the thunk/alias.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127751