[PATCH] D65286: [OpenCL] Allow OpenCL C style vector initialization in C++

[Evgeniy via Phabricator via cfe-commits]

ebrevnov added a comment.

Please be aware about build bot failure 
(http://lab.llvm.org:8011/builders/clang-cmake-thumbv7-full-sh/builds/2185) 
most likely caused by this change.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D65286



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

[PATCH] D102090: [CMake][ELF] Link libLLVM.so and libclang-cpp.so with -Bsymbolic-functions

[Evgeniy via Phabricator via cfe-commits]

ebrevnov added a comment.

While -Bsymbolic-funtions brings nice performance improvements it also changes 
symbol resolution order. That means we effectively disabled  preemption for 
functions and all references from inside libLLVM*.so will be resolved locally.  
But references to global data can still be interposed by external definitions. 
Do I understand correctly that main argument against using -Bsymbolic is 
potential issue with equality comparison of address of global? Or anything else?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D102090

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

[PATCH] D102090: [CMake][ELF] Link libLLVM.so and libclang-cpp.so with -Bsymbolic-functions

[Evgeniy via Phabricator via cfe-commits]

ebrevnov added a comment.

In D102090#3169726 , @rnk wrote:

> In D102090#3169439 , @ebrevnov 
> wrote:
>
>> While -Bsymbolic-funtions brings nice performance improvements it also 
>> changes symbol resolution order. That means we effectively disabled  
>> preemption for functions and all references from inside libLLVM*.so will be 
>> resolved locally.  But references to global data can still be interposed by 
>> external definitions. Do I understand correctly that main argument against 
>> using -Bsymbolic is potential issue with equality comparison of address of 
>> global? Or anything else?
>
> I think it has less to do with the uniqueness of the addresses and more to do 
> with the deduplication of the global storage. If you have an inline C++ 
> global variable present in LLVM's headers (think a static data member of a 
> class template instantiation), things go wrong quickly if there are two 
> copies of this global, one in LLVM, and the other in the user's binary. 
> Updates from one DSO will not be visible in the other.

I don't see what's wrong here (maybe you have specific example). IMHO, quite 
opposite, LLVM will consistently use it's own instance while user's binary it's 
own as if  there were two globals with different names in the first place. I 
have exactly that real life case where LLVM's function (coming from libLLVM*.so 
since functions are not interposed) refers to a global from the user's binary 
with the same name by completely different semantics.

> If you arrange the same situation with functions, they are usually 
> functionally equivalent even if there are minor code differences.

I don't think it is safe to assume that. Maybe in most cases, but not always.

> Generally, users are not trying to preempt LLVM's own function definitions. 
> The typical use cases are to override C library functionality such as malloc. 
> The performance benefit of -Bsymbolic-functions is worth making LLVM's own 
> functions non-interposable.

I don't think this is really relevant to the subject since performance is 
outside of the scope of my question. The question is if -Bsymbolic can protect 
us from unintentional preemption by user defined globals.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D102090

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

[PATCH] D102090: [CMake][ELF] Link libLLVM.so and libclang-cpp.so with -Bsymbolic-functions

[Evgeniy via Phabricator via cfe-commits]

ebrevnov added a comment.

In D102090#3172538 , @MaskRay wrote:

> To add on rnk's comment (deduplication of vague linkage data which may be 
> included in multiple shared objects), another big reason is -Bsymbolic data 
> does not work with copy relocations. -fno-pic programs generally cannot avoid 
> copy relocations (except mips; mips did this thing correctly).

Do you (by "doesn't work") mean if executable refers to a global defined in 
LLVM.*so then each of them will use their own copy?

> GCC 5 x86-64 has a regression 
>  that -fpie code can have 
> copy relocations, too.

Looks like the bug itself is still opened. I guess you meant to refer to the 
the following change:  After "x86-64: Optimize access to globals in PIE with 
copy reloc", GCC x86-64 asks the assembler to produce an R_X86_64_PC32 for an 
external data access." Looks like intended change ... why do you call it a 
regression?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D102090

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

[PATCH] D137149: Use PassGate from LLVMContext if any otherwise global one

[Evgeniy via Phabricator via cfe-commits]

ebrevnov updated this revision to Diff 477446.
ebrevnov marked an inline comment as done.
ebrevnov added a comment.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Updated


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137149

Files:
  clang/lib/CodeGen/BackendUtil.cpp
  llvm/include/llvm/IR/OptBisect.h
  llvm/include/llvm/Passes/StandardInstrumentations.h
  llvm/lib/Analysis/CallGraphSCCPass.cpp
  llvm/lib/Analysis/LoopPass.cpp
  llvm/lib/Analysis/RegionPass.cpp
  llvm/lib/IR/LLVMContextImpl.cpp
  llvm/lib/IR/OptBisect.cpp
  llvm/lib/IR/Pass.cpp
  llvm/lib/LTO/LTOBackend.cpp
  llvm/lib/LTO/ThinLTOCodeGenerator.cpp
  llvm/lib/Passes/PassBuilderBindings.cpp
  llvm/lib/Passes/StandardInstrumentations.cpp
  llvm/tools/opt/NewPMDriver.cpp
  llvm/unittests/IR/LegacyPassManagerTest.cpp
  llvm/unittests/IR/PassManagerTest.cpp

Index: llvm/unittests/IR/PassManagerTest.cpp
===
--- llvm/unittests/IR/PassManagerTest.cpp
+++ llvm/unittests/IR/PassManagerTest.cpp
@@ -826,7 +826,7 @@
   FunctionAnalysisManager FAM;
   FunctionPassManager FPM;
   PassInstrumentationCallbacks PIC;
-  StandardInstrumentations SI(/*DebugLogging*/ true);
+  StandardInstrumentations SI(M->getContext(), /*DebugLogging*/ true);
   SI.registerCallbacks(PIC, &FAM);
   FAM.registerPass([&] { return PassInstrumentationAnalysis(&PIC); });
   FAM.registerPass([&] { return DominatorTreeAnalysis(); });
@@ -872,7 +872,7 @@
   FunctionAnalysisManager FAM;
   FunctionPassManager FPM;
   PassInstrumentationCallbacks PIC;
-  StandardInstrumentations SI(/*DebugLogging*/ true);
+  StandardInstrumentations SI(M->getContext(), /*DebugLogging*/ true);
   SI.registerCallbacks(PIC, &FAM);
   FAM.registerPass([&] { return PassInstrumentationAnalysis(&PIC); });
   FAM.registerPass([&] { return DominatorTreeAnalysis(); });
@@ -937,7 +937,7 @@
   FunctionAnalysisManager FAM;
   FunctionPassManager FPM;
   PassInstrumentationCallbacks PIC;
-  StandardInstrumentations SI(/*DebugLogging*/ true);
+  StandardInstrumentations SI(M->getContext(), /*DebugLogging*/ true);
   SI.registerCallbacks(PIC, &FAM);
   FAM.registerPass([&] { return PassInstrumentationAnalysis(&PIC); });
   FAM.registerPass([&] { return DominatorTreeAnalysis(); });
Index: llvm/unittests/IR/LegacyPassManagerTest.cpp
===
--- llvm/unittests/IR/LegacyPassManagerTest.cpp
+++ llvm/unittests/IR/LegacyPassManagerTest.cpp
@@ -359,10 +359,8 @@
 struct CustomOptPassGate : public OptPassGate {
   bool Skip;
   CustomOptPassGate(bool Skip) : Skip(Skip) { }
-  bool shouldRunPass(const Pass *P, StringRef IRDescription) override {
-if (P->getPassKind() == PT_Module)
-  return !Skip;
-return OptPassGate::shouldRunPass(P, IRDescription);
+  bool shouldRunPass(const StringRef PassName, StringRef IRDescription) override {
+return !Skip;
   }
   bool isEnabled() const override { return true; }
 };
Index: llvm/tools/opt/NewPMDriver.cpp
===
--- llvm/tools/opt/NewPMDriver.cpp
+++ llvm/tools/opt/NewPMDriver.cpp
@@ -354,8 +354,8 @@
   PrintPassOptions PrintPassOpts;
   PrintPassOpts.Verbose = DebugPM == DebugLogging::Verbose;
   PrintPassOpts.SkipAnalyses = DebugPM == DebugLogging::Quiet;
-  StandardInstrumentations SI(DebugPM != DebugLogging::None, VerifyEachPass,
-  PrintPassOpts);
+  StandardInstrumentations SI(M.getContext(), DebugPM != DebugLogging::None,
+  VerifyEachPass, PrintPassOpts);
   SI.registerCallbacks(PIC, &FAM);
   DebugifyEachInstrumentation Debugify;
   DebugifyStatsMap DIStatsMap;
Index: llvm/lib/Passes/StandardInstrumentations.cpp
===
--- llvm/lib/Passes/StandardInstrumentations.cpp
+++ llvm/lib/Passes/StandardInstrumentations.cpp
@@ -766,27 +766,35 @@
   return ShouldRun;
 }
 
-void OptBisectInstrumentation::registerCallbacks(
+bool OptPassGateInstrumentation::shouldRun(StringRef PassName, Any IR) {
+  if (isIgnored(PassName))
+return true;
+
+  bool ShouldRun =
+  Context.getOptPassGate().shouldRunPass(PassName, getIRName(IR));
+  if (!ShouldRun && !this->HasWrittenIR && !OptBisectPrintIRPath.empty()) {
+// FIXME: print IR if limit is higher than number of opt-bisect
+// invocations
+this->HasWrittenIR = true;
+const Module *M = unwrapModule(IR, /*Force=*/true);
+assert((M && &M->getContext() == &Context) && "Missing/Mismatching Module");
+std::error_code EC;
+raw_fd_ostream OS(OptBisectPrintIRPath, EC);
+if (EC)
+  report_fatal_error(errorCodeToError(EC));
+M->print(OS, nullptr);
+  }
+  return ShouldRun;
+}
+
+void OptPassGateInstrumentation::registerCallbacks(
 PassInstrume

[PATCH] D137149: Use PassGate from LLVMContext if any otherwise global one

[Evgeniy via Phabricator via cfe-commits]

ebrevnov updated this revision to Diff 477706.
ebrevnov added a comment.
Herald added a project: Flang.

Fixed build error in flang/lib/Frontend/FrontendActions.cpp


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137149

Files:
  clang/lib/CodeGen/BackendUtil.cpp
  flang/lib/Frontend/FrontendActions.cpp
  llvm/include/llvm/IR/OptBisect.h
  llvm/include/llvm/Passes/StandardInstrumentations.h
  llvm/lib/Analysis/CallGraphSCCPass.cpp
  llvm/lib/Analysis/LoopPass.cpp
  llvm/lib/Analysis/RegionPass.cpp
  llvm/lib/IR/LLVMContextImpl.cpp
  llvm/lib/IR/OptBisect.cpp
  llvm/lib/IR/Pass.cpp
  llvm/lib/LTO/LTOBackend.cpp
  llvm/lib/LTO/ThinLTOCodeGenerator.cpp
  llvm/lib/Passes/PassBuilderBindings.cpp
  llvm/lib/Passes/StandardInstrumentations.cpp
  llvm/tools/opt/NewPMDriver.cpp
  llvm/unittests/IR/LegacyPassManagerTest.cpp
  llvm/unittests/IR/PassManagerTest.cpp

Index: llvm/unittests/IR/PassManagerTest.cpp
===
--- llvm/unittests/IR/PassManagerTest.cpp
+++ llvm/unittests/IR/PassManagerTest.cpp
@@ -826,7 +826,7 @@
   FunctionAnalysisManager FAM;
   FunctionPassManager FPM;
   PassInstrumentationCallbacks PIC;
-  StandardInstrumentations SI(/*DebugLogging*/ true);
+  StandardInstrumentations SI(M->getContext(), /*DebugLogging*/ true);
   SI.registerCallbacks(PIC, &FAM);
   FAM.registerPass([&] { return PassInstrumentationAnalysis(&PIC); });
   FAM.registerPass([&] { return DominatorTreeAnalysis(); });
@@ -872,7 +872,7 @@
   FunctionAnalysisManager FAM;
   FunctionPassManager FPM;
   PassInstrumentationCallbacks PIC;
-  StandardInstrumentations SI(/*DebugLogging*/ true);
+  StandardInstrumentations SI(M->getContext(), /*DebugLogging*/ true);
   SI.registerCallbacks(PIC, &FAM);
   FAM.registerPass([&] { return PassInstrumentationAnalysis(&PIC); });
   FAM.registerPass([&] { return DominatorTreeAnalysis(); });
@@ -937,7 +937,7 @@
   FunctionAnalysisManager FAM;
   FunctionPassManager FPM;
   PassInstrumentationCallbacks PIC;
-  StandardInstrumentations SI(/*DebugLogging*/ true);
+  StandardInstrumentations SI(M->getContext(), /*DebugLogging*/ true);
   SI.registerCallbacks(PIC, &FAM);
   FAM.registerPass([&] { return PassInstrumentationAnalysis(&PIC); });
   FAM.registerPass([&] { return DominatorTreeAnalysis(); });
Index: llvm/unittests/IR/LegacyPassManagerTest.cpp
===
--- llvm/unittests/IR/LegacyPassManagerTest.cpp
+++ llvm/unittests/IR/LegacyPassManagerTest.cpp
@@ -359,10 +359,8 @@
 struct CustomOptPassGate : public OptPassGate {
   bool Skip;
   CustomOptPassGate(bool Skip) : Skip(Skip) { }
-  bool shouldRunPass(const Pass *P, StringRef IRDescription) override {
-if (P->getPassKind() == PT_Module)
-  return !Skip;
-return OptPassGate::shouldRunPass(P, IRDescription);
+  bool shouldRunPass(const StringRef PassName, StringRef IRDescription) override {
+return !Skip;
   }
   bool isEnabled() const override { return true; }
 };
Index: llvm/tools/opt/NewPMDriver.cpp
===
--- llvm/tools/opt/NewPMDriver.cpp
+++ llvm/tools/opt/NewPMDriver.cpp
@@ -354,8 +354,8 @@
   PrintPassOptions PrintPassOpts;
   PrintPassOpts.Verbose = DebugPM == DebugLogging::Verbose;
   PrintPassOpts.SkipAnalyses = DebugPM == DebugLogging::Quiet;
-  StandardInstrumentations SI(DebugPM != DebugLogging::None, VerifyEachPass,
-  PrintPassOpts);
+  StandardInstrumentations SI(M.getContext(), DebugPM != DebugLogging::None,
+  VerifyEachPass, PrintPassOpts);
   SI.registerCallbacks(PIC, &FAM);
   DebugifyEachInstrumentation Debugify;
   DebugifyStatsMap DIStatsMap;
Index: llvm/lib/Passes/StandardInstrumentations.cpp
===
--- llvm/lib/Passes/StandardInstrumentations.cpp
+++ llvm/lib/Passes/StandardInstrumentations.cpp
@@ -767,27 +767,35 @@
   return ShouldRun;
 }
 
-void OptBisectInstrumentation::registerCallbacks(
+bool OptPassGateInstrumentation::shouldRun(StringRef PassName, Any IR) {
+  if (isIgnored(PassName))
+return true;
+
+  bool ShouldRun =
+  Context.getOptPassGate().shouldRunPass(PassName, getIRName(IR));
+  if (!ShouldRun && !this->HasWrittenIR && !OptBisectPrintIRPath.empty()) {
+// FIXME: print IR if limit is higher than number of opt-bisect
+// invocations
+this->HasWrittenIR = true;
+const Module *M = unwrapModule(IR, /*Force=*/true);
+assert((M && &M->getContext() == &Context) && "Missing/Mismatching Module");
+std::error_code EC;
+raw_fd_ostream OS(OptBisectPrintIRPath, EC);
+if (EC)
+  report_fatal_error(errorCodeToError(EC));
+M->print(OS, nullptr);
+  }
+  return ShouldRun;
+}
+
+void OptPassGateInstrumentation::registerCallbacks(
 Pa

[PATCH] D102090: [CMake][ELF] Link libLLVM.so and libclang-cpp.so with -Bsymbolic-functions

[Evgeniy via Phabricator via cfe-commits]

ebrevnov added a comment.

> ! In D102090#3174187 , @rnk wrote:
>  In any case, -Bsymbolic is not a good default for the project because of 
> issues like this.

I see. Thank you.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D102090

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