[PATCH] D101011: [Attr] Add "noipa" function attribute

Sun Apr 25 15:03:34 PDT 2021

MaskRay added a comment.

In D101011#2713514 <https://reviews.llvm.org/D101011#2713514>, @dblaikie wrote:

> @MaskRay @serge-sans-paille - you folks have any thoughts on this (see also the specific discussion thread in this review with @JDevlieghere). It looks like this attribute could allow per-function support for "-fsemantic-interposition" that would potentially replace the existing module metadata support for Semantic Interposition, perhaps? Is that feasible, would this be the right behavior? the right design/direction?
>
> (also, I'm considering renaming this to "nointeropt" and changing "optnone" to "nointraopt" for symmetry/clarity (& then implementing clang optnone as "nointeropt+nointraopt"), in case that helps make the names more general/useful for different use cases)

I think the module flag metadata `"SemanticInterposition"` is more of a workaround for the existing (30+) uses for `GlobalValue::isInterposable`.
Many probably should respect the proposed LLVM IR `noipa` (subject to rename) by using a helper function (`>>! In D101011#2713514 <https://reviews.llvm.org/D101011#2713514>, @dblaikie wrote:

> @MaskRay @serge-sans-paille - you folks have any thoughts on this (see also the specific discussion thread in this review with @JDevlieghere). It looks like this attribute could allow per-function support for "-fsemantic-interposition" that would potentially replace the existing module metadata support for Semantic Interposition, perhaps? Is that feasible, would this be the right behavior? the right design/direction?
>
> (also, I'm considering renaming this to "nointeropt" and changing "optnone" to "nointraopt" for symmetry/clarity (& then implementing clang optnone as "nointeropt+nointraopt"), in case that helps make the names more general/useful for different use cases)

I think the module flag metadata `"SemanticInterposition"` is more of a workaround for the existing (30+) uses for `GlobalValue::isInterposable`.
Many probably should respect the proposed LLVM IR `noipa` (subject to rename) by using a helper function (similar to `isDefinitionExact`).
If we simply make `GlobalValue::isInterposable` return false if the global value doesn't have `dso_local`, we may regress some optimization passes.
Ideally the frontend has set dso_local/dso_preemptable correctly so `GlobalValue::isInterposable` shouldn't need to check `"SemanticInterposition"`.

Instrumentation passes can create functions on the fly. They are usually `internal`. If not (I don't know such a case), a module flag metadata serves as the purpose for setting the default dso_local/dso_preemptable.
I don't think such synthesized functions care about dso_local optimizations so this argument retaining `"SemanticInterposition"` is very weak.
If we simply make `GlobalValue::isInterposable` return false if the global value doesn't have `dso_local`, we may regress some optimization passes.
Ideally the frontend has set dso_local/dso_preemptable correctly so `GlobalValue::isInterposable` shouldn't need to check `"SemanticInterposition"`.

Instrumentation passes can create functions on the fly. They are usually `internal`. If not (I don't know such a case), a module flag metadata serves as the purpose for setting the default dso_local/dso_preemptable.
I don't think such synthesized functions care about dso_local optimizations so this argument retaining `"SemanticInterposition"` is very weak.

================
Comment at: llvm/lib/IR/Globals.cpp:329

+  bool GlobalValue::mayBeDerefined() const {
+    switch (getLinkage()) {
----------------
Sent D101264 to refactor this function a bit.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D101011