[PATCH] D72829: Implement -fsemantic-interposition

[Fangrui Song via Phabricator via llvm-commits]

MaskRay added a comment.

The logic looks good to me.

Linkages which were not interposable before and can be interposable now: available_externally, linkonce_odr, weak_odr, external, and appending.

`isDefinitionExact` returned true for external before, and can return false if SemanticInterposition is set.



================
Comment at: clang/include/clang/Driver/Options.td:3266
+def fsemantic_interposition : Flag<["-"], "fsemantic-interposition">, Group<f_Group>, Flags<[CC1Option]>;
+def fno_semantic_interposition: Flag<["-"], "fno-semantic-interposition">, Group<f_Group>, Flags<[CC1Option]>;
 defm non_call_exceptions : BooleanFFlag<"non-call-exceptions">, Group<clang_ignored_f_Group>;
----------------
fno_semantic_interposition does not need to have the CC1Option flag.

In clangDriver you'll need to do:
```lang=cpp
  if (Args.hasFlag(options::OPT_fsemantic_interposition, options::OPT_fno_semantic_interposition, false))
```


================
Comment at: llvm/include/llvm/IR/GlobalValue.h:287
 
+  bool isDSOPreemptable() const { return !IsDSOLocal; }
+
----------------
It seems that this utility is not very necessary.


================
Comment at: llvm/lib/Analysis/InlineCost.cpp:1845
     } else if (GlobalAlias *GA = dyn_cast<GlobalAlias>(V)) {
-      if (GA->isInterposable())
+      if (GA->isInterposable() || GA->isDSOPreemptable())
         break;
----------------
Why this change?


================
Comment at: llvm/lib/IR/Globals.cpp:101
+    return true;
+  return isInterposableLinkage(getLinkage());
+}
----------------
Checking `isInterposableLinkage(getLinkage())` first may be more efficient.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72829