subject:"\[PATCH\] D72829\: Implement \-fsemantic\-interposition"

[PATCH] D72829: Implement -fsemantic-interposition

2020-01-31 Thread serge via Phabricator via cfe-commits

This revision was automatically updated to reflect the committed changes.
Closed by commit rGfd09f12f32f5: Implement -fsemantic-interposition (authored 
by serge-sans-paille).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72829

Files:
  clang/docs/ClangCommandLineReference.rst
  clang/include/clang/Basic/LangOptions.def
  clang/include/clang/Driver/Options.td
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/test/CodeGen/semantic-interposition.c
  clang/test/Driver/clang_f_opts.c
  llvm/include/llvm/IR/GlobalValue.h
  llvm/include/llvm/IR/Module.h
  llvm/lib/IR/Globals.cpp
  llvm/lib/IR/Module.cpp
  llvm/lib/IR/Verifier.cpp
  llvm/test/Transforms/Inline/inline-semantic-interposition.ll
  llvm/test/Verifier/module-flags-semantic-interposition.ll

Index: llvm/test/Verifier/module-flags-semantic-interposition.ll
===
--- /dev/null
+++ llvm/test/Verifier/module-flags-semantic-interposition.ll
@@ -0,0 +1,12 @@
+; RUN: not llvm-as < %s -o /dev/null 2>&1 | FileCheck %s
+
+target datalayout = "e-m:e-p270:32:32-p271:32:32-p272:64:64-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-unknown-linux-gnu"
+
+@foo = dso_local global i32 1, align 4
+
+!llvm.module.flags = !{!0}
+
+!0 = !{i32 1, !"SemanticInterposition", float 1.}
+
+; CHECK: SemanticInterposition metadata requires constant integer argument
Index: llvm/test/Transforms/Inline/inline-semantic-interposition.ll
===
--- /dev/null
+++ llvm/test/Transforms/Inline/inline-semantic-interposition.ll
@@ -0,0 +1,26 @@
+; Check that @callee1 gets inlined while @callee2 is not, because of
+; SemanticInterposition.
+
+; RUN: opt < %s -inline -S | FileCheck %s
+
+define internal i32 @callee1(i32 %A) {
+  ret i32 %A
+}
+
+define i32 @callee2(i32 %A) {
+  ret i32 %A
+}
+
+; CHECK-LABEL: @caller
+define i32 @caller(i32 %A) {
+; CHECK-NOT: call i32 @callee1(i32 %A)
+  %A1 = call i32 @callee1(i32 %A)
+; CHECK: %A2 = call i32 @callee2(i32 %A)
+  %A2 = call i32 @callee2(i32 %A)
+; CHECK: add i32 %A, %A2
+  %R = add i32 %A1, %A2
+  ret i32 %R
+}
+
+!llvm.module.flags = !{!0}
+!0 = !{i32 1, !"SemanticInterposition", i32 1}
Index: llvm/lib/IR/Verifier.cpp
===
--- llvm/lib/IR/Verifier.cpp
+++ llvm/lib/IR/Verifier.cpp
@@ -1476,6 +1476,13 @@
"'Linker Options' named metadata no longer supported");
   }
 
+  if (ID->getString() == "SemanticInterposition") {
+ConstantInt *Value =
+mdconst::dyn_extract_or_null(Op->getOperand(2));
+Assert(Value,
+   "SemanticInterposition metadata requires constant integer argument");
+  }
+
   if (ID->getString() == "CG Profile") {
 for (const MDOperand  : cast(Op->getOperand(2))->operands())
   visitModuleFlagCGProfileEntry(MDO);
Index: llvm/lib/IR/Module.cpp
===
--- llvm/lib/IR/Module.cpp
+++ llvm/lib/IR/Module.cpp
@@ -555,6 +555,20 @@
: getModuleFlag("ProfileSummary"));
 }
 
+bool Module::getSemanticInterposition() const {
+  Metadata *MF = getModuleFlag("SemanticInterposition");
+
+  auto *Val = cast_or_null(MF);
+  if (!Val)
+return false;
+
+  return cast(Val->getValue())->getZExtValue();
+}
+
+void Module::setSemanticInterposition(bool SI) {
+  addModuleFlag(ModFlagBehavior::Error, "SemanticInterposition", SI);
+}
+
 void Module::setOwnedMemoryBuffer(std::unique_ptr MB) {
   OwnedMemoryBuffer = std::move(MB);
 }
Index: llvm/lib/IR/Globals.cpp
===
--- llvm/lib/IR/Globals.cpp
+++ llvm/lib/IR/Globals.cpp
@@ -94,6 +94,13 @@
   llvm_unreachable("not a global");
 }
 
+bool GlobalValue::isInterposable() const {
+  if (isInterposableLinkage(getLinkage()))
+return true;
+  return getParent() && getParent()->getSemanticInterposition() &&
+ !isDSOLocal();
+}
+
 unsigned GlobalValue::getAlignment() const {
   if (auto *GA = dyn_cast(this)) {
 // In general we cannot compute this at the IR level, but we try.
Index: llvm/include/llvm/IR/Module.h
===
--- llvm/include/llvm/IR/Module.h
+++ llvm/include/llvm/IR/Module.h
@@ -848,6 +848,12 @@
   Metadata *getProfileSummary(bool IsCS);
   /// @}
 
+  /// Returns whether semantic interposition is to be respected.
+  bool getSemanticInterposition() const;
+
+  /// Set whether semantic interposition is to be respected.
+  void setSemanticInterposition(bool);
+
   /// Returns true if PLT should be avoided for RTLib calls.
   bool getRtLibUseGOT() const;
 
Index: llvm/include/llvm/IR/GlobalValue.h
===
---

[PATCH] D72829: Implement -fsemantic-interposition

2020-01-29 Thread pre-merge checks [bot] via Phabricator via cfe-commits

merge_guards_bot added a comment.

{icon check-circle color=green} Unit tests: pass. 62325 tests passed, 0 failed 
and 838 were skipped.

{icon check-circle color=green} clang-tidy: pass.

{icon check-circle color=green} clang-format: pass.

Build artifacts 
: 
diff.json 
,
 clang-tidy.txt 
,
 clang-format.patch 
,
 CMakeCache.txt 
,
 console-log.txt 
,
 test-results.xml 


//Pre-merge checks is in beta. Report issue 
.
 Please join beta  or enable 
it for your project 
.//


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72829



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D72829: Implement -fsemantic-interposition

2020-01-29 Thread serge via Phabricator via cfe-commits

serge-sans-paille updated this revision to Diff 241351.
serge-sans-paille added a comment.

Fix documentation ordering, thank goes to @arichardson for spotting that.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72829

Files:
  clang/docs/ClangCommandLineReference.rst
  clang/include/clang/Basic/LangOptions.def
  clang/include/clang/Driver/Options.td
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/test/CodeGen/semantic-interposition.c
  clang/test/Driver/clang_f_opts.c
  llvm/include/llvm/IR/GlobalValue.h
  llvm/include/llvm/IR/Module.h
  llvm/lib/IR/Globals.cpp
  llvm/lib/IR/Module.cpp
  llvm/lib/IR/Verifier.cpp
  llvm/test/Transforms/Inline/inline-semantic-interposition.ll
  llvm/test/Verifier/module-flags-semantic-interposition.ll

Index: llvm/test/Verifier/module-flags-semantic-interposition.ll
===
--- /dev/null
+++ llvm/test/Verifier/module-flags-semantic-interposition.ll
@@ -0,0 +1,12 @@
+; RUN: not llvm-as < %s -o /dev/null 2>&1 | FileCheck %s
+
+target datalayout = "e-m:e-p270:32:32-p271:32:32-p272:64:64-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-unknown-linux-gnu"
+
+@foo = dso_local global i32 1, align 4
+
+!llvm.module.flags = !{!0}
+
+!0 = !{i32 1, !"SemanticInterposition", float 1.}
+
+; CHECK: SemanticInterposition metadata requires constant integer argument
Index: llvm/test/Transforms/Inline/inline-semantic-interposition.ll
===
--- /dev/null
+++ llvm/test/Transforms/Inline/inline-semantic-interposition.ll
@@ -0,0 +1,26 @@
+; Check that @callee1 gets inlined while @callee2 is not, because of
+; SemanticInterposition.
+
+; RUN: opt < %s -inline -S | FileCheck %s
+
+define internal i32 @callee1(i32 %A) {
+  ret i32 %A
+}
+
+define i32 @callee2(i32 %A) {
+  ret i32 %A
+}
+
+; CHECK-LABEL: @caller
+define i32 @caller(i32 %A) {
+; CHECK-NOT: call i32 @callee1(i32 %A)
+  %A1 = call i32 @callee1(i32 %A)
+; CHECK: %A2 = call i32 @callee2(i32 %A)
+  %A2 = call i32 @callee2(i32 %A)
+; CHECK: add i32 %A, %A2
+  %R = add i32 %A1, %A2
+  ret i32 %R
+}
+
+!llvm.module.flags = !{!0}
+!0 = !{i32 1, !"SemanticInterposition", i32 1}
Index: llvm/lib/IR/Verifier.cpp
===
--- llvm/lib/IR/Verifier.cpp
+++ llvm/lib/IR/Verifier.cpp
@@ -1476,6 +1476,13 @@
"'Linker Options' named metadata no longer supported");
   }
 
+  if (ID->getString() == "SemanticInterposition") {
+ConstantInt *Value =
+mdconst::dyn_extract_or_null(Op->getOperand(2));
+Assert(Value,
+   "SemanticInterposition metadata requires constant integer argument");
+  }
+
   if (ID->getString() == "CG Profile") {
 for (const MDOperand  : cast(Op->getOperand(2))->operands())
   visitModuleFlagCGProfileEntry(MDO);
Index: llvm/lib/IR/Module.cpp
===
--- llvm/lib/IR/Module.cpp
+++ llvm/lib/IR/Module.cpp
@@ -554,6 +554,20 @@
: getModuleFlag("ProfileSummary"));
 }
 
+bool Module::getSemanticInterposition() const {
+  Metadata *MF = getModuleFlag("SemanticInterposition");
+
+  auto *Val = cast_or_null(MF);
+  if (!Val)
+return false;
+
+  return cast(Val->getValue())->getZExtValue();
+}
+
+void Module::setSemanticInterposition(bool SI) {
+  addModuleFlag(ModFlagBehavior::Error, "SemanticInterposition", SI);
+}
+
 void Module::setOwnedMemoryBuffer(std::unique_ptr MB) {
   OwnedMemoryBuffer = std::move(MB);
 }
Index: llvm/lib/IR/Globals.cpp
===
--- llvm/lib/IR/Globals.cpp
+++ llvm/lib/IR/Globals.cpp
@@ -94,6 +94,13 @@
   llvm_unreachable("not a global");
 }
 
+bool GlobalValue::isInterposable() const {
+  if (isInterposableLinkage(getLinkage()))
+return true;
+  return getParent() && getParent()->getSemanticInterposition() &&
+ !isDSOLocal();
+}
+
 unsigned GlobalValue::getAlignment() const {
   if (auto *GA = dyn_cast(this)) {
 // In general we cannot compute this at the IR level, but we try.
Index: llvm/include/llvm/IR/Module.h
===
--- llvm/include/llvm/IR/Module.h
+++ llvm/include/llvm/IR/Module.h
@@ -848,6 +848,12 @@
   Metadata *getProfileSummary(bool IsCS);
   /// @}
 
+  /// Returns whether semantic interposition is to be respected.
+  bool getSemanticInterposition() const;
+
+  /// Set whether semantic interposition is to be respected.
+  void setSemanticInterposition(bool);
+
   /// Returns true if PLT should be avoided for RTLib calls.
   bool getRtLibUseGOT() const;
 
Index: llvm/include/llvm/IR/GlobalValue.h
===
--- llvm/include/llvm/IR/GlobalValue.h
+++

[PATCH] D72829: Implement -fsemantic-interposition

2020-01-29 Thread Alexander Richardson via Phabricator via cfe-commits

arichardson added a comment.

Thanks documentation looks much better now.

Comment at: clang/docs/ClangCommandLineReference.rst:907
+
+.. option:: -fsemantic-interposition, -fno-semantic-interposition
+

arichardson wrote:
> arichardson wrote:
> > This looks like it should be reordered?
> Sorry I mean that it looks like all other entries are `.. option::` followed 
> by the description but here it's the other way around. It seems like this 
> change will make the documention of -fsanitize-* be `Enable semantic 
> interposition`.
It still looks like the documentation here is for the wrong option. I think it 
should be:

```
.. option:: -fsanitize=,..., -fno-sanitize=,...

Turn on runtime checks for various forms of undefined or suspicious behavior. 
See user manual for available checks

.. option:: -fno-semantic-interposition, -fsemantic-interposition

Enable semantic interposition. Semantic interposition allows for the
interposition of a symbol by another at run time, thus preventing a range of
inter-procedural optimisation.
```

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72829

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D72829: Implement -fsemantic-interposition

2020-01-29 Thread pre-merge checks [bot] via Phabricator via cfe-commits

merge_guards_bot added a comment.

{icon check-circle color=green} Unit tests: pass. 62312 tests passed, 0 failed 
and 838 were skipped.

{icon check-circle color=green} clang-tidy: pass.

{icon check-circle color=green} clang-format: pass.

Build artifacts 
: 
diff.json 
,
 clang-tidy.txt 
,
 clang-format.patch 
,
 CMakeCache.txt 
,
 console-log.txt 
,
 test-results.xml 


//Pre-merge checks is in beta. Report issue 
.
 Please join beta  or enable 
it for your project 
.//


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72829



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D72829: Implement -fsemantic-interposition

2020-01-29 Thread serge via Phabricator via cfe-commits

serge-sans-paille updated this revision to Diff 241254.
serge-sans-paille added a comment.

take @sfertile review into account.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72829

Files:
  clang/docs/ClangCommandLineReference.rst
  clang/include/clang/Basic/LangOptions.def
  clang/include/clang/Driver/Options.td
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/test/CodeGen/semantic-interposition.c
  clang/test/Driver/clang_f_opts.c
  llvm/include/llvm/IR/GlobalValue.h
  llvm/include/llvm/IR/Module.h
  llvm/lib/IR/Globals.cpp
  llvm/lib/IR/Module.cpp
  llvm/lib/IR/Verifier.cpp
  llvm/test/Transforms/Inline/inline-semantic-interposition.ll
  llvm/test/Verifier/module-flags-semantic-interposition.ll

Index: llvm/test/Verifier/module-flags-semantic-interposition.ll
===
--- /dev/null
+++ llvm/test/Verifier/module-flags-semantic-interposition.ll
@@ -0,0 +1,12 @@
+; RUN: not llvm-as < %s -o /dev/null 2>&1 | FileCheck %s
+
+target datalayout = "e-m:e-p270:32:32-p271:32:32-p272:64:64-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-unknown-linux-gnu"
+
+@foo = dso_local global i32 1, align 4
+
+!llvm.module.flags = !{!0}
+
+!0 = !{i32 1, !"SemanticInterposition", float 1.}
+
+; CHECK: SemanticInterposition metadata requires constant integer argument
Index: llvm/test/Transforms/Inline/inline-semantic-interposition.ll
===
--- /dev/null
+++ llvm/test/Transforms/Inline/inline-semantic-interposition.ll
@@ -0,0 +1,26 @@
+; Check that @callee1 gets inlined while @callee2 is not, because of
+; SemanticInterposition.
+
+; RUN: opt < %s -inline -S | FileCheck %s
+
+define internal i32 @callee1(i32 %A) {
+  ret i32 %A
+}
+
+define i32 @callee2(i32 %A) {
+  ret i32 %A
+}
+
+; CHECK-LABEL: @caller
+define i32 @caller(i32 %A) {
+; CHECK-NOT: call i32 @callee1(i32 %A)
+  %A1 = call i32 @callee1(i32 %A)
+; CHECK: %A2 = call i32 @callee2(i32 %A)
+  %A2 = call i32 @callee2(i32 %A)
+; CHECK: add i32 %A, %A2
+  %R = add i32 %A1, %A2
+  ret i32 %R
+}
+
+!llvm.module.flags = !{!0}
+!0 = !{i32 1, !"SemanticInterposition", i32 1}
Index: llvm/lib/IR/Verifier.cpp
===
--- llvm/lib/IR/Verifier.cpp
+++ llvm/lib/IR/Verifier.cpp
@@ -1476,6 +1476,13 @@
"'Linker Options' named metadata no longer supported");
   }
 
+  if (ID->getString() == "SemanticInterposition") {
+ConstantInt *Value =
+mdconst::dyn_extract_or_null(Op->getOperand(2));
+Assert(Value,
+   "SemanticInterposition metadata requires constant integer argument");
+  }
+
   if (ID->getString() == "CG Profile") {
 for (const MDOperand  : cast(Op->getOperand(2))->operands())
   visitModuleFlagCGProfileEntry(MDO);
Index: llvm/lib/IR/Module.cpp
===
--- llvm/lib/IR/Module.cpp
+++ llvm/lib/IR/Module.cpp
@@ -554,6 +554,20 @@
: getModuleFlag("ProfileSummary"));
 }
 
+bool Module::getSemanticInterposition() const {
+  Metadata *MF = getModuleFlag("SemanticInterposition");
+
+  auto *Val = cast_or_null(MF);
+  if (!Val)
+return false;
+
+  return cast(Val->getValue())->getZExtValue();
+}
+
+void Module::setSemanticInterposition(bool SI) {
+  addModuleFlag(ModFlagBehavior::Error, "SemanticInterposition", SI);
+}
+
 void Module::setOwnedMemoryBuffer(std::unique_ptr MB) {
   OwnedMemoryBuffer = std::move(MB);
 }
Index: llvm/lib/IR/Globals.cpp
===
--- llvm/lib/IR/Globals.cpp
+++ llvm/lib/IR/Globals.cpp
@@ -94,6 +94,13 @@
   llvm_unreachable("not a global");
 }
 
+bool GlobalValue::isInterposable() const {
+  if (isInterposableLinkage(getLinkage()))
+return true;
+  return getParent() && getParent()->getSemanticInterposition() &&
+ !isDSOLocal();
+}
+
 unsigned GlobalValue::getAlignment() const {
   if (auto *GA = dyn_cast(this)) {
 // In general we cannot compute this at the IR level, but we try.
Index: llvm/include/llvm/IR/Module.h
===
--- llvm/include/llvm/IR/Module.h
+++ llvm/include/llvm/IR/Module.h
@@ -848,6 +848,12 @@
   Metadata *getProfileSummary(bool IsCS);
   /// @}
 
+  /// Returns whether semantic interposition is to be respected.
+  bool getSemanticInterposition() const;
+
+  /// Set whether semantic interposition is to be respected.
+  void setSemanticInterposition(bool);
+
   /// Returns true if PLT should be avoided for RTLib calls.
   bool getRtLibUseGOT() const;
 
Index: llvm/include/llvm/IR/GlobalValue.h
===
--- llvm/include/llvm/IR/GlobalValue.h
+++ llvm/include/llvm/IR/GlobalValue.h
@@

[PATCH] D72829: Implement -fsemantic-interposition

2020-01-29 Thread Sean Fertile via Phabricator via cfe-commits

sfertile added a comment.

In D72829#1846353 , @arichardson wrote:

> As this is user-facing documentation I feel like there should be a slightly 
> longer explaning what the option does.

+1 on this, otherwise LGTM. Thanks for implementing this!

Comment at: llvm/include/llvm/IR/GlobalValue.h:426
   /// Return true if this global's definition can be substituted with an
   /// *arbitrary* definition at link time.  We cannot do any IPO or inlinining
   /// across interposable call edges, since the callee can be replaced with

Minor nit: We have extended this to now determine if a definition can be 
substituted with an arbitrary definition at link time or load time. 

I would suggest changing the first `link time` to `link time or load time`, and 
changing
`since the callee can be replaced with something arbitrary at link time.` to 
`since the callee can be replaced with something arbitrary.`,

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72829

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D72829: Implement -fsemantic-interposition

2020-01-29 Thread pre-merge checks [bot] via Phabricator via cfe-commits

merge_guards_bot added a comment.

{icon check-circle color=green} Unit tests: pass. 62300 tests passed, 0 failed 
and 837 were skipped.

{icon check-circle color=green} clang-tidy: pass.

{icon check-circle color=green} clang-format: pass.

Build artifacts 
: 
diff.json 
,
 clang-tidy.txt 
,
 clang-format.patch 
,
 CMakeCache.txt 
,
 console-log.txt 
,
 test-results.xml 


//Pre-merge checks is in beta. Report issue 
.
 Please join beta  or enable 
it for your project 
.//


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72829



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D72829: Implement -fsemantic-interposition

2020-01-29 Thread serge via Phabricator via cfe-commits

serge-sans-paille added a comment.

> As this is user-facing documentation I feel like there should be a slightly 
> longer explaning what the option does.

done!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72829



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D72829: Implement -fsemantic-interposition

2020-01-29 Thread serge via Phabricator via cfe-commits

serge-sans-paille updated this revision to Diff 241152.
serge-sans-paille added a comment.

Update user-level documentation.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72829

Files:
  clang/docs/ClangCommandLineReference.rst
  clang/include/clang/Basic/LangOptions.def
  clang/include/clang/Driver/Options.td
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/test/CodeGen/semantic-interposition.c
  clang/test/Driver/clang_f_opts.c
  llvm/include/llvm/IR/GlobalValue.h
  llvm/include/llvm/IR/Module.h
  llvm/lib/IR/Globals.cpp
  llvm/lib/IR/Module.cpp
  llvm/lib/IR/Verifier.cpp
  llvm/test/Transforms/Inline/inline-semantic-interposition.ll
  llvm/test/Verifier/module-flags-semantic-interposition.ll

Index: llvm/test/Verifier/module-flags-semantic-interposition.ll
===
--- /dev/null
+++ llvm/test/Verifier/module-flags-semantic-interposition.ll
@@ -0,0 +1,12 @@
+; RUN: not llvm-as < %s -o /dev/null 2>&1 | FileCheck %s
+
+target datalayout = "e-m:e-p270:32:32-p271:32:32-p272:64:64-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-unknown-linux-gnu"
+
+@foo = dso_local global i32 1, align 4
+
+!llvm.module.flags = !{!0}
+
+!0 = !{i32 1, !"SemanticInterposition", float 1.}
+
+; CHECK: SemanticInterposition metadata requires constant integer argument
Index: llvm/test/Transforms/Inline/inline-semantic-interposition.ll
===
--- /dev/null
+++ llvm/test/Transforms/Inline/inline-semantic-interposition.ll
@@ -0,0 +1,26 @@
+; Check that @callee1 gets inlined while @callee2 is not, because of
+; SemanticInterposition.
+
+; RUN: opt < %s -inline -S | FileCheck %s
+
+define internal i32 @callee1(i32 %A) {
+  ret i32 %A
+}
+
+define i32 @callee2(i32 %A) {
+  ret i32 %A
+}
+
+; CHECK-LABEL: @caller
+define i32 @caller(i32 %A) {
+; CHECK-NOT: call i32 @callee1(i32 %A)
+  %A1 = call i32 @callee1(i32 %A)
+; CHECK: %A2 = call i32 @callee2(i32 %A)
+  %A2 = call i32 @callee2(i32 %A)
+; CHECK: add i32 %A, %A2
+  %R = add i32 %A1, %A2
+  ret i32 %R
+}
+
+!llvm.module.flags = !{!0}
+!0 = !{i32 1, !"SemanticInterposition", i32 1}
Index: llvm/lib/IR/Verifier.cpp
===
--- llvm/lib/IR/Verifier.cpp
+++ llvm/lib/IR/Verifier.cpp
@@ -1476,6 +1476,13 @@
"'Linker Options' named metadata no longer supported");
   }
 
+  if (ID->getString() == "SemanticInterposition") {
+ConstantInt *Value =
+mdconst::dyn_extract_or_null(Op->getOperand(2));
+Assert(Value,
+   "SemanticInterposition metadata requires constant integer argument");
+  }
+
   if (ID->getString() == "CG Profile") {
 for (const MDOperand  : cast(Op->getOperand(2))->operands())
   visitModuleFlagCGProfileEntry(MDO);
Index: llvm/lib/IR/Module.cpp
===
--- llvm/lib/IR/Module.cpp
+++ llvm/lib/IR/Module.cpp
@@ -554,6 +554,20 @@
: getModuleFlag("ProfileSummary"));
 }
 
+bool Module::getSemanticInterposition() const {
+  Metadata *MF = getModuleFlag("SemanticInterposition");
+
+  auto *Val = cast_or_null(MF);
+  if (!Val)
+return false;
+
+  return cast(Val->getValue())->getZExtValue();
+}
+
+void Module::setSemanticInterposition(bool SI) {
+  addModuleFlag(ModFlagBehavior::Error, "SemanticInterposition", SI);
+}
+
 void Module::setOwnedMemoryBuffer(std::unique_ptr MB) {
   OwnedMemoryBuffer = std::move(MB);
 }
Index: llvm/lib/IR/Globals.cpp
===
--- llvm/lib/IR/Globals.cpp
+++ llvm/lib/IR/Globals.cpp
@@ -94,6 +94,13 @@
   llvm_unreachable("not a global");
 }
 
+bool GlobalValue::isInterposable() const {
+  if (isInterposableLinkage(getLinkage()))
+return true;
+  return getParent() && getParent()->getSemanticInterposition() &&
+ !isDSOLocal();
+}
+
 unsigned GlobalValue::getAlignment() const {
   if (auto *GA = dyn_cast(this)) {
 // In general we cannot compute this at the IR level, but we try.
Index: llvm/include/llvm/IR/Module.h
===
--- llvm/include/llvm/IR/Module.h
+++ llvm/include/llvm/IR/Module.h
@@ -848,6 +848,12 @@
   Metadata *getProfileSummary(bool IsCS);
   /// @}
 
+  /// Returns whether semantic interposition is to be respected.
+  bool getSemanticInterposition() const;
+
+  /// Set whether semantic interposition is to be respected.
+  void setSemanticInterposition(bool);
+
   /// Returns true if PLT should be avoided for RTLib calls.
   bool getRtLibUseGOT() const;
 
Index: llvm/include/llvm/IR/GlobalValue.h
===
--- llvm/include/llvm/IR/GlobalValue.h
+++ llvm/include/llvm/IR/GlobalValue.h
@@

[PATCH] D72829: Implement -fsemantic-interposition

2020-01-29 Thread Sean Fertile via Phabricator via cfe-commits

sfertile added a comment.

Aewsome, thanks for implementing this. I was on vacation for a bit and somehow 
missed this review in my queue when I cam back but having a look at it now.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72829



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D72829: Implement -fsemantic-interposition

2020-01-29 Thread Alexander Richardson via Phabricator via cfe-commits

arichardson added a comment.

In D72829#1846157 , @serge-sans-paille 
wrote:

> @arichardson doc updated, any other advice?


No comments on the code just wondering if the documentation should be expanded.
I just came across this review by chance and it was not clear to me what 
"Enable semantic interposition" means.
I had to read the code and look the GCC documentation+llvm mailing list posts 
to understand it.

As this is user-facing documentation I feel like there should be a slightly 
longer explaning what the option does.

For comparison here is the GCC command line flag documentation

  -fsemantic-interposition
  Some object formats, like ELF, allow interposing of symbols by the dynamic 
linker. This means that for symbols exported from the DSO, the compiler cannot 
perform interprocedural propagation, inlining and other optimizations in 
anticipation that the function or variable in question may change. While this 
feature is useful, for example, to rewrite memory allocation functions by a 
debugging implementation, it is expensive in the terms of code quality. With 
-fno-semantic-interposition the compiler assumes that if interposition happens 
for functions the overwriting function will have precisely the same semantics 
(and side effects). Similarly if interposition happens for variables, the 
constructor of the variable will be the same. The flag has no effect for 
functions explicitly declared inline (where it is never allowed for 
interposition to change semantics) and for symbols explicitly declared weak.

Other than this, the change looks good to me.




Comment at: clang/docs/ClangCommandLineReference.rst:907
+
+.. option:: -fsemantic-interposition, -fno-semantic-interposition
+

arichardson wrote:
> This looks like it should be reordered?
Sorry I mean that it looks like all other entries are `.. option::` followed by 
the description but here it's the other way around. It seems like this change 
will make the documention of -fsanitize-* be `Enable semantic interposition`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72829



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D72829: Implement -fsemantic-interposition

2020-01-28 Thread pre-merge checks [bot] via Phabricator via cfe-commits

merge_guards_bot added a comment.

{icon check-circle color=green} Unit tests: pass. 62286 tests passed, 0 failed 
and 827 were skipped.

{icon check-circle color=green} clang-tidy: pass.

{icon check-circle color=green} clang-format: pass.

Build artifacts 
: 
diff.json 
,
 clang-tidy.txt 
,
 clang-format.patch 
,
 CMakeCache.txt 
,
 console-log.txt 
,
 test-results.xml 


//Pre-merge checks is in beta. Report issue 
.
 Please join beta  or enable 
it for your project 
.//


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72829



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D72829: Implement -fsemantic-interposition

2020-01-28 Thread serge via Phabricator via cfe-commits

serge-sans-paille updated this revision to Diff 241055.
serge-sans-paille added a comment.

@arichardson doc updated, any other advice?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72829

Files:
  clang/docs/ClangCommandLineReference.rst
  clang/include/clang/Basic/LangOptions.def
  clang/include/clang/Driver/Options.td
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/test/CodeGen/semantic-interposition.c
  clang/test/Driver/clang_f_opts.c
  llvm/include/llvm/IR/GlobalValue.h
  llvm/include/llvm/IR/Module.h
  llvm/lib/IR/Globals.cpp
  llvm/lib/IR/Module.cpp
  llvm/lib/IR/Verifier.cpp
  llvm/test/Transforms/Inline/inline-semantic-interposition.ll
  llvm/test/Verifier/module-flags-semantic-interposition.ll

Index: llvm/test/Verifier/module-flags-semantic-interposition.ll
===
--- /dev/null
+++ llvm/test/Verifier/module-flags-semantic-interposition.ll
@@ -0,0 +1,12 @@
+; RUN: not llvm-as < %s -o /dev/null 2>&1 | FileCheck %s
+
+target datalayout = "e-m:e-p270:32:32-p271:32:32-p272:64:64-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-unknown-linux-gnu"
+
+@foo = dso_local global i32 1, align 4
+
+!llvm.module.flags = !{!0}
+
+!0 = !{i32 1, !"SemanticInterposition", float 1.}
+
+; CHECK: SemanticInterposition metadata requires constant integer argument
Index: llvm/test/Transforms/Inline/inline-semantic-interposition.ll
===
--- /dev/null
+++ llvm/test/Transforms/Inline/inline-semantic-interposition.ll
@@ -0,0 +1,26 @@
+; Check that @callee1 gets inlined while @callee2 is not, because of
+; SemanticInterposition.
+
+; RUN: opt < %s -inline -S | FileCheck %s
+
+define internal i32 @callee1(i32 %A) {
+  ret i32 %A
+}
+
+define i32 @callee2(i32 %A) {
+  ret i32 %A
+}
+
+; CHECK-LABEL: @caller
+define i32 @caller(i32 %A) {
+; CHECK-NOT: call i32 @callee1(i32 %A)
+  %A1 = call i32 @callee1(i32 %A)
+; CHECK: %A2 = call i32 @callee2(i32 %A)
+  %A2 = call i32 @callee2(i32 %A)
+; CHECK: add i32 %A, %A2
+  %R = add i32 %A1, %A2
+  ret i32 %R
+}
+
+!llvm.module.flags = !{!0}
+!0 = !{i32 1, !"SemanticInterposition", i32 1}
Index: llvm/lib/IR/Verifier.cpp
===
--- llvm/lib/IR/Verifier.cpp
+++ llvm/lib/IR/Verifier.cpp
@@ -1476,6 +1476,13 @@
"'Linker Options' named metadata no longer supported");
   }
 
+  if (ID->getString() == "SemanticInterposition") {
+ConstantInt *Value =
+mdconst::dyn_extract_or_null(Op->getOperand(2));
+Assert(Value,
+   "SemanticInterposition metadata requires constant integer argument");
+  }
+
   if (ID->getString() == "CG Profile") {
 for (const MDOperand  : cast(Op->getOperand(2))->operands())
   visitModuleFlagCGProfileEntry(MDO);
Index: llvm/lib/IR/Module.cpp
===
--- llvm/lib/IR/Module.cpp
+++ llvm/lib/IR/Module.cpp
@@ -554,6 +554,20 @@
: getModuleFlag("ProfileSummary"));
 }
 
+bool Module::getSemanticInterposition() const {
+  Metadata *MF = getModuleFlag("SemanticInterposition");
+
+  auto *Val = cast_or_null(MF);
+  if (!Val)
+return false;
+
+  return cast(Val->getValue())->getZExtValue();
+}
+
+void Module::setSemanticInterposition(bool SI) {
+  addModuleFlag(ModFlagBehavior::Error, "SemanticInterposition", SI);
+}
+
 void Module::setOwnedMemoryBuffer(std::unique_ptr MB) {
   OwnedMemoryBuffer = std::move(MB);
 }
Index: llvm/lib/IR/Globals.cpp
===
--- llvm/lib/IR/Globals.cpp
+++ llvm/lib/IR/Globals.cpp
@@ -94,6 +94,13 @@
   llvm_unreachable("not a global");
 }
 
+bool GlobalValue::isInterposable() const {
+  if (isInterposableLinkage(getLinkage()))
+return true;
+  return getParent() && getParent()->getSemanticInterposition() &&
+ !isDSOLocal();
+}
+
 unsigned GlobalValue::getAlignment() const {
   if (auto *GA = dyn_cast(this)) {
 // In general we cannot compute this at the IR level, but we try.
Index: llvm/include/llvm/IR/Module.h
===
--- llvm/include/llvm/IR/Module.h
+++ llvm/include/llvm/IR/Module.h
@@ -848,6 +848,12 @@
   Metadata *getProfileSummary(bool IsCS);
   /// @}
 
+  /// Returns whether semantic interposition is to be respected.
+  bool getSemanticInterposition() const;
+
+  /// Set whether semantic interposition is to be respected.
+  void setSemanticInterposition(bool);
+
   /// Returns true if PLT should be avoided for RTLib calls.
   bool getRtLibUseGOT() const;
 
Index: llvm/include/llvm/IR/GlobalValue.h
===
--- llvm/include/llvm/IR/GlobalValue.h
+++

[PATCH] D72829: Implement -fsemantic-interposition

2020-01-28 Thread Alexander Richardson via Phabricator via cfe-commits

arichardson added inline comments.



Comment at: clang/docs/ClangCommandLineReference.rst:907
+
+.. option:: -fsemantic-interposition, -fno-semantic-interposition
+

This looks like it should be reordered?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72829



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D72829: Implement -fsemantic-interposition

2020-01-28 Thread pre-merge checks [bot] via Phabricator via cfe-commits

merge_guards_bot added a comment.

{icon check-circle color=green} Unit tests: pass. 62281 tests passed, 0 failed 
and 827 were skipped.

{icon check-circle color=green} clang-tidy: pass.

{icon check-circle color=green} clang-format: pass.

Build artifacts 
: 
diff.json 
,
 clang-tidy.txt 
,
 clang-format.patch 
,
 CMakeCache.txt 
,
 console-log.txt 
,
 test-results.xml 


//Pre-merge checks is in beta. Report issue 
.
 Please join beta  or enable 
it for your project 
.//


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72829



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D72829: Implement -fsemantic-interposition

2020-01-28 Thread Fangrui Song via Phabricator via cfe-commits

MaskRay accepted this revision.
MaskRay added a comment.

In D72829#1845396 , @serge-sans-paille 
wrote:

> > I suppose you mean whether "SemanticInterposition" should be invalidated 
> > when "PIC Level" does not exist or "PIE Level" exists. I am a bit inclined 
> > to make it more flexible/orthogonal in the backend, i.e. 
> > SemanticInterposition is in affect even if "PIE Level" is set.
>
> OK, looks good as is, then?


Yes!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72829



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D72829: Implement -fsemantic-interposition

2020-01-28 Thread serge via Phabricator via cfe-commits

serge-sans-paille marked an inline comment as done.
serge-sans-paille added a comment.

> I suppose you mean whether "SemanticInterposition" should be invalidated when 
> "PIC Level" does not exist or "PIE Level" exists. I am a bit inclined to make 
> it more flexible/orthogonal in the backend, i.e. SemanticInterposition is in 
> affect even if "PIE Level" is set.

OK, looks good as is, then?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72829



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D72829: Implement -fsemantic-interposition

2020-01-28 Thread serge via Phabricator via cfe-commits

serge-sans-paille updated this revision to Diff 240966.
serge-sans-paille marked an inline comment as done.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72829

Files:
  clang/docs/ClangCommandLineReference.rst
  clang/include/clang/Basic/LangOptions.def
  clang/include/clang/Driver/Options.td
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/test/CodeGen/semantic-interposition.c
  clang/test/Driver/clang_f_opts.c
  llvm/include/llvm/IR/GlobalValue.h
  llvm/include/llvm/IR/Module.h
  llvm/lib/IR/Globals.cpp
  llvm/lib/IR/Module.cpp
  llvm/lib/IR/Verifier.cpp
  llvm/test/Transforms/Inline/inline-semantic-interposition.ll
  llvm/test/Verifier/module-flags-semantic-interposition.ll

Index: llvm/test/Verifier/module-flags-semantic-interposition.ll
===
--- /dev/null
+++ llvm/test/Verifier/module-flags-semantic-interposition.ll
@@ -0,0 +1,12 @@
+; RUN: not llvm-as < %s -o /dev/null 2>&1 | FileCheck %s
+
+target datalayout = "e-m:e-p270:32:32-p271:32:32-p272:64:64-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-unknown-linux-gnu"
+
+@foo = dso_local global i32 1, align 4
+
+!llvm.module.flags = !{!0}
+
+!0 = !{i32 1, !"SemanticInterposition", float 1.}
+
+; CHECK: SemanticInterposition metadata requires constant integer argument
Index: llvm/test/Transforms/Inline/inline-semantic-interposition.ll
===
--- /dev/null
+++ llvm/test/Transforms/Inline/inline-semantic-interposition.ll
@@ -0,0 +1,26 @@
+; Check that @callee1 gets inlined while @callee2 is not, because of
+; SemanticInterposition.
+
+; RUN: opt < %s -inline -S | FileCheck %s
+
+define internal i32 @callee1(i32 %A) {
+  ret i32 %A
+}
+
+define i32 @callee2(i32 %A) {
+  ret i32 %A
+}
+
+; CHECK-LABEL: @caller
+define i32 @caller(i32 %A) {
+; CHECK-NOT: call i32 @callee1(i32 %A)
+  %A1 = call i32 @callee1(i32 %A)
+; CHECK: %A2 = call i32 @callee2(i32 %A)
+  %A2 = call i32 @callee2(i32 %A)
+; CHECK: add i32 %A, %A2
+  %R = add i32 %A1, %A2
+  ret i32 %R
+}
+
+!llvm.module.flags = !{!0}
+!0 = !{i32 1, !"SemanticInterposition", i32 1}
Index: llvm/lib/IR/Verifier.cpp
===
--- llvm/lib/IR/Verifier.cpp
+++ llvm/lib/IR/Verifier.cpp
@@ -1476,6 +1476,13 @@
"'Linker Options' named metadata no longer supported");
   }
 
+  if (ID->getString() == "SemanticInterposition") {
+ConstantInt *Value =
+mdconst::dyn_extract_or_null(Op->getOperand(2));
+Assert(Value,
+   "SemanticInterposition metadata requires constant integer argument");
+  }
+
   if (ID->getString() == "CG Profile") {
 for (const MDOperand  : cast(Op->getOperand(2))->operands())
   visitModuleFlagCGProfileEntry(MDO);
Index: llvm/lib/IR/Module.cpp
===
--- llvm/lib/IR/Module.cpp
+++ llvm/lib/IR/Module.cpp
@@ -554,6 +554,20 @@
: getModuleFlag("ProfileSummary"));
 }
 
+bool Module::getSemanticInterposition() const {
+  Metadata *MF = getModuleFlag("SemanticInterposition");
+
+  auto *Val = cast_or_null(MF);
+  if (!Val)
+return false;
+
+  return cast(Val->getValue())->getZExtValue();
+}
+
+void Module::setSemanticInterposition(bool SI) {
+  addModuleFlag(ModFlagBehavior::Error, "SemanticInterposition", SI);
+}
+
 void Module::setOwnedMemoryBuffer(std::unique_ptr MB) {
   OwnedMemoryBuffer = std::move(MB);
 }
Index: llvm/lib/IR/Globals.cpp
===
--- llvm/lib/IR/Globals.cpp
+++ llvm/lib/IR/Globals.cpp
@@ -94,6 +94,13 @@
   llvm_unreachable("not a global");
 }
 
+bool GlobalValue::isInterposable() const {
+  if (isInterposableLinkage(getLinkage()))
+return true;
+  return getParent() && getParent()->getSemanticInterposition() &&
+ !isDSOLocal();
+}
+
 unsigned GlobalValue::getAlignment() const {
   if (auto *GA = dyn_cast(this)) {
 // In general we cannot compute this at the IR level, but we try.
Index: llvm/include/llvm/IR/Module.h
===
--- llvm/include/llvm/IR/Module.h
+++ llvm/include/llvm/IR/Module.h
@@ -848,6 +848,12 @@
   Metadata *getProfileSummary(bool IsCS);
   /// @}
 
+  /// Returns whether semantic interposition is to be respected.
+  bool getSemanticInterposition() const;
+
+  /// Set whether semantic interposition is to be respected.
+  void setSemanticInterposition(bool);
+
   /// Returns true if PLT should be avoided for RTLib calls.
   bool getRtLibUseGOT() const;
 
Index: llvm/include/llvm/IR/GlobalValue.h
===
--- llvm/include/llvm/IR/GlobalValue.h
+++ llvm/include/llvm/IR/GlobalValue.h
@@ -426,7 +426,7 @@

[PATCH] D72829: Implement -fsemantic-interposition

2020-01-28 Thread Fangrui Song via Phabricator via cfe-commits

MaskRay added inline comments.



Comment at: clang/lib/Frontend/CompilerInvocation.cpp:3011
+  if (Args.hasArg(OPT_fsemantic_interposition) && Opts.PICLevel && !Opts.PIE)
+Opts.SemanticInterposition = 1;
+

`Opts.SemanticInterposition` is initialized to 0 and is not assigned elsewhere.

`Opts.SemanticInterposition = ...` should be possible.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72829



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D72829: Implement -fsemantic-interposition

2020-01-28 Thread Fangrui Song via Phabricator via cfe-commits

MaskRay added a comment.

In D72829#1844050 , @serge-sans-paille 
wrote:

> @MaskRay should we add a verifier step to check that 
> pie/pic/semanticinterposition module flags are consistent, or leave that to 
> clang ?

I suppose you mean whether "SemanticInterposition" should be invalidated when 
`"PIC Level"` does not exist or `"PIE Level"` exists. I am a bit inclined to 
make it more flexible/orthogonal in the backend, i.e. SemanticInterposition is 
in affect even if "PIE Level" is set.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72829

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D72829: Implement -fsemantic-interposition

2020-01-28 Thread pre-merge checks [bot] via Phabricator via cfe-commits

merge_guards_bot added a comment.

{icon check-circle color=green} Unit tests: pass. 62258 tests passed, 0 failed 
and 827 were skipped.

{icon check-circle color=green} clang-tidy: pass.

{icon times-circle color=red} clang-format: fail. Please format your changes 
with clang-format by running `git-clang-format HEAD^` or applying this patch 
.

Build artifacts 
: 
diff.json 
,
 clang-tidy.txt 
,
 clang-format.patch 
,
 CMakeCache.txt 
,
 console-log.txt 
,
 test-results.xml 


//Pre-merge checks is in beta. Report issue 
.
 Please join beta  or enable 
it for your project 
.//


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72829



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D72829: Implement -fsemantic-interposition

2020-01-28 Thread serge via Phabricator via cfe-commits

serge-sans-paille updated this revision to Diff 240813.
serge-sans-paille added a comment.

@MaskRay should we add a verifier step to check that 
pie/pic/semanticinterposition module flags are consistent, or leave that to 
clang ?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72829

Files:
  clang/docs/ClangCommandLineReference.rst
  clang/include/clang/Basic/LangOptions.def
  clang/include/clang/Driver/Options.td
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/test/CodeGen/semantic-interposition.c
  clang/test/Driver/clang_f_opts.c
  llvm/include/llvm/IR/GlobalValue.h
  llvm/include/llvm/IR/Module.h
  llvm/lib/IR/Globals.cpp
  llvm/lib/IR/Module.cpp
  llvm/lib/IR/Verifier.cpp
  llvm/test/Transforms/Inline/inline-semantic-interposition.ll
  llvm/test/Verifier/module-flags-semantic-interposition.ll

Index: llvm/test/Verifier/module-flags-semantic-interposition.ll
===
--- /dev/null
+++ llvm/test/Verifier/module-flags-semantic-interposition.ll
@@ -0,0 +1,12 @@
+; RUN: not llvm-as < %s -o /dev/null 2>&1 | FileCheck %s
+
+target datalayout = "e-m:e-p270:32:32-p271:32:32-p272:64:64-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-unknown-linux-gnu"
+
+@foo = dso_local global i32 1, align 4
+
+!llvm.module.flags = !{!0}
+
+!0 = !{i32 1, !"SemanticInterposition", float 1.}
+
+; CHECK: SemanticInterposition metadata requires constant integer argument
Index: llvm/test/Transforms/Inline/inline-semantic-interposition.ll
===
--- /dev/null
+++ llvm/test/Transforms/Inline/inline-semantic-interposition.ll
@@ -0,0 +1,26 @@
+; Check that @callee1 gets inlined while @callee2 is not, because of
+; SemanticInterposition.
+
+; RUN: opt < %s -inline -S | FileCheck %s
+
+define internal i32 @callee1(i32 %A) {
+  ret i32 %A
+}
+
+define i32 @callee2(i32 %A) {
+  ret i32 %A
+}
+
+; CHECK-LABEL: @caller
+define i32 @caller(i32 %A) {
+; CHECK-NOT: call i32 @callee1(i32 %A)
+  %A1 = call i32 @callee1(i32 %A)
+; CHECK: %A2 = call i32 @callee2(i32 %A)
+  %A2 = call i32 @callee2(i32 %A)
+; CHECK: add i32 %A, %A2
+  %R = add i32 %A1, %A2
+  ret i32 %R
+}
+
+!llvm.module.flags = !{!0}
+!0 = !{i32 1, !"SemanticInterposition", i32 1}
Index: llvm/lib/IR/Verifier.cpp
===
--- llvm/lib/IR/Verifier.cpp
+++ llvm/lib/IR/Verifier.cpp
@@ -1476,6 +1476,13 @@
"'Linker Options' named metadata no longer supported");
   }
 
+  if (ID->getString() == "SemanticInterposition") {
+ConstantInt *Value =
+mdconst::dyn_extract_or_null(Op->getOperand(2));
+Assert(Value,
+   "SemanticInterposition metadata requires constant integer argument");
+  }
+
   if (ID->getString() == "CG Profile") {
 for (const MDOperand  : cast(Op->getOperand(2))->operands())
   visitModuleFlagCGProfileEntry(MDO);
Index: llvm/lib/IR/Module.cpp
===
--- llvm/lib/IR/Module.cpp
+++ llvm/lib/IR/Module.cpp
@@ -554,6 +554,20 @@
: getModuleFlag("ProfileSummary"));
 }
 
+bool Module::getSemanticInterposition() const {
+  Metadata *MF = getModuleFlag("SemanticInterposition");
+
+  auto *Val = cast_or_null(MF);
+  if (!Val)
+return false;
+
+  return cast(Val->getValue())->getZExtValue();
+}
+
+void Module::setSemanticInterposition(bool SI) {
+  addModuleFlag(ModFlagBehavior::Error, "SemanticInterposition", SI);
+}
+
 void Module::setOwnedMemoryBuffer(std::unique_ptr MB) {
   OwnedMemoryBuffer = std::move(MB);
 }
Index: llvm/lib/IR/Globals.cpp
===
--- llvm/lib/IR/Globals.cpp
+++ llvm/lib/IR/Globals.cpp
@@ -94,6 +94,13 @@
   llvm_unreachable("not a global");
 }
 
+bool GlobalValue::isInterposable() const {
+  if (isInterposableLinkage(getLinkage()))
+return true;
+  return getParent() && getParent()->getSemanticInterposition() &&
+ !isDSOLocal();
+}
+
 unsigned GlobalValue::getAlignment() const {
   if (auto *GA = dyn_cast(this)) {
 // In general we cannot compute this at the IR level, but we try.
Index: llvm/include/llvm/IR/Module.h
===
--- llvm/include/llvm/IR/Module.h
+++ llvm/include/llvm/IR/Module.h
@@ -848,6 +848,12 @@
   Metadata *getProfileSummary(bool IsCS);
   /// @}
 
+  /// Returns whether semantic interposition is to be respected.
+  bool getSemanticInterposition() const;
+
+  /// Set whether semantic interposition is to be respected.
+  void setSemanticInterposition(bool);
+
   /// Returns true if PLT should be avoided for RTLib calls.
   bool getRtLibUseGOT() const;
 
Index: llvm/include/llvm/IR/GlobalValue.h

[PATCH] D72829: Implement -fsemantic-interposition

2020-01-28 Thread serge via Phabricator via cfe-commits

serge-sans-paille marked 3 inline comments as done.
serge-sans-paille added inline comments.



Comment at: clang/lib/CodeGen/CodeGenModule.cpp:488
+  if (Context.getLangOpts().SemanticInterposition &&
+  (Context.getLangOpts().PICLevel && !Context.getLangOpts().PIE))
+// Require various optimization to respect semantic interposition.

MaskRay wrote:
> MaskRay wrote:
> > The parentheses can be deleted.
> An alternative is to move the PIC/PIE logic to 
> lib/Frontend/CompilerInvocation.cpp
> 
> Then we should be able to use `if 
> (Context.getLangOpts().SemanticInterposition)` here
:+1: like that one


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72829



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D72829: Implement -fsemantic-interposition

2020-01-27 Thread Fangrui Song via Phabricator via cfe-commits

MaskRay added inline comments.



Comment at: clang/lib/CodeGen/CodeGenModule.cpp:488
+  if (Context.getLangOpts().SemanticInterposition &&
+  (Context.getLangOpts().PICLevel && !Context.getLangOpts().PIE))
+// Require various optimization to respect semantic interposition.

MaskRay wrote:
> The parentheses can be deleted.
An alternative is to move the PIC/PIE logic to 
lib/Frontend/CompilerInvocation.cpp

Then we should be able to use `if 
(Context.getLangOpts().SemanticInterposition)` here



Comment at: llvm/test/Transforms/Inline/inline-semantic-interposition.ll:1
+; RUN: opt < %s -inline  -S | FileCheck %s
+

serge-sans-paille wrote:
> MaskRay wrote:
> > excess space
> > 
> > Add a comment what happened here.
> I'm not sure about the excess space comment, but comment added.
Nit: there are two spaces between -inline and -S.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72829



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D72829: Implement -fsemantic-interposition

2020-01-27 Thread pre-merge checks [bot] via Phabricator via cfe-commits

merge_guards_bot added a comment.

{icon check-circle color=green} Unit tests: pass. 62209 tests passed, 0 failed 
and 815 were skipped.

{icon check-circle color=green} clang-tidy: pass.

{icon check-circle color=green} clang-format: pass.

Build artifacts 
: 
diff.json 
,
 clang-tidy.txt 
,
 clang-format.patch 
,
 CMakeCache.txt 
,
 console-log.txt 
,
 test-results.xml 


//Pre-merge checks is in beta. Report issue 
.
 Please join beta  or enable 
it for your project 
.//


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72829



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D72829: Implement -fsemantic-interposition

2020-01-27 Thread serge via Phabricator via cfe-commits

serge-sans-paille marked 2 inline comments as done.
serge-sans-paille added inline comments.



Comment at: clang/lib/Frontend/CompilerInvocation.cpp:2685
+  if (Args.hasArg(OPT_fsemantic_interposition))
+Opts.SemanticInterposition = 1;
+

MaskRay wrote:
> ` Opts.SemanticInterposition = Args.hasArg(OPT_fsemantic_interposition);`
That was on purpose, so that we're backward compatible with existing situation. 
Without the flag, the genrated .ll doesn't differ. That's less explicit though. 
Does it make sense to you?



Comment at: llvm/test/Transforms/Inline/inline-semantic-interposition.ll:1
+; RUN: opt < %s -inline  -S | FileCheck %s
+

MaskRay wrote:
> excess space
> 
> Add a comment what happened here.
I'm not sure about the excess space comment, but comment added.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72829



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D72829: Implement -fsemantic-interposition

2020-01-27 Thread serge via Phabricator via cfe-commits

serge-sans-paille updated this revision to Diff 240486.
serge-sans-paille added a comment.

Improve & comment examples


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72829

Files:
  clang/docs/ClangCommandLineReference.rst
  clang/include/clang/Basic/LangOptions.def
  clang/include/clang/Driver/Options.td
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/test/CodeGen/semantic-interposition.c
  clang/test/Driver/clang_f_opts.c
  llvm/include/llvm/IR/GlobalValue.h
  llvm/include/llvm/IR/Module.h
  llvm/lib/IR/Globals.cpp
  llvm/lib/IR/Module.cpp
  llvm/lib/IR/Verifier.cpp
  llvm/test/Transforms/Inline/inline-semantic-interposition.ll
  llvm/test/Verifier/module-flags-semantic-interposition.ll

Index: llvm/test/Verifier/module-flags-semantic-interposition.ll
===
--- /dev/null
+++ llvm/test/Verifier/module-flags-semantic-interposition.ll
@@ -0,0 +1,12 @@
+; RUN: not llvm-as < %s -o /dev/null 2>&1 | FileCheck %s
+
+target datalayout = "e-m:e-p270:32:32-p271:32:32-p272:64:64-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-unknown-linux-gnu"
+
+@foo = dso_local global i32 1, align 4
+
+!llvm.module.flags = !{!0}
+
+!0 = !{i32 1, !"SemanticInterposition", float 1.}
+
+; CHECK: SemanticInterposition metadata requires constant integer argument
Index: llvm/test/Transforms/Inline/inline-semantic-interposition.ll
===
--- /dev/null
+++ llvm/test/Transforms/Inline/inline-semantic-interposition.ll
@@ -0,0 +1,26 @@
+; Check that @callee1 gets inlined while @callee2 is not, because of
+; SemanticInterposition.
+
+; RUN: opt < %s -inline  -S | FileCheck %s
+
+define internal i32 @callee1(i32 %A) {
+  ret i32 %A
+}
+
+define i32 @callee2(i32 %A) {
+  ret i32 %A
+}
+
+; CHECK-LABEL: @caller
+define i32 @caller(i32 %A) {
+; CHECK-NOT: call i32 @callee1(i32 %A)
+  %A1 = call i32 @callee1(i32 %A)
+; CHECK: %A2 = call i32 @callee2(i32 %A)
+  %A2 = call i32 @callee2(i32 %A)
+; CHECK: add i32 %A, %A2
+  %R = add i32 %A1, %A2
+  ret i32 %R
+}
+
+!llvm.module.flags = !{!0}
+!0 = !{i32 1, !"SemanticInterposition", i32 1}
Index: llvm/lib/IR/Verifier.cpp
===
--- llvm/lib/IR/Verifier.cpp
+++ llvm/lib/IR/Verifier.cpp
@@ -1476,6 +1476,13 @@
"'Linker Options' named metadata no longer supported");
   }
 
+  if (ID->getString() == "SemanticInterposition") {
+ConstantInt *Value =
+mdconst::dyn_extract_or_null(Op->getOperand(2));
+Assert(Value,
+   "SemanticInterposition metadata requires constant integer argument");
+  }
+
   if (ID->getString() == "CG Profile") {
 for (const MDOperand  : cast(Op->getOperand(2))->operands())
   visitModuleFlagCGProfileEntry(MDO);
Index: llvm/lib/IR/Module.cpp
===
--- llvm/lib/IR/Module.cpp
+++ llvm/lib/IR/Module.cpp
@@ -554,6 +554,20 @@
: getModuleFlag("ProfileSummary"));
 }
 
+bool Module::getSemanticInterposition() const {
+  Metadata *MF = getModuleFlag("SemanticInterposition");
+
+  auto *Val = cast_or_null(MF);
+  if (!Val)
+return false;
+
+  return cast(Val->getValue())->getZExtValue();
+}
+
+void Module::setSemanticInterposition(bool SI) {
+  addModuleFlag(ModFlagBehavior::Error, "SemanticInterposition", SI);
+}
+
 void Module::setOwnedMemoryBuffer(std::unique_ptr MB) {
   OwnedMemoryBuffer = std::move(MB);
 }
Index: llvm/lib/IR/Globals.cpp
===
--- llvm/lib/IR/Globals.cpp
+++ llvm/lib/IR/Globals.cpp
@@ -94,6 +94,13 @@
   llvm_unreachable("not a global");
 }
 
+bool GlobalValue::isInterposable() const {
+  if (isInterposableLinkage(getLinkage()))
+return true;
+  return getParent() && getParent()->getSemanticInterposition() &&
+ !isDSOLocal();
+}
+
 unsigned GlobalValue::getAlignment() const {
   if (auto *GA = dyn_cast(this)) {
 // In general we cannot compute this at the IR level, but we try.
Index: llvm/include/llvm/IR/Module.h
===
--- llvm/include/llvm/IR/Module.h
+++ llvm/include/llvm/IR/Module.h
@@ -848,6 +848,12 @@
   Metadata *getProfileSummary(bool IsCS);
   /// @}
 
+  /// Returns whether semantic interposition is to be respected.
+  bool getSemanticInterposition() const;
+
+  /// Set whether semantic interposition is to be respected.
+  void setSemanticInterposition(bool);
+
   /// Returns true if PLT should be avoided for RTLib calls.
   bool getRtLibUseGOT() const;
 
Index: llvm/include/llvm/IR/GlobalValue.h
===
--- llvm/include/llvm/IR/GlobalValue.h
+++ llvm/include/llvm/IR/GlobalValue.h
@@ -426,7

[PATCH] D72829: Implement -fsemantic-interposition

2020-01-24 Thread Fangrui Song via Phabricator via cfe-commits

MaskRay added inline comments.



Comment at: clang/lib/Frontend/CompilerInvocation.cpp:2685
+  if (Args.hasArg(OPT_fsemantic_interposition))
+Opts.SemanticInterposition = 1;
+

` Opts.SemanticInterposition = Args.hasArg(OPT_fsemantic_interposition);`



Comment at: clang/test/CodeGen/semantic-interposition.c:1
+// RUN: %clang_cc1 -emit-llvm -fsemantic-interposition -pic-level 0 %s -o - | 
FileCheck %s -check-prefix=CHECK-NOPIC-NOPIE
+// RUN: %clang_cc1 -emit-llvm -fsemantic-interposition -pic-level 1 %s -o - | 
FileCheck %s -check-prefix=CHECK-PIC-NOPIE

Add a file-level comment what configurations produce SemanticInterposition



Comment at: clang/test/CodeGen/semantic-interposition.c:8
+
+// CHECK-NOPIC-NOPIE-NOT: "SemanticInterposition"
+// CHECK-PIC-NOPIE: !{{[0-9]+}} = !{i32 1, !"SemanticInterposition", i32 1}

`CHECK-NOPIC-NOPIE-NOT` and `CHECK-PIC-PIE-NOT` can be combined (e.g. 
`INTERPOSE:` vs `NO-INTERPOSE-NOT:` )



Comment at: llvm/test/Transforms/Inline/inline-semantic-interposition.ll:1
+; RUN: opt < %s -inline  -S | FileCheck %s
+

excess space

Add a comment what happened here.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72829



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D72829: Implement -fsemantic-interposition

2020-01-24 Thread pre-merge checks [bot] via Phabricator via cfe-commits

merge_guards_bot added a comment.

{icon times-circle color=red} Unit tests: fail. 62174 tests passed, 5 failed 
and 813 were skipped.

  failed: libc++.std/language_support/cmp/cmp_partialord/partialord.pass.cpp
  failed: libc++.std/language_support/cmp/cmp_strongeq/cmp.strongeq.pass.cpp
  failed: libc++.std/language_support/cmp/cmp_strongord/strongord.pass.cpp
  failed: libc++.std/language_support/cmp/cmp_weakeq/cmp.weakeq.pass.cpp
  failed: libc++.std/language_support/cmp/cmp_weakord/weakord.pass.cpp

{icon check-circle color=green} clang-tidy: pass.

{icon check-circle color=green} clang-format: pass.

Build artifacts 
: 
diff.json 
,
 clang-tidy.txt 
,
 clang-format.patch 
,
 CMakeCache.txt 
,
 console-log.txt 
,
 test-results.xml 


//Pre-merge checks is in beta. Report issue 
.
 Please join beta  or enable 
it for your project 
.//


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72829



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D72829: Implement -fsemantic-interposition

2020-01-24 Thread serge via Phabricator via cfe-commits

serge-sans-paille updated this revision to Diff 240187.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72829

Files:
  clang/docs/ClangCommandLineReference.rst
  clang/include/clang/Basic/LangOptions.def
  clang/include/clang/Driver/Options.td
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/test/CodeGen/semantic-interposition.c
  clang/test/Driver/clang_f_opts.c
  llvm/include/llvm/IR/GlobalValue.h
  llvm/include/llvm/IR/Module.h
  llvm/lib/IR/Globals.cpp
  llvm/lib/IR/Module.cpp
  llvm/lib/IR/Verifier.cpp
  llvm/test/Transforms/Inline/inline-semantic-interposition.ll
  llvm/test/Verifier/module-flags-semantic-interposition.ll

Index: llvm/test/Verifier/module-flags-semantic-interposition.ll
===
--- /dev/null
+++ llvm/test/Verifier/module-flags-semantic-interposition.ll
@@ -0,0 +1,12 @@
+; RUN: not llvm-as < %s -o /dev/null 2>&1 | FileCheck %s
+
+target datalayout = "e-m:e-p270:32:32-p271:32:32-p272:64:64-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-unknown-linux-gnu"
+
+@foo = dso_local global i32 1, align 4
+
+!llvm.module.flags = !{!0}
+
+!0 = !{i32 1, !"SemanticInterposition", float 1.}
+
+; CHECK: SemanticInterposition metadata requires constant integer argument
Index: llvm/test/Transforms/Inline/inline-semantic-interposition.ll
===
--- /dev/null
+++ llvm/test/Transforms/Inline/inline-semantic-interposition.ll
@@ -0,0 +1,24 @@
+; RUN: opt < %s -inline  -S | FileCheck %s
+
+define internal i32 @callee1(i32 %A) {
+  ret i32 %A
+}
+define i32 @callee2(i32 %A) {
+  ret i32 %A
+}
+
+; CHECK-LABEL: @caller
+define i32 @caller(i32 %A) {
+; CHECK-NOT: call i32 @callee1(i32 %A)
+  %A1 = call i32 @callee1(i32 %A)
+; CHECK: %A2 = call i32 @callee2(i32 %A)
+  %A2 = call i32 @callee2(i32 %A)
+; CHECK: add i32 %A, %A2
+  %R = add i32 %A1, %A2
+  ret i32 %R
+}
+
+
+!llvm.module.flags = !{!0}
+
+!0 = !{i32 1, !"SemanticInterposition", i32 1}
Index: llvm/lib/IR/Verifier.cpp
===
--- llvm/lib/IR/Verifier.cpp
+++ llvm/lib/IR/Verifier.cpp
@@ -1476,6 +1476,13 @@
"'Linker Options' named metadata no longer supported");
   }
 
+  if (ID->getString() == "SemanticInterposition") {
+ConstantInt *Value =
+mdconst::dyn_extract_or_null(Op->getOperand(2));
+Assert(Value,
+   "SemanticInterposition metadata requires constant integer argument");
+  }
+
   if (ID->getString() == "CG Profile") {
 for (const MDOperand  : cast(Op->getOperand(2))->operands())
   visitModuleFlagCGProfileEntry(MDO);
Index: llvm/lib/IR/Module.cpp
===
--- llvm/lib/IR/Module.cpp
+++ llvm/lib/IR/Module.cpp
@@ -554,6 +554,20 @@
: getModuleFlag("ProfileSummary"));
 }
 
+bool Module::getSemanticInterposition() const {
+  Metadata *MF = getModuleFlag("SemanticInterposition");
+
+  auto *Val = cast_or_null(MF);
+  if (!Val)
+return false;
+
+  return cast(Val->getValue())->getZExtValue();
+}
+
+void Module::setSemanticInterposition(bool SI) {
+  addModuleFlag(ModFlagBehavior::Error, "SemanticInterposition", SI);
+}
+
 void Module::setOwnedMemoryBuffer(std::unique_ptr MB) {
   OwnedMemoryBuffer = std::move(MB);
 }
Index: llvm/lib/IR/Globals.cpp
===
--- llvm/lib/IR/Globals.cpp
+++ llvm/lib/IR/Globals.cpp
@@ -94,6 +94,13 @@
   llvm_unreachable("not a global");
 }
 
+bool GlobalValue::isInterposable() const {
+  if (isInterposableLinkage(getLinkage()))
+return true;
+  return getParent() && getParent()->getSemanticInterposition() &&
+ !isDSOLocal();
+}
+
 unsigned GlobalValue::getAlignment() const {
   if (auto *GA = dyn_cast(this)) {
 // In general we cannot compute this at the IR level, but we try.
Index: llvm/include/llvm/IR/Module.h
===
--- llvm/include/llvm/IR/Module.h
+++ llvm/include/llvm/IR/Module.h
@@ -848,6 +848,12 @@
   Metadata *getProfileSummary(bool IsCS);
   /// @}
 
+  /// Returns whether semantic interposition is to be respected.
+  bool getSemanticInterposition() const;
+
+  /// Set whether semantic interposition is to be respected.
+  void setSemanticInterposition(bool);
+
   /// Returns true if PLT should be avoided for RTLib calls.
   bool getRtLibUseGOT() const;
 
Index: llvm/include/llvm/IR/GlobalValue.h
===
--- llvm/include/llvm/IR/GlobalValue.h
+++ llvm/include/llvm/IR/GlobalValue.h
@@ -426,7 +426,7 @@
   /// *arbitrary* definition at link time.  We cannot do any IPO or inlinining
   /// across interposable call edges, since the callee can be replaced

[PATCH] D72829: Implement -fsemantic-interposition

2020-01-24 Thread pre-merge checks [bot] via Phabricator via cfe-commits

merge_guards_bot added a comment.

{icon times-circle color=red} Unit tests: fail. 62155 tests passed, 5 failed 
and 811 were skipped.

  failed: libc++.std/language_support/cmp/cmp_partialord/partialord.pass.cpp
  failed: libc++.std/language_support/cmp/cmp_strongeq/cmp.strongeq.pass.cpp
  failed: libc++.std/language_support/cmp/cmp_strongord/strongord.pass.cpp
  failed: libc++.std/language_support/cmp/cmp_weakeq/cmp.weakeq.pass.cpp
  failed: libc++.std/language_support/cmp/cmp_weakord/weakord.pass.cpp

{icon check-circle color=green} clang-tidy: pass.

{icon check-circle color=green} clang-format: pass.

Build artifacts 
: 
diff.json 
,
 clang-tidy.txt 
,
 clang-format.patch 
,
 CMakeCache.txt 
,
 console-log.txt 
,
 test-results.xml 


//Pre-merge checks is in beta. Report issue 
.
 Please join beta  or enable 
it for your project 
.//


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72829



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D72829: Implement -fsemantic-interposition

2020-01-24 Thread serge via Phabricator via cfe-commits

serge-sans-paille updated this revision to Diff 240110.
serge-sans-paille added a comment.

Formatting nits + rebase


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72829

Files:
  clang/docs/ClangCommandLineReference.rst
  clang/include/clang/Basic/LangOptions.def
  clang/include/clang/Driver/Options.td
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/test/CodeGen/semantic-interposition.c
  clang/test/Driver/clang_f_opts.c
  llvm/include/llvm/IR/GlobalValue.h
  llvm/include/llvm/IR/Module.h
  llvm/lib/IR/Globals.cpp
  llvm/lib/IR/Module.cpp
  llvm/lib/IR/Verifier.cpp
  llvm/test/Transforms/Inline/inline-semantic-interposition.ll
  llvm/test/Verifier/module-flags-semantic-interposition.ll

Index: llvm/test/Verifier/module-flags-semantic-interposition.ll
===
--- /dev/null
+++ llvm/test/Verifier/module-flags-semantic-interposition.ll
@@ -0,0 +1,12 @@
+; RUN: not llvm-as < %s -o /dev/null 2>&1 | FileCheck %s
+
+target datalayout = "e-m:e-p270:32:32-p271:32:32-p272:64:64-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-unknown-linux-gnu"
+
+@foo = dso_local global i32 1, align 4
+
+!llvm.module.flags = !{!0}
+
+!0 = !{i32 1, !"SemanticInterposition", float 1.}
+
+; CHECK: SemanticInterposition metadata requires constant integer argument
Index: llvm/test/Transforms/Inline/inline-semantic-interposition.ll
===
--- /dev/null
+++ llvm/test/Transforms/Inline/inline-semantic-interposition.ll
@@ -0,0 +1,24 @@
+; RUN: opt < %s -inline  -S | FileCheck %s
+
+define internal i32 @callee1(i32 %A) {
+  ret i32 %A
+}
+define i32 @callee2(i32 %A) {
+  ret i32 %A
+}
+
+; CHECK-LABEL: @caller
+define i32 @caller(i32 %A) {
+; CHECK-NOT: call i32 @callee1(i32 %A)
+  %A1 = call i32 @callee1(i32 %A)
+; CHECK: %A2 = call i32 @callee2(i32 %A)
+  %A2 = call i32 @callee2(i32 %A)
+; CHECK: add i32 %A, %A2
+  %R = add i32 %A1, %A2
+  ret i32 %R
+}
+
+
+!llvm.module.flags = !{!0}
+
+!0 = !{i32 1, !"SemanticInterposition", i32 1}
Index: llvm/lib/IR/Verifier.cpp
===
--- llvm/lib/IR/Verifier.cpp
+++ llvm/lib/IR/Verifier.cpp
@@ -1476,6 +1476,13 @@
"'Linker Options' named metadata no longer supported");
   }
 
+  if (ID->getString() == "SemanticInterposition") {
+ConstantInt *Value =
+mdconst::dyn_extract_or_null(Op->getOperand(2));
+Assert(Value,
+   "SemanticInterposition metadata requires constant integer argument");
+  }
+
   if (ID->getString() == "CG Profile") {
 for (const MDOperand  : cast(Op->getOperand(2))->operands())
   visitModuleFlagCGProfileEntry(MDO);
Index: llvm/lib/IR/Module.cpp
===
--- llvm/lib/IR/Module.cpp
+++ llvm/lib/IR/Module.cpp
@@ -554,6 +554,20 @@
: getModuleFlag("ProfileSummary"));
 }
 
+bool Module::getSemanticInterposition() const {
+  Metadata *MF = getModuleFlag("SemanticInterposition");
+
+  auto *Val = cast_or_null(MF);
+  if (!Val)
+return false;
+
+  return cast(Val->getValue())->getZExtValue();
+}
+
+void Module::setSemanticInterposition(bool SI) {
+  addModuleFlag(ModFlagBehavior::Error, "SemanticInterposition", SI);
+}
+
 void Module::setOwnedMemoryBuffer(std::unique_ptr MB) {
   OwnedMemoryBuffer = std::move(MB);
 }
Index: llvm/lib/IR/Globals.cpp
===
--- llvm/lib/IR/Globals.cpp
+++ llvm/lib/IR/Globals.cpp
@@ -94,6 +94,13 @@
   llvm_unreachable("not a global");
 }
 
+bool GlobalValue::isInterposable() const {
+  if (isInterposableLinkage(getLinkage()))
+return true;
+  return getParent() && getParent()->getSemanticInterposition() &&
+ !isDSOLocal();
+}
+
 unsigned GlobalValue::getAlignment() const {
   if (auto *GA = dyn_cast(this)) {
 // In general we cannot compute this at the IR level, but we try.
Index: llvm/include/llvm/IR/Module.h
===
--- llvm/include/llvm/IR/Module.h
+++ llvm/include/llvm/IR/Module.h
@@ -848,6 +848,12 @@
   Metadata *getProfileSummary(bool IsCS);
   /// @}
 
+  /// Returns whether semantic interposition is to be respected.
+  bool getSemanticInterposition() const;
+
+  /// Set whether semantic interposition is to be respected.
+  void setSemanticInterposition(bool);
+
   /// Returns true if PLT should be avoided for RTLib calls.
   bool getRtLibUseGOT() const;
 
Index: llvm/include/llvm/IR/GlobalValue.h
===
--- llvm/include/llvm/IR/GlobalValue.h
+++ llvm/include/llvm/IR/GlobalValue.h
@@ -426,7 +426,7 @@
   /// *arbitrary* definition at link time.  We cannot do any IPO or inlinining
   ///

[PATCH] D72829: Implement -fsemantic-interposition

2020-01-23 Thread pre-merge checks [bot] via Phabricator via cfe-commits

merge_guards_bot added a comment.

{icon times-circle color=red} Unit tests: fail. 62147 tests passed, 5 failed 
and 811 were skipped.

  failed: libc++.std/language_support/cmp/cmp_partialord/partialord.pass.cpp
  failed: libc++.std/language_support/cmp/cmp_strongeq/cmp.strongeq.pass.cpp
  failed: libc++.std/language_support/cmp/cmp_strongord/strongord.pass.cpp
  failed: libc++.std/language_support/cmp/cmp_weakeq/cmp.weakeq.pass.cpp
  failed: libc++.std/language_support/cmp/cmp_weakord/weakord.pass.cpp

{icon check-circle color=green} clang-tidy: pass.

{icon check-circle color=green} clang-format: pass.

Build artifacts 
: 
diff.json 
,
 clang-tidy.txt 
,
 clang-format.patch 
,
 CMakeCache.txt 
,
 console-log.txt 
,
 test-results.xml 


//Pre-merge checks is in beta. Report issue 
.
 Please join beta  or enable 
it for your project 
.//


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72829



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D72829: Implement -fsemantic-interposition

2020-01-23 Thread Fangrui Song via Phabricator via cfe-commits

MaskRay accepted this revision.
MaskRay added inline comments.



Comment at: clang/lib/CodeGen/CodeGenModule.cpp:488
+  if (Context.getLangOpts().SemanticInterposition &&
+  (Context.getLangOpts().PICLevel && !Context.getLangOpts().PIE))
+// Require various optimization to respect semantic interposition.

The parentheses can be deleted.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72829



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D72829: Implement -fsemantic-interposition

2020-01-23 Thread serge via Phabricator via cfe-commits

serge-sans-paille updated this revision to Diff 239996.
serge-sans-paille added a comment.

Take into account pie/pic interaction with semantic-interposition
More test case
Remove premature release note entry


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72829

Files:
  clang/docs/ClangCommandLineReference.rst
  clang/include/clang/Basic/LangOptions.def
  clang/include/clang/Driver/Options.td
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/test/CodeGen/semantic-interposition.c
  clang/test/Driver/clang_f_opts.c
  llvm/include/llvm/IR/GlobalValue.h
  llvm/include/llvm/IR/Module.h
  llvm/lib/IR/Globals.cpp
  llvm/lib/IR/Module.cpp
  llvm/lib/IR/Verifier.cpp
  llvm/test/Transforms/Inline/inline-semantic-interposition.ll
  llvm/test/Verifier/module-flags-semantic-interposition.ll

Index: llvm/test/Verifier/module-flags-semantic-interposition.ll
===
--- /dev/null
+++ llvm/test/Verifier/module-flags-semantic-interposition.ll
@@ -0,0 +1,12 @@
+; RUN: not llvm-as < %s -o /dev/null 2>&1 | FileCheck %s
+
+target datalayout = "e-m:e-p270:32:32-p271:32:32-p272:64:64-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-unknown-linux-gnu"
+
+@foo = dso_local global i32 1, align 4
+
+!llvm.module.flags = !{!0}
+
+!0 = !{i32 1, !"SemanticInterposition", float 1.}
+
+; CHECK: SemanticInterposition metadata requires constant integer argument
Index: llvm/test/Transforms/Inline/inline-semantic-interposition.ll
===
--- /dev/null
+++ llvm/test/Transforms/Inline/inline-semantic-interposition.ll
@@ -0,0 +1,24 @@
+; RUN: opt < %s -inline  -S | FileCheck %s
+
+define internal i32 @callee1(i32 %A) {
+  ret i32 %A
+}
+define i32 @callee2(i32 %A) {
+  ret i32 %A
+}
+
+; CHECK-LABEL: @caller
+define i32 @caller(i32 %A) {
+; CHECK-NOT: call i32 @callee1(i32 %A)
+  %A1 = call i32 @callee1(i32 %A)
+; CHECK: %A2 = call i32 @callee2(i32 %A)
+  %A2 = call i32 @callee2(i32 %A)
+; CHECK: add i32 %A, %A2
+  %R = add i32 %A1, %A2
+  ret i32 %R
+}
+
+
+!llvm.module.flags = !{!0}
+
+!0 = !{i32 1, !"SemanticInterposition", i32 1}
Index: llvm/lib/IR/Verifier.cpp
===
--- llvm/lib/IR/Verifier.cpp
+++ llvm/lib/IR/Verifier.cpp
@@ -1476,6 +1476,13 @@
"'Linker Options' named metadata no longer supported");
   }
 
+  if (ID->getString() == "SemanticInterposition") {
+ConstantInt *Value =
+mdconst::dyn_extract_or_null(Op->getOperand(2));
+Assert(Value,
+   "SemanticInterposition metadata requires constant integer argument");
+  }
+
   if (ID->getString() == "CG Profile") {
 for (const MDOperand  : cast(Op->getOperand(2))->operands())
   visitModuleFlagCGProfileEntry(MDO);
Index: llvm/lib/IR/Module.cpp
===
--- llvm/lib/IR/Module.cpp
+++ llvm/lib/IR/Module.cpp
@@ -554,6 +554,20 @@
: getModuleFlag("ProfileSummary"));
 }
 
+bool Module::getSemanticInterposition() const {
+  Metadata *MF = getModuleFlag("SemanticInterposition");
+
+  auto *Val = cast_or_null(MF);
+  if (!Val)
+return false;
+
+  return cast(Val->getValue())->getZExtValue();
+}
+
+void Module::setSemanticInterposition(bool SI) {
+  addModuleFlag(ModFlagBehavior::Error, "SemanticInterposition", SI);
+}
+
 void Module::setOwnedMemoryBuffer(std::unique_ptr MB) {
   OwnedMemoryBuffer = std::move(MB);
 }
Index: llvm/lib/IR/Globals.cpp
===
--- llvm/lib/IR/Globals.cpp
+++ llvm/lib/IR/Globals.cpp
@@ -94,6 +94,13 @@
   llvm_unreachable("not a global");
 }
 
+bool GlobalValue::isInterposable() const {
+  if (isInterposableLinkage(getLinkage()))
+return true;
+  return getParent() && getParent()->getSemanticInterposition() &&
+ !isDSOLocal();
+}
+
 unsigned GlobalValue::getAlignment() const {
   if (auto *GA = dyn_cast(this)) {
 // In general we cannot compute this at the IR level, but we try.
Index: llvm/include/llvm/IR/Module.h
===
--- llvm/include/llvm/IR/Module.h
+++ llvm/include/llvm/IR/Module.h
@@ -848,6 +848,12 @@
   Metadata *getProfileSummary(bool IsCS);
   /// @}
 
+  /// Returns whether semantic interposition is to be respected.
+  bool getSemanticInterposition() const;
+
+  /// Set whether semantic interposition is to be respected.
+  void setSemanticInterposition(bool);
+
   /// Returns true if PLT should be avoided for RTLib calls.
   bool getRtLibUseGOT() const;
 
Index: llvm/include/llvm/IR/GlobalValue.h
===
--- llvm/include/llvm/IR/GlobalValue.h
+++ llvm/include/llvm/IR/GlobalValue.h
@@ -426,7 +426,7

[PATCH] D72829: Implement -fsemantic-interposition

2020-01-23 Thread serge via Phabricator via cfe-commits

serge-sans-paille added a comment.



> I feel that we are still behind a complete `-fsemantic-interposition`... I 
> don't know whether we should mention that the option is experimental.

So do I. I can mention that in the -help output and in the changelog.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72829



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D72829: Implement -fsemantic-interposition

2020-01-23 Thread Fangrui Song via Phabricator via cfe-commits

MaskRay added a comment.

In D72829#1835933 , @serge-sans-paille 
wrote:

> Add Release note and doc. @MaskRay can you confirm current state is ok?


I feel that we are still behind a complete `-fsemantic-interposition`... I 
don't know whether we should mention that the option is experimental.




Comment at: clang/lib/CodeGen/CodeGenModule.cpp:487
+
+  if (Context.getLangOpts().SemanticInterposition)
+// Require various optimization to respect semantic interposition.

This should also check -fno-pic -fpie vs -fpic.

In GCC, only -fpic -fPIC enable semantic interposition.

-fsemantic-interpositions seems a no-op with -fno-pic or -fpie.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72829



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D72829: Implement -fsemantic-interposition

2020-01-23 Thread pre-merge checks [bot] via Phabricator via cfe-commits

merge_guards_bot added a comment.

{icon check-circle color=green} Unit tests: pass. 62135 tests passed, 0 failed 
and 808 were skipped.

{icon check-circle color=green} clang-tidy: pass.

{icon check-circle color=green} clang-format: pass.

Build artifacts 
: 
diff.json 
,
 clang-tidy.txt 
,
 clang-format.patch 
,
 CMakeCache.txt 
,
 console-log.txt 
,
 test-results.xml 


//Pre-merge checks is in beta. Report issue 
.
 Please | join beta  or | 
enable it for your project 
.//


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72829



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D72829: Implement -fsemantic-interposition

2020-01-23 Thread serge via Phabricator via cfe-commits

serge-sans-paille updated this revision to Diff 239857.
serge-sans-paille added a comment.

Add Release note and doc. @MaskRay can you confirm current state is ok?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72829

Files:
  clang/docs/ClangCommandLineReference.rst
  clang/docs/ReleaseNotes.rst
  clang/include/clang/Basic/LangOptions.def
  clang/include/clang/Driver/Options.td
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/lib/Frontend/CompilerInvocation.cpp
  llvm/include/llvm/IR/GlobalValue.h
  llvm/include/llvm/IR/Module.h
  llvm/lib/IR/Globals.cpp
  llvm/lib/IR/Module.cpp
  llvm/lib/IR/Verifier.cpp
  llvm/test/Transforms/Inline/inline-semantic-interposition.ll
  llvm/test/Verifier/module-flags-semantic-interposition.ll

Index: llvm/test/Verifier/module-flags-semantic-interposition.ll
===
--- /dev/null
+++ llvm/test/Verifier/module-flags-semantic-interposition.ll
@@ -0,0 +1,12 @@
+; RUN: not llvm-as < %s -o /dev/null 2>&1 | FileCheck %s
+
+target datalayout = "e-m:e-p270:32:32-p271:32:32-p272:64:64-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-unknown-linux-gnu"
+
+@foo = dso_local global i32 1, align 4
+
+!llvm.module.flags = !{!0}
+
+!0 = !{i32 1, !"SemanticInterposition", float 1.}
+
+; CHECK: SemanticInterposition metadata requires constant integer argument
Index: llvm/test/Transforms/Inline/inline-semantic-interposition.ll
===
--- /dev/null
+++ llvm/test/Transforms/Inline/inline-semantic-interposition.ll
@@ -0,0 +1,24 @@
+; RUN: opt < %s -inline  -S | FileCheck %s
+
+define internal i32 @callee1(i32 %A) {
+  ret i32 %A
+}
+define i32 @callee2(i32 %A) {
+  ret i32 %A
+}
+
+; CHECK-LABEL: @caller
+define i32 @caller(i32 %A) {
+; CHECK-NOT: call i32 @callee1(i32 %A)
+  %A1 = call i32 @callee1(i32 %A)
+; CHECK: %A2 = call i32 @callee2(i32 %A)
+  %A2 = call i32 @callee2(i32 %A)
+; CHECK: add i32 %A, %A2
+  %R = add i32 %A1, %A2
+  ret i32 %R
+}
+
+
+!llvm.module.flags = !{!0}
+
+!0 = !{i32 1, !"SemanticInterposition", i32 1}
Index: llvm/lib/IR/Verifier.cpp
===
--- llvm/lib/IR/Verifier.cpp
+++ llvm/lib/IR/Verifier.cpp
@@ -1476,6 +1476,13 @@
"'Linker Options' named metadata no longer supported");
   }
 
+  if (ID->getString() == "SemanticInterposition") {
+ConstantInt *Value =
+mdconst::dyn_extract_or_null(Op->getOperand(2));
+Assert(Value,
+   "SemanticInterposition metadata requires constant integer argument");
+  }
+
   if (ID->getString() == "CG Profile") {
 for (const MDOperand  : cast(Op->getOperand(2))->operands())
   visitModuleFlagCGProfileEntry(MDO);
Index: llvm/lib/IR/Module.cpp
===
--- llvm/lib/IR/Module.cpp
+++ llvm/lib/IR/Module.cpp
@@ -554,6 +554,20 @@
: getModuleFlag("ProfileSummary"));
 }
 
+bool Module::getSemanticInterposition() const {
+  Metadata *MF = getModuleFlag("SemanticInterposition");
+
+  auto *Val = cast_or_null(MF);
+  if (!Val)
+return false;
+
+  return cast(Val->getValue())->getZExtValue();
+}
+
+void Module::setSemanticInterposition(bool SI) {
+  addModuleFlag(ModFlagBehavior::Error, "SemanticInterposition", SI);
+}
+
 void Module::setOwnedMemoryBuffer(std::unique_ptr MB) {
   OwnedMemoryBuffer = std::move(MB);
 }
Index: llvm/lib/IR/Globals.cpp
===
--- llvm/lib/IR/Globals.cpp
+++ llvm/lib/IR/Globals.cpp
@@ -94,6 +94,13 @@
   llvm_unreachable("not a global");
 }
 
+bool GlobalValue::isInterposable() const {
+  if (isInterposableLinkage(getLinkage()))
+return true;
+  return getParent() && getParent()->getSemanticInterposition() &&
+ !isDSOLocal();
+}
+
 unsigned GlobalValue::getAlignment() const {
   if (auto *GA = dyn_cast(this)) {
 // In general we cannot compute this at the IR level, but we try.
Index: llvm/include/llvm/IR/Module.h
===
--- llvm/include/llvm/IR/Module.h
+++ llvm/include/llvm/IR/Module.h
@@ -848,6 +848,12 @@
   Metadata *getProfileSummary(bool IsCS);
   /// @}
 
+  /// Returns whether semantic interposition is to be respected.
+  bool getSemanticInterposition() const;
+
+  /// Set whether semantic interposition is to be respected.
+  void setSemanticInterposition(bool);
+
   /// Returns true if PLT should be avoided for RTLib calls.
   bool getRtLibUseGOT() const;
 
Index: llvm/include/llvm/IR/GlobalValue.h
===
--- llvm/include/llvm/IR/GlobalValue.h
+++ llvm/include/llvm/IR/GlobalValue.h
@@ -426,7 +426,7 @@
   /// *arbitrary* definition at link time.  We cannot do any IPO or inlinining
   /// across

[PATCH] D72829: Implement -fsemantic-interposition

2020-01-23 Thread pre-merge checks [bot] via Phabricator via cfe-commits

merge_guards_bot added a comment.

{icon check-circle color=green} Unit tests: pass. 62132 tests passed, 0 failed 
and 808 were skipped.

{icon check-circle color=green} clang-tidy: pass.

{icon check-circle color=green} clang-format: pass.

Build artifacts 
: 
diff.json 
,
 clang-tidy.txt 
,
 clang-format.patch 
,
 CMakeCache.txt 
,
 console-log.txt 
,
 test-results.xml 



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72829



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D72829: Implement -fsemantic-interposition

2020-01-23 Thread serge via Phabricator via cfe-commits

serge-sans-paille updated this revision to Diff 239837.
serge-sans-paille added a comment.

Formatting.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72829

Files:
  clang/include/clang/Basic/LangOptions.def
  clang/include/clang/Driver/Options.td
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/lib/Frontend/CompilerInvocation.cpp
  llvm/include/llvm/IR/GlobalValue.h
  llvm/include/llvm/IR/Module.h
  llvm/lib/IR/Globals.cpp
  llvm/lib/IR/Module.cpp
  llvm/lib/IR/Verifier.cpp
  llvm/test/Transforms/Inline/inline-semantic-interposition.ll
  llvm/test/Verifier/module-flags-semantic-interposition.ll

Index: llvm/test/Verifier/module-flags-semantic-interposition.ll
===
--- /dev/null
+++ llvm/test/Verifier/module-flags-semantic-interposition.ll
@@ -0,0 +1,12 @@
+; RUN: not llvm-as < %s -o /dev/null 2>&1 | FileCheck %s
+
+target datalayout = "e-m:e-p270:32:32-p271:32:32-p272:64:64-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-unknown-linux-gnu"
+
+@foo = dso_local global i32 1, align 4
+
+!llvm.module.flags = !{!0}
+
+!0 = !{i32 1, !"SemanticInterposition", float 1.}
+
+; CHECK: SemanticInterposition metadata requires constant integer argument
Index: llvm/test/Transforms/Inline/inline-semantic-interposition.ll
===
--- /dev/null
+++ llvm/test/Transforms/Inline/inline-semantic-interposition.ll
@@ -0,0 +1,24 @@
+; RUN: opt < %s -inline  -S | FileCheck %s
+
+define internal i32 @callee1(i32 %A) {
+  ret i32 %A
+}
+define i32 @callee2(i32 %A) {
+  ret i32 %A
+}
+
+; CHECK-LABEL: @caller
+define i32 @caller(i32 %A) {
+; CHECK-NOT: call i32 @callee1(i32 %A)
+  %A1 = call i32 @callee1(i32 %A)
+; CHECK: %A2 = call i32 @callee2(i32 %A)
+  %A2 = call i32 @callee2(i32 %A)
+; CHECK: add i32 %A, %A2
+  %R = add i32 %A1, %A2
+  ret i32 %R
+}
+
+
+!llvm.module.flags = !{!0}
+
+!0 = !{i32 1, !"SemanticInterposition", i32 1}
Index: llvm/lib/IR/Verifier.cpp
===
--- llvm/lib/IR/Verifier.cpp
+++ llvm/lib/IR/Verifier.cpp
@@ -1476,6 +1476,13 @@
"'Linker Options' named metadata no longer supported");
   }
 
+  if (ID->getString() == "SemanticInterposition") {
+ConstantInt *Value =
+mdconst::dyn_extract_or_null(Op->getOperand(2));
+Assert(Value,
+   "SemanticInterposition metadata requires constant integer argument");
+  }
+
   if (ID->getString() == "CG Profile") {
 for (const MDOperand  : cast(Op->getOperand(2))->operands())
   visitModuleFlagCGProfileEntry(MDO);
Index: llvm/lib/IR/Module.cpp
===
--- llvm/lib/IR/Module.cpp
+++ llvm/lib/IR/Module.cpp
@@ -554,6 +554,20 @@
: getModuleFlag("ProfileSummary"));
 }
 
+bool Module::getSemanticInterposition() const {
+  Metadata *MF = getModuleFlag("SemanticInterposition");
+
+  auto *Val = cast_or_null(MF);
+  if (!Val)
+return false;
+
+  return cast(Val->getValue())->getZExtValue();
+}
+
+void Module::setSemanticInterposition(bool SI) {
+  addModuleFlag(ModFlagBehavior::Error, "SemanticInterposition", SI);
+}
+
 void Module::setOwnedMemoryBuffer(std::unique_ptr MB) {
   OwnedMemoryBuffer = std::move(MB);
 }
Index: llvm/lib/IR/Globals.cpp
===
--- llvm/lib/IR/Globals.cpp
+++ llvm/lib/IR/Globals.cpp
@@ -94,6 +94,13 @@
   llvm_unreachable("not a global");
 }
 
+bool GlobalValue::isInterposable() const {
+  if (isInterposableLinkage(getLinkage()))
+return true;
+  return getParent() && getParent()->getSemanticInterposition() &&
+ !isDSOLocal();
+}
+
 unsigned GlobalValue::getAlignment() const {
   if (auto *GA = dyn_cast(this)) {
 // In general we cannot compute this at the IR level, but we try.
Index: llvm/include/llvm/IR/Module.h
===
--- llvm/include/llvm/IR/Module.h
+++ llvm/include/llvm/IR/Module.h
@@ -848,6 +848,12 @@
   Metadata *getProfileSummary(bool IsCS);
   /// @}
 
+  /// Returns whether semantic interposition is to be respected.
+  bool getSemanticInterposition() const;
+
+  /// Set whether semantic interposition is to be respected.
+  void setSemanticInterposition(bool);
+
   /// Returns true if PLT should be avoided for RTLib calls.
   bool getRtLibUseGOT() const;
 
Index: llvm/include/llvm/IR/GlobalValue.h
===
--- llvm/include/llvm/IR/GlobalValue.h
+++ llvm/include/llvm/IR/GlobalValue.h
@@ -426,7 +426,7 @@
   /// *arbitrary* definition at link time.  We cannot do any IPO or inlinining
   /// across interposable call edges, since the callee can be replaced with
   /// something arbitrary at link time.
-  bool isInterposable()

[PATCH] D72829: Implement -fsemantic-interposition

2020-01-23 Thread pre-merge checks [bot] via Phabricator via cfe-commits

merge_guards_bot added a comment.

{icon check-circle color=green} Unit tests: pass. 62134 tests passed, 0 failed 
and 808 were skipped.

{icon check-circle color=green} clang-tidy: pass.

{icon check-circle color=green} clang-format: pass.

Build artifacts 
: 
diff.json 
,
 clang-tidy.txt 
,
 clang-format.patch 
,
 CMakeCache.txt 
,
 console-log.txt 
,
 test-results.xml 



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72829



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D72829: Implement -fsemantic-interposition

2020-01-23 Thread serge via Phabricator via cfe-commits

serge-sans-paille updated this revision to Diff 239809.
serge-sans-paille added a comment.

Take review into account


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72829

Files:
  clang/include/clang/Basic/LangOptions.def
  clang/include/clang/Driver/Options.td
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/lib/Frontend/CompilerInvocation.cpp
  llvm/include/llvm/IR/GlobalValue.h
  llvm/include/llvm/IR/Module.h
  llvm/lib/IR/Globals.cpp
  llvm/lib/IR/Module.cpp
  llvm/lib/IR/Verifier.cpp
  llvm/test/Transforms/Inline/inline-semantic-interposition.ll
  llvm/test/Verifier/module-flags-semantic-interposition.ll

Index: llvm/test/Verifier/module-flags-semantic-interposition.ll
===
--- /dev/null
+++ llvm/test/Verifier/module-flags-semantic-interposition.ll
@@ -0,0 +1,12 @@
+; RUN: not llvm-as < %s -o /dev/null 2>&1 | FileCheck %s
+
+target datalayout = "e-m:e-p270:32:32-p271:32:32-p272:64:64-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-unknown-linux-gnu"
+
+@foo = dso_local global i32 1, align 4
+
+!llvm.module.flags = !{!0}
+
+!0 = !{i32 1, !"SemanticInterposition", float 1.}
+
+; CHECK: SemanticInterposition metadata requires constant integer argument
Index: llvm/test/Transforms/Inline/inline-semantic-interposition.ll
===
--- /dev/null
+++ llvm/test/Transforms/Inline/inline-semantic-interposition.ll
@@ -0,0 +1,24 @@
+; RUN: opt < %s -inline  -S | FileCheck %s
+
+define internal i32 @callee1(i32 %A) {
+  ret i32 %A
+}
+define i32 @callee2(i32 %A) {
+  ret i32 %A
+}
+
+; CHECK-LABEL: @caller
+define i32 @caller(i32 %A) {
+; CHECK-NOT: call i32 @callee1(i32 %A)
+  %A1 = call i32 @callee1(i32 %A)
+; CHECK: %A2 = call i32 @callee2(i32 %A)
+  %A2 = call i32 @callee2(i32 %A)
+; CHECK: add i32 %A, %A2
+  %R = add i32 %A1, %A2
+  ret i32 %R
+}
+
+
+!llvm.module.flags = !{!0}
+
+!0 = !{i32 1, !"SemanticInterposition", i32 1}
Index: llvm/lib/IR/Verifier.cpp
===
--- llvm/lib/IR/Verifier.cpp
+++ llvm/lib/IR/Verifier.cpp
@@ -1476,6 +1476,13 @@
"'Linker Options' named metadata no longer supported");
   }
 
+  if (ID->getString() == "SemanticInterposition") {
+ConstantInt *Value =
+mdconst::dyn_extract_or_null(Op->getOperand(2));
+Assert(Value,
+   "SemanticInterposition metadata requires constant integer argument");
+  }
+
   if (ID->getString() == "CG Profile") {
 for (const MDOperand  : cast(Op->getOperand(2))->operands())
   visitModuleFlagCGProfileEntry(MDO);
Index: llvm/lib/IR/Module.cpp
===
--- llvm/lib/IR/Module.cpp
+++ llvm/lib/IR/Module.cpp
@@ -554,6 +554,20 @@
: getModuleFlag("ProfileSummary"));
 }
 
+bool Module::getSemanticInterposition() const {
+  auto *Val =
+  cast_or_null(getModuleFlag("SemanticInterposition"));
+
+  if (!Val)
+return false;
+
+  return cast(Val->getValue())->getZExtValue();
+}
+
+void Module::setSemanticInterposition(bool SI) {
+  addModuleFlag(ModFlagBehavior::Error, "SemanticInterposition", SI);
+}
+
 void Module::setOwnedMemoryBuffer(std::unique_ptr MB) {
   OwnedMemoryBuffer = std::move(MB);
 }
Index: llvm/lib/IR/Globals.cpp
===
--- llvm/lib/IR/Globals.cpp
+++ llvm/lib/IR/Globals.cpp
@@ -94,6 +94,13 @@
   llvm_unreachable("not a global");
 }
 
+bool GlobalValue::isInterposable() const {
+  if (isInterposableLinkage(getLinkage()))
+return true;
+  return getParent() && getParent()->getSemanticInterposition() &&
+ !isDSOLocal();
+}
+
 unsigned GlobalValue::getAlignment() const {
   if (auto *GA = dyn_cast(this)) {
 // In general we cannot compute this at the IR level, but we try.
Index: llvm/include/llvm/IR/Module.h
===
--- llvm/include/llvm/IR/Module.h
+++ llvm/include/llvm/IR/Module.h
@@ -848,6 +848,12 @@
   Metadata *getProfileSummary(bool IsCS);
   /// @}
 
+  /// Returns whether semantic interposition is to be respected.
+  bool getSemanticInterposition() const;
+
+  /// Set whether semantic interposition is to be respected.
+  void setSemanticInterposition(bool);
+
   /// Returns true if PLT should be avoided for RTLib calls.
   bool getRtLibUseGOT() const;
 
Index: llvm/include/llvm/IR/GlobalValue.h
===
--- llvm/include/llvm/IR/GlobalValue.h
+++ llvm/include/llvm/IR/GlobalValue.h
@@ -426,7 +426,7 @@
   /// *arbitrary* definition at link time.  We cannot do any IPO or inlinining
   /// across interposable call edges, since the callee can be replaced with
   /// something arbitrary at link time.
-  bool isInterposable() const

[PATCH] D72829: Implement -fsemantic-interposition

2020-01-23 Thread George Rimar via Phabricator via cfe-commits

grimar added inline comments.



Comment at: llvm/lib/IR/Module.cpp:558
+bool Module::getSemanticInterposition() const {
+  auto *Val =
+  cast_or_null(getModuleFlag("SemanticInterposition"));

A minor nit about style:
This probably would be nicer as

```
  if (auto *Val = cast_or_null(
  getModuleFlag("SemanticInterposition")))
return cast(Val->getValue())->getZExtValue();
  return false;
```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72829



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D72829: Implement -fsemantic-interposition

2020-01-22 Thread Fangrui Song via Phabricator via cfe-commits

MaskRay accepted this revision.
MaskRay added inline comments.
This revision is now accepted and ready to land.



Comment at: llvm/include/llvm/IR/GlobalValue.h:287
 
+  bool isDSOPreemptable() const { return !IsDSOLocal; }
+

MaskRay wrote:
> It seems that this utility is not very necessary.
`isDSOPreemptable` is not used in the new patch. Forgot to remove?



Comment at: llvm/include/llvm/IR/Module.h:851
 
+  /// @name Utility functions for querying and setting semantic interposition
+  /// @{

It does not seem that we use `@{` `@}` nowadays... They don't add much 
documentation value so they can probably be removed.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72829



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D72829: Implement -fsemantic-interposition

2020-01-22 Thread pre-merge checks [bot] via Phabricator via cfe-commits

merge_guards_bot added a comment.

{icon check-circle color=green} Unit tests: pass. 62107 tests passed, 0 failed 
and 808 were skipped.

{icon question-circle color=gray} clang-tidy: unknown.

{icon check-circle color=green} clang-format: pass.

Build artifacts 
: 
diff.json 
,
 clang-format.patch 
,
 CMakeCache.txt 
,
 console-log.txt 
,
 test-results.xml 



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72829



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D72829: Implement -fsemantic-interposition

2020-01-22 Thread serge via Phabricator via cfe-commits

serge-sans-paille added a comment.

> I tried modifying `GlobalValue::maybeSetDsoLocal`: `setDSOLocal(true)` for 
> ExternalLinkage. Unfortunately that will break 1337 tests. If not so many 
> tests need fixing, I wish we can place `getParent() && 
> getParent()->getSemanticInterposition()` logic in `maybeSetDsoLocal` and 
> simplify `isInterposable`.

Tried that and ended up with various check failures.
I also tried to update isInterposableLinkage to either

- set more linkage type as interposable unconditionnaly, and with no surprise, 
it triggers plenty check failures
- accept a mandatory « `HonorSemanticInterposition` argument, so that both 
`isInterposableLinkage` and `isInterposable` are consistent, but in some cases, 
`isInterposableLinkage` doesn't have a module reference to gather the 
ModuleFlag from...


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72829



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D72829: Implement -fsemantic-interposition

2020-01-22 Thread serge via Phabricator via cfe-commits

serge-sans-paille updated this revision to Diff 239598.
serge-sans-paille added a comment.

- extra  test case


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72829

Files:
  clang/include/clang/Basic/LangOptions.def
  clang/include/clang/Driver/Options.td
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/lib/Frontend/CompilerInvocation.cpp
  llvm/include/llvm/IR/GlobalValue.h
  llvm/include/llvm/IR/Module.h
  llvm/lib/IR/Globals.cpp
  llvm/lib/IR/Module.cpp
  llvm/lib/IR/Verifier.cpp
  llvm/test/Transforms/Inline/inline-semantic-interposition.ll
  llvm/test/Verifier/module-flags-semantic-interposition.ll

Index: llvm/test/Verifier/module-flags-semantic-interposition.ll
===
--- /dev/null
+++ llvm/test/Verifier/module-flags-semantic-interposition.ll
@@ -0,0 +1,12 @@
+; RUN: not llvm-as < %s -o /dev/null 2>&1 | FileCheck %s
+
+target datalayout = "e-m:e-p270:32:32-p271:32:32-p272:64:64-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-unknown-linux-gnu"
+
+@foo = dso_local global i32 1, align 4
+
+!llvm.module.flags = !{!0}
+
+!0 = !{i32 1, !"SemanticInterposition", float 1.}
+
+; CHECK: SemanticInterposition metadata requires constant integer argument
Index: llvm/test/Transforms/Inline/inline-semantic-interposition.ll
===
--- /dev/null
+++ llvm/test/Transforms/Inline/inline-semantic-interposition.ll
@@ -0,0 +1,24 @@
+; RUN: opt < %s -inline  -S | FileCheck %s
+
+define internal i32 @callee1(i32 %A) {
+  ret i32 %A
+}
+define i32 @callee2(i32 %A) {
+  ret i32 %A
+}
+
+; CHECK-LABEL: @caller
+define i32 @caller(i32 %A) {
+; CHECK-NOT: call i32 @callee1(i32 %A)
+  %A1 = call i32 @callee1(i32 %A)
+; CHECK: %A2 = call i32 @callee2(i32 %A)
+  %A2 = call i32 @callee2(i32 %A)
+; CHECK: add i32 %A, %A2
+  %R = add i32 %A1, %A2
+  ret i32 %R
+}
+
+
+!llvm.module.flags = !{!0}
+
+!0 = !{i32 1, !"SemanticInterposition", i32 1}
Index: llvm/lib/IR/Verifier.cpp
===
--- llvm/lib/IR/Verifier.cpp
+++ llvm/lib/IR/Verifier.cpp
@@ -1476,6 +1476,13 @@
"'Linker Options' named metadata no longer supported");
   }
 
+  if (ID->getString() == "SemanticInterposition") {
+ConstantInt *Value =
+mdconst::dyn_extract_or_null(Op->getOperand(2));
+Assert(Value,
+   "SemanticInterposition metadata requires constant integer argument");
+  }
+
   if (ID->getString() == "CG Profile") {
 for (const MDOperand  : cast(Op->getOperand(2))->operands())
   visitModuleFlagCGProfileEntry(MDO);
Index: llvm/lib/IR/Module.cpp
===
--- llvm/lib/IR/Module.cpp
+++ llvm/lib/IR/Module.cpp
@@ -554,6 +554,20 @@
: getModuleFlag("ProfileSummary"));
 }
 
+bool Module::getSemanticInterposition() const {
+  auto *Val =
+  cast_or_null(getModuleFlag("SemanticInterposition"));
+
+  if (!Val)
+return false;
+
+  return cast(Val->getValue())->getZExtValue();
+}
+
+void Module::setSemanticInterposition(bool SI) {
+  addModuleFlag(ModFlagBehavior::Error, "SemanticInterposition", SI);
+}
+
 void Module::setOwnedMemoryBuffer(std::unique_ptr MB) {
   OwnedMemoryBuffer = std::move(MB);
 }
Index: llvm/lib/IR/Globals.cpp
===
--- llvm/lib/IR/Globals.cpp
+++ llvm/lib/IR/Globals.cpp
@@ -94,6 +94,13 @@
   llvm_unreachable("not a global");
 }
 
+bool GlobalValue::isInterposable() const {
+  if (isInterposableLinkage(getLinkage()))
+return true;
+  return getParent() && getParent()->getSemanticInterposition() &&
+ !isDSOLocal();
+}
+
 unsigned GlobalValue::getAlignment() const {
   if (auto *GA = dyn_cast(this)) {
 // In general we cannot compute this at the IR level, but we try.
Index: llvm/include/llvm/IR/Module.h
===
--- llvm/include/llvm/IR/Module.h
+++ llvm/include/llvm/IR/Module.h
@@ -848,6 +848,17 @@
   Metadata *getProfileSummary(bool IsCS);
   /// @}
 
+  /// @name Utility functions for querying and setting semantic interposition
+  /// @{
+
+  /// Returns whether semantic interposition is to be respected.
+  bool getSemanticInterposition() const;
+
+  /// Set whether semantic interposition is to be respected.
+  void setSemanticInterposition(bool);
+
+  /// @}
+
   /// Returns true if PLT should be avoided for RTLib calls.
   bool getRtLibUseGOT() const;
 
Index: llvm/include/llvm/IR/GlobalValue.h
===
--- llvm/include/llvm/IR/GlobalValue.h
+++ llvm/include/llvm/IR/GlobalValue.h
@@ -284,6 +284,8 @@
 return IsDSOLocal;
   }
 
+  bool isDSOPreemptable() const { return !IsDSOLocal; }
+
   bool hasPartition() const {

[PATCH] D72829: Implement -fsemantic-interposition

2020-01-21 Thread Fangrui Song via Phabricator via cfe-commits

MaskRay added a comment.

In D72829#1831017 , @serge-sans-paille 
wrote:

> > Linkages which were not interposable before and can be interposable now: 
> > available_externally, linkonce_odr, weak_odr, external, and appending.
>
> @MaskRay I understand the motivation behind that sentence, but do we want 
> that change to be non-conditional, i.e. not guarded by 
> -fsemantic-interposition ?


I tried modifying `GlobalValue::maybeSetDsoLocal`: `setDSOLocal(true)` for 
ExternalLinkage. Unfortunately that will break 1337 tests. If not so many tests 
need fixing, I wish we can place `getParent() && 
getParent()->getSemanticInterposition()` logic in `maybeSetDsoLocal` and 
simplify `isInterposable`.

  if (!isInterposableLinkage(getLinkage()))
return false;
  - return getParent() && getParent()->getSemanticInterposition() && 
!isDSOLocal();
  + return !isDSOLocal(); 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72829



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D72829: Implement -fsemantic-interposition

2020-01-21 Thread Fangrui Song via Phabricator via cfe-commits

MaskRay added inline comments.



Comment at: llvm/lib/IR/Globals.cpp:101
+return true;
+  return isInterposableLinkage(getLinkage());
+}

MaskRay wrote:
> MaskRay wrote:
> > Checking `isInterposableLinkage(getLinkage())` first may be more efficient.
> `if (!isInterposableLinkage(getLinkage())) return false;`
```lang=cpp
if (!isInterposableLinkage(getLinkage()))
  return false;
return getParent() && getParent()->getSemanticInterposition() && !isDSOLocal();
```



Comment at: llvm/lib/IR/Module.cpp:564
+
+  return cast(Val->getValue())->getZExtValue();
+}

A test/llvm/Verifier/ test checking "SemanticInterposition" must be a 
ConstantInt will be nice.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72829



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D72829: Implement -fsemantic-interposition

2020-01-21 Thread pre-merge checks [bot] via Phabricator via cfe-commits

merge_guards_bot added a comment.

{icon check-circle color=green} Unit tests: pass. 62055 tests passed, 0 failed 
and 784 were skipped.

{icon question-circle color=gray} clang-tidy: unknown.

{icon check-circle color=green} clang-format: pass.

Build artifacts 
: 
diff.json 
,
 clang-format.patch 
,
 CMakeCache.txt 
,
 console-log.txt 
,
 test-results.xml 



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72829



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D72829: Implement -fsemantic-interposition

2020-01-21 Thread serge via Phabricator via cfe-commits

serge-sans-paille added a comment.

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

@MaskRay I understand the motivation behind that sentence, but do we want that 
change to be non-conditional, i.e. not guarded by -fsemantic-interposition ?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72829



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D72829: Implement -fsemantic-interposition

2020-01-21 Thread serge via Phabricator via cfe-commits

serge-sans-paille updated this revision to Diff 239286.
serge-sans-paille added a comment.

Take into account review


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72829

Files:
  clang/include/clang/Basic/LangOptions.def
  clang/include/clang/Driver/Options.td
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/lib/Frontend/CompilerInvocation.cpp
  llvm/include/llvm/IR/GlobalValue.h
  llvm/include/llvm/IR/Module.h
  llvm/lib/IR/Globals.cpp
  llvm/lib/IR/Module.cpp

Index: llvm/lib/IR/Module.cpp
===
--- llvm/lib/IR/Module.cpp
+++ llvm/lib/IR/Module.cpp
@@ -554,6 +554,20 @@
: getModuleFlag("ProfileSummary"));
 }
 
+bool Module::getSemanticInterposition() const {
+  auto *Val =
+  cast_or_null(getModuleFlag("SemanticInterposition"));
+
+  if (!Val)
+return false;
+
+  return cast(Val->getValue())->getZExtValue();
+}
+
+void Module::setSemanticInterposition(bool SI) {
+  addModuleFlag(ModFlagBehavior::Error, "SemanticInterposition", SI);
+}
+
 void Module::setOwnedMemoryBuffer(std::unique_ptr MB) {
   OwnedMemoryBuffer = std::move(MB);
 }
Index: llvm/lib/IR/Globals.cpp
===
--- llvm/lib/IR/Globals.cpp
+++ llvm/lib/IR/Globals.cpp
@@ -94,6 +94,20 @@
   llvm_unreachable("not a global");
 }
 
+bool GlobalValue::isInterposable() const {
+
+  if (isInterposableLinkage(getLinkage()))
+return true;
+
+  bool const HonorSemanticInterposition =
+  getParent() && getParent()->getSemanticInterposition();
+
+  if (HonorSemanticInterposition && !isDSOLocal())
+return true;
+
+  return false;
+}
+
 unsigned GlobalValue::getAlignment() const {
   if (auto *GA = dyn_cast(this)) {
 // In general we cannot compute this at the IR level, but we try.
Index: llvm/include/llvm/IR/Module.h
===
--- llvm/include/llvm/IR/Module.h
+++ llvm/include/llvm/IR/Module.h
@@ -848,6 +848,17 @@
   Metadata *getProfileSummary(bool IsCS);
   /// @}
 
+  /// @name Utility functions for querying and setting semantic interposition
+  /// @{
+
+  /// Returns whether semantic interposition is to be respected.
+  bool getSemanticInterposition() const;
+
+  /// Set whether semantic interposition is to be respected.
+  void setSemanticInterposition(bool);
+
+  /// @}
+
   /// Returns true if PLT should be avoided for RTLib calls.
   bool getRtLibUseGOT() const;
 
Index: llvm/include/llvm/IR/GlobalValue.h
===
--- llvm/include/llvm/IR/GlobalValue.h
+++ llvm/include/llvm/IR/GlobalValue.h
@@ -284,6 +284,8 @@
 return IsDSOLocal;
   }
 
+  bool isDSOPreemptable() const { return !IsDSOLocal; }
+
   bool hasPartition() const {
 return HasPartition;
   }
@@ -426,7 +428,7 @@
   /// *arbitrary* definition at link time.  We cannot do any IPO or inlinining
   /// across interposable call edges, since the callee can be replaced with
   /// something arbitrary at link time.
-  bool isInterposable() const { return isInterposableLinkage(getLinkage()); }
+  bool isInterposable() const;
 
   bool hasExternalLinkage() const { return isExternalLinkage(getLinkage()); }
   bool hasAvailableExternallyLinkage() const {
Index: clang/lib/Frontend/CompilerInvocation.cpp
===
--- clang/lib/Frontend/CompilerInvocation.cpp
+++ clang/lib/Frontend/CompilerInvocation.cpp
@@ -2685,6 +2685,9 @@
 Opts.setValueVisibilityMode(DefaultVisibility);
   }
 
+  if (Args.hasArg(OPT_fsemantic_interposition))
+Opts.SemanticInterposition = 1;
+
   // The type-visibility mode defaults to the value-visibility mode.
   if (Arg *typeVisOpt = Args.getLastArg(OPT_ftype_visibility)) {
 Opts.setTypeVisibilityMode(parseVisibility(typeVisOpt, Args, Diags));
Index: clang/lib/Driver/ToolChains/Clang.cpp
===
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -5037,6 +5037,10 @@
   options::OPT_fno_emulated_tls);
   Args.AddLastArg(CmdArgs, options::OPT_fkeep_static_consts);
 
+  if (Args.hasFlag(options::OPT_fsemantic_interposition,
+   options::OPT_fno_semantic_interposition, false))
+CmdArgs.push_back("-fsemantic-interposition");
+
   // AltiVec-like language extensions aren't relevant for assembling.
   if (!isa(JA) || Output.getType() != types::TY_PP_Asm)
 Args.AddLastArg(CmdArgs, options::OPT_fzvector);
Index: clang/lib/CodeGen/CodeGenModule.cpp
===
--- clang/lib/CodeGen/CodeGenModule.cpp
+++ clang/lib/CodeGen/CodeGenModule.cpp
@@ -483,6 +483,11 @@
 getModule().addModuleFlag(llvm::Module::Max, "Dwarf Version",

[PATCH] D72829: Implement -fsemantic-interposition

2020-01-20 Thread Fangrui Song via Phabricator via cfe-commits

MaskRay added inline comments.



Comment at: llvm/lib/IR/Globals.cpp:101
+return true;
+  return isInterposableLinkage(getLinkage());
+}

MaskRay wrote:
> Checking `isInterposableLinkage(getLinkage())` first may be more efficient.
`if (!isInterposableLinkage(getLinkage())) return false;`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72829



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D72829: Implement -fsemantic-interposition

2020-01-20 Thread Fangrui Song via Phabricator via cfe-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, Flags<[CC1Option]>;
+def fno_semantic_interposition: Flag<["-"], "fno-semantic-interposition">, 
Group, Flags<[CC1Option]>;
 defm non_call_exceptions : BooleanFFlag<"non-call-exceptions">, 
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(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



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D72829: Implement -fsemantic-interposition

2020-01-16 Thread pre-merge checks [bot] via Phabricator via cfe-commits

merge_guards_bot added a comment.

{icon check-circle color=green} Unit tests: pass. 61912 tests passed, 0 failed 
and 783 were skipped.

{icon question-circle color=gray} clang-tidy: unknown.

{icon check-circle color=green} clang-format: pass.

Build artifacts 
: 
diff.json 
,
 clang-format.patch 
,
 CMakeCache.txt 
,
 console-log.txt 
,
 test-results.xml 



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72829



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D72829: Implement -fsemantic-interposition

2020-01-16 Thread serge via Phabricator via cfe-commits

serge-sans-paille added a comment.

@MaskRay no example yet, I'd like your opinion on the approach first. It has 
the advantage of being non intrusive to the code base...


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72829



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D72829: Implement -fsemantic-interposition

2020-01-16 Thread serge via Phabricator via cfe-commits

serge-sans-paille created this revision.
serge-sans-paille added reviewers: MaskRay, hfinkel.
Herald added subscribers: llvm-commits, cfe-commits, haicheng, hiraditya, 
eraman.
Herald added projects: clang, LLVM.
serge-sans-paille added a comment.

@MaskRay no example yet, I'd like your opinion on the approach first. It has 
the advantage of being non intrusive to the code base...


First attempt at implementing -fsemantic-interposition.

Rely on `GlobalValue::isInterposable` that already captures most of the 
expected behavior.

Rely on a `ModuleFlag` to state whether we should respect SemanticInterposition 
or not. The default remains no.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D72829

Files:
  clang/include/clang/Basic/LangOptions.def
  clang/include/clang/Driver/Options.td
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/lib/Frontend/CompilerInvocation.cpp
  llvm/include/llvm/IR/GlobalValue.h
  llvm/include/llvm/IR/Module.h
  llvm/lib/Analysis/InlineCost.cpp
  llvm/lib/IR/Globals.cpp
  llvm/lib/IR/Module.cpp

Index: llvm/lib/IR/Module.cpp
===
--- llvm/lib/IR/Module.cpp
+++ llvm/lib/IR/Module.cpp
@@ -559,6 +559,20 @@
: getModuleFlag("ProfileSummary"));
 }
 
+bool Module::getSemanticInterposition() const {
+  auto *Val =
+  cast_or_null(getModuleFlag("SemanticInterposition"));
+
+  if (!Val)
+return false;
+
+  return cast(Val->getValue())->getZExtValue();
+}
+
+void Module::setSemanticInterposition(bool SI) {
+  addModuleFlag(ModFlagBehavior::Error, "SemanticInterposition", SI);
+}
+
 void Module::setOwnedMemoryBuffer(std::unique_ptr MB) {
   OwnedMemoryBuffer = std::move(MB);
 }
Index: llvm/lib/IR/Globals.cpp
===
--- llvm/lib/IR/Globals.cpp
+++ llvm/lib/IR/Globals.cpp
@@ -94,6 +94,13 @@
   llvm_unreachable("not a global");
 }
 
+bool GlobalValue::isInterposable() const {
+  if (getParent() && getParent()->getSemanticInterposition() &&
+  isDSOPreemptable())
+return true;
+  return isInterposableLinkage(getLinkage());
+}
+
 unsigned GlobalValue::getAlignment() const {
   if (auto *GA = dyn_cast(this)) {
 // In general we cannot compute this at the IR level, but we try.
Index: llvm/lib/Analysis/InlineCost.cpp
===
--- llvm/lib/Analysis/InlineCost.cpp
+++ llvm/lib/Analysis/InlineCost.cpp
@@ -1842,7 +1842,7 @@
 } else if (Operator::getOpcode(V) == Instruction::BitCast) {
   V = cast(V)->getOperand(0);
 } else if (GlobalAlias *GA = dyn_cast(V)) {
-  if (GA->isInterposable())
+  if (GA->isInterposable() || GA->isDSOPreemptable())
 break;
   V = GA->getAliasee();
 } else {
Index: llvm/include/llvm/IR/Module.h
===
--- llvm/include/llvm/IR/Module.h
+++ llvm/include/llvm/IR/Module.h
@@ -846,6 +846,17 @@
   Metadata *getProfileSummary(bool IsCS);
   /// @}
 
+  /// @name Utility functions for querying and setting semantic interposition
+  /// @{
+
+  /// Returns whether semantic interposition is to be respected.
+  bool getSemanticInterposition() const;
+
+  /// Set whether semantic interposition is to be respected.
+  void setSemanticInterposition(bool);
+
+  /// @}
+
   /// Returns true if PLT should be avoided for RTLib calls.
   bool getRtLibUseGOT() const;
 
Index: llvm/include/llvm/IR/GlobalValue.h
===
--- llvm/include/llvm/IR/GlobalValue.h
+++ llvm/include/llvm/IR/GlobalValue.h
@@ -284,6 +284,8 @@
 return IsDSOLocal;
   }
 
+  bool isDSOPreemptable() const { return !IsDSOLocal; }
+
   bool hasPartition() const {
 return HasPartition;
   }
@@ -426,7 +428,7 @@
   /// *arbitrary* definition at link time.  We cannot do any IPO or inlinining
   /// across interposable call edges, since the callee can be replaced with
   /// something arbitrary at link time.
-  bool isInterposable() const { return isInterposableLinkage(getLinkage()); }
+  bool isInterposable() const;
 
   bool hasExternalLinkage() const { return isExternalLinkage(getLinkage()); }
   bool hasAvailableExternallyLinkage() const {
Index: clang/lib/Frontend/CompilerInvocation.cpp
===
--- clang/lib/Frontend/CompilerInvocation.cpp
+++ clang/lib/Frontend/CompilerInvocation.cpp
@@ -2679,6 +2679,13 @@
 Opts.setValueVisibilityMode(DefaultVisibility);
   }
 
+  // Semantic interposition mode defaults to "false".
+  if (Arg *siOpt = Args.getLastArg(OPT_fsemantic_interposition,
+   OPT_fno_semantic_interposition)) {
+Opts.SemanticInterposition =
+unsigned(siOpt->getOption().getID() == OPT_fsemantic_interposition);
+  }
+
   // The type-visibility mode defaults to the value-visibility mode.
   if

59 matches

Mail list logo