[PATCH] D80692: Run Coverage pass before other *San passes under new pass manager, round 2

2020-05-28 Thread Arthur Eubanks via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rG1285e8bcac2c: Run Coverage pass before other *San passes 
under new pass manager, round 2 (authored by aeubanks).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80692

Files:
  clang/lib/CodeGen/BackendUtil.cpp
  clang/test/CodeGen/sancov-new-pm.c
  llvm/include/llvm/Passes/PassBuilder.h
  llvm/lib/Passes/PassBuilder.cpp
  llvm/tools/opt/NewPMDriver.cpp

Index: llvm/tools/opt/NewPMDriver.cpp
===
--- llvm/tools/opt/NewPMDriver.cpp
+++ llvm/tools/opt/NewPMDriver.cpp
@@ -194,7 +194,7 @@
 });
   if (tryParsePipelineText(PB, OptimizerLastEPPipeline))
 PB.registerOptimizerLastEPCallback(
-[, VerifyEachPass, DebugLogging](FunctionPassManager ,
+[, VerifyEachPass, DebugLogging](ModulePassManager ,
 PassBuilder::OptimizationLevel) {
   ExitOnError Err("Unable to parse OptimizerLastEP pipeline: ");
   Err(PB.parsePassPipeline(PM, OptimizerLastEPPipeline, VerifyEachPass,
Index: llvm/lib/Passes/PassBuilder.cpp
===
--- llvm/lib/Passes/PassBuilder.cpp
+++ llvm/lib/Passes/PassBuilder.cpp
@@ -1073,12 +1073,12 @@
   if (PTO.Coroutines)
 OptimizePM.addPass(CoroCleanupPass());
 
-  for (auto  : OptimizerLastEPCallbacks)
-C(OptimizePM, Level);
-
   // Add the core optimizing pipeline.
   MPM.addPass(createModuleToFunctionPassAdaptor(std::move(OptimizePM)));
 
+  for (auto  : OptimizerLastEPCallbacks)
+C(MPM, Level);
+
   if (PTO.CallGraphProfile)
 MPM.addPass(CGProfilePass());
 
Index: llvm/include/llvm/Passes/PassBuilder.h
===
--- llvm/include/llvm/Passes/PassBuilder.h
+++ llvm/include/llvm/Passes/PassBuilder.h
@@ -600,7 +600,7 @@
   /// is not triggered at O0. Extensions to the O0 pipeline should append their
   /// passes to the end of the overall pipeline.
   void registerOptimizerLastEPCallback(
-  const std::function ) {
+  const std::function ) {
 OptimizerLastEPCallbacks.push_back(C);
   }
 
@@ -728,7 +728,7 @@
   CGSCCOptimizerLateEPCallbacks;
   SmallVector, 2>
   VectorizerStartEPCallbacks;
-  SmallVector, 2>
+  SmallVector, 2>
   OptimizerLastEPCallbacks;
   // Module callbacks
   SmallVector, 2>
Index: clang/test/CodeGen/sancov-new-pm.c
===
--- clang/test/CodeGen/sancov-new-pm.c
+++ clang/test/CodeGen/sancov-new-pm.c
@@ -1,10 +1,6 @@
 // Test that SanitizerCoverage works under the new pass manager.
 // RUN: %clang -target x86_64-linux-gnu -fsanitize=fuzzer %s -fexperimental-new-pass-manager -S -emit-llvm -o - | FileCheck %s --check-prefixes=CHECK,CHECK-O0
 // RUN: %clang -target x86_64-linux-gnu -fsanitize=fuzzer %s -fexperimental-new-pass-manager -O2 -S -emit-llvm -o - | FileCheck %s --check-prefixes=CHECK,CHECK-O2
-// RUN: %clang -target x86_64-linux-gnu -fsanitize=fuzzer %s -fexperimental-new-pass-manager -flto -S -emit-llvm -o - | FileCheck %s --check-prefixes=CHECK,CHECK-O0
-// RUN: %clang -target x86_64-linux-gnu -fsanitize=fuzzer %s -fexperimental-new-pass-manager -flto -O2 -S -emit-llvm -o - | FileCheck %s --check-prefixes=CHECK,CHECK-O2
-// RUN: %clang -target x86_64-linux-gnu -fsanitize=fuzzer %s -fexperimental-new-pass-manager -flto=thin -S -emit-llvm -o - | FileCheck %s --check-prefixes=CHECK,CHECK-O0
-// RUN: %clang -target x86_64-linux-gnu -fsanitize=fuzzer %s -fexperimental-new-pass-manager -flto=thin -O2 -S -emit-llvm -o - | FileCheck %s --check-prefixes=CHECK,CHECK-O2,CHECK-O2-THINLTO
 
 extern void *memcpy(void *, const void *, unsigned long);
 extern int printf(const char *restrict, ...);
@@ -29,10 +25,10 @@
 // CHECK-O0-DAG: declare void @__sanitizer_cov_trace_cmp2(i16 zeroext, i16 zeroext)
 // CHECK-O0-DAG: declare void @__sanitizer_cov_trace_cmp4(i32 zeroext, i32 zeroext)
 // CHECK-O0-DAG: declare void @__sanitizer_cov_trace_cmp8(i64, i64)
-// CHECK-O2-THINLTO-NOT: declare void @__sanitizer_cov_trace_const_cmp1(i8 zeroext, i8 zeroext)
+// CHECK-O2-NOT: declare void @__sanitizer_cov_trace_const_cmp1(i8 zeroext, i8 zeroext)
 // CHECK-O0-DAG: declare void @__sanitizer_cov_trace_const_cmp2(i16 zeroext, i16 zeroext)
 // CHECK-O0-DAG: declare void @__sanitizer_cov_trace_const_cmp4(i32 zeroext, i32 zeroext)
-// CHECK-O2-THINLTO-NOT: declare void @__sanitizer_cov_trace_const_cmp8(i64, i64)
+// CHECK-O2-NOT: declare void @__sanitizer_cov_trace_const_cmp8(i64, i64)
 // CHECK-O0-DAG: declare void @__sanitizer_cov_trace_div4(i32 zeroext)
 // CHECK-O0-DAG: declare void @__sanitizer_cov_trace_div8(i64)
 // CHECK-O0-DAG: declare void @__sanitizer_cov_trace_gep(i64)
Index: clang/lib/CodeGen/BackendUtil.cpp

[PATCH] D80692: Run Coverage pass before other *San passes under new pass manager, round 2

2020-05-28 Thread Arthur Eubanks via Phabricator via cfe-commits
aeubanks updated this revision to Diff 267087.
aeubanks added a comment.

Update some CHECKs


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80692

Files:
  clang/lib/CodeGen/BackendUtil.cpp
  clang/test/CodeGen/sancov-new-pm.c
  llvm/include/llvm/Passes/PassBuilder.h
  llvm/lib/Passes/PassBuilder.cpp
  llvm/tools/opt/NewPMDriver.cpp

Index: llvm/tools/opt/NewPMDriver.cpp
===
--- llvm/tools/opt/NewPMDriver.cpp
+++ llvm/tools/opt/NewPMDriver.cpp
@@ -194,7 +194,7 @@
 });
   if (tryParsePipelineText(PB, OptimizerLastEPPipeline))
 PB.registerOptimizerLastEPCallback(
-[, VerifyEachPass, DebugLogging](FunctionPassManager ,
+[, VerifyEachPass, DebugLogging](ModulePassManager ,
 PassBuilder::OptimizationLevel) {
   ExitOnError Err("Unable to parse OptimizerLastEP pipeline: ");
   Err(PB.parsePassPipeline(PM, OptimizerLastEPPipeline, VerifyEachPass,
Index: llvm/lib/Passes/PassBuilder.cpp
===
--- llvm/lib/Passes/PassBuilder.cpp
+++ llvm/lib/Passes/PassBuilder.cpp
@@ -1073,12 +1073,12 @@
   if (PTO.Coroutines)
 OptimizePM.addPass(CoroCleanupPass());
 
-  for (auto  : OptimizerLastEPCallbacks)
-C(OptimizePM, Level);
-
   // Add the core optimizing pipeline.
   MPM.addPass(createModuleToFunctionPassAdaptor(std::move(OptimizePM)));
 
+  for (auto  : OptimizerLastEPCallbacks)
+C(MPM, Level);
+
   if (PTO.CallGraphProfile)
 MPM.addPass(CGProfilePass());
 
Index: llvm/include/llvm/Passes/PassBuilder.h
===
--- llvm/include/llvm/Passes/PassBuilder.h
+++ llvm/include/llvm/Passes/PassBuilder.h
@@ -600,7 +600,7 @@
   /// is not triggered at O0. Extensions to the O0 pipeline should append their
   /// passes to the end of the overall pipeline.
   void registerOptimizerLastEPCallback(
-  const std::function ) {
+  const std::function ) {
 OptimizerLastEPCallbacks.push_back(C);
   }
 
@@ -728,7 +728,7 @@
   CGSCCOptimizerLateEPCallbacks;
   SmallVector, 2>
   VectorizerStartEPCallbacks;
-  SmallVector, 2>
+  SmallVector, 2>
   OptimizerLastEPCallbacks;
   // Module callbacks
   SmallVector, 2>
Index: clang/test/CodeGen/sancov-new-pm.c
===
--- clang/test/CodeGen/sancov-new-pm.c
+++ clang/test/CodeGen/sancov-new-pm.c
@@ -1,10 +1,6 @@
 // Test that SanitizerCoverage works under the new pass manager.
 // RUN: %clang -target x86_64-linux-gnu -fsanitize=fuzzer %s -fexperimental-new-pass-manager -S -emit-llvm -o - | FileCheck %s --check-prefixes=CHECK,CHECK-O0
 // RUN: %clang -target x86_64-linux-gnu -fsanitize=fuzzer %s -fexperimental-new-pass-manager -O2 -S -emit-llvm -o - | FileCheck %s --check-prefixes=CHECK,CHECK-O2
-// RUN: %clang -target x86_64-linux-gnu -fsanitize=fuzzer %s -fexperimental-new-pass-manager -flto -S -emit-llvm -o - | FileCheck %s --check-prefixes=CHECK,CHECK-O0
-// RUN: %clang -target x86_64-linux-gnu -fsanitize=fuzzer %s -fexperimental-new-pass-manager -flto -O2 -S -emit-llvm -o - | FileCheck %s --check-prefixes=CHECK,CHECK-O2
-// RUN: %clang -target x86_64-linux-gnu -fsanitize=fuzzer %s -fexperimental-new-pass-manager -flto=thin -S -emit-llvm -o - | FileCheck %s --check-prefixes=CHECK,CHECK-O0
-// RUN: %clang -target x86_64-linux-gnu -fsanitize=fuzzer %s -fexperimental-new-pass-manager -flto=thin -O2 -S -emit-llvm -o - | FileCheck %s --check-prefixes=CHECK,CHECK-O2,CHECK-O2-THINLTO
 
 extern void *memcpy(void *, const void *, unsigned long);
 extern int printf(const char *restrict, ...);
@@ -29,10 +25,10 @@
 // CHECK-O0-DAG: declare void @__sanitizer_cov_trace_cmp2(i16 zeroext, i16 zeroext)
 // CHECK-O0-DAG: declare void @__sanitizer_cov_trace_cmp4(i32 zeroext, i32 zeroext)
 // CHECK-O0-DAG: declare void @__sanitizer_cov_trace_cmp8(i64, i64)
-// CHECK-O2-THINLTO-NOT: declare void @__sanitizer_cov_trace_const_cmp1(i8 zeroext, i8 zeroext)
+// CHECK-O2-NOT: declare void @__sanitizer_cov_trace_const_cmp1(i8 zeroext, i8 zeroext)
 // CHECK-O0-DAG: declare void @__sanitizer_cov_trace_const_cmp2(i16 zeroext, i16 zeroext)
 // CHECK-O0-DAG: declare void @__sanitizer_cov_trace_const_cmp4(i32 zeroext, i32 zeroext)
-// CHECK-O2-THINLTO-NOT: declare void @__sanitizer_cov_trace_const_cmp8(i64, i64)
+// CHECK-O2-NOT: declare void @__sanitizer_cov_trace_const_cmp8(i64, i64)
 // CHECK-O0-DAG: declare void @__sanitizer_cov_trace_div4(i32 zeroext)
 // CHECK-O0-DAG: declare void @__sanitizer_cov_trace_div8(i64)
 // CHECK-O0-DAG: declare void @__sanitizer_cov_trace_gep(i64)
Index: clang/lib/CodeGen/BackendUtil.cpp
===
--- clang/lib/CodeGen/BackendUtil.cpp
+++ clang/lib/CodeGen/BackendUtil.cpp
@@ -32,6 +32,7 @@
 

[PATCH] D80692: Run Coverage pass before other *San passes under new pass manager, round 2

2020-05-28 Thread Leonard Chan via Phabricator via cfe-commits
leonardchan accepted this revision.
leonardchan added a comment.

In D80692#2061739 , @aeubanks wrote:

> I foolishly submitted without running check-all, and it turns out this broke 
> a test under check-clang.
>  Looks like https://reviews.llvm.org/D62888 added tests in sancov-new-pm.c to 
> make sure that sancov + LTO work together, but they won't after this change 
> since the (Thin)LTO pipeline doesn't run things under 
> `registerOptimizerLastEPCallback()`. So I deleted those tests. Other 
> sanitizers don't work under LTO right now (at least for non-O0) because of 
> the same thing. Does this make sense?


Makes sense. Could you also remove any CHECK lines for RUNs that were removed?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80692



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


[PATCH] D80692: Run Coverage pass before other *San passes under new pass manager, round 2

2020-05-28 Thread Arthur Eubanks via Phabricator via cfe-commits
aeubanks updated this revision to Diff 267073.
aeubanks added a comment.

I foolishly submitted without running check-all, and it turns out this broke a 
test under check-clang.
Looks like https://reviews.llvm.org/D62888 added tests in sancov-new-pm.c to 
make sure that sancov + LTO work together, but they won't after this change 
since the (Thin)LTO pipeline doesn't run things under 
`registerOptimizerLastEPCallback()`. So I deleted those tests. Other sanitizers 
don't work under LTO right now (at least for non-O0) because of the same thing. 
Does this make sense?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80692

Files:
  clang/lib/CodeGen/BackendUtil.cpp
  clang/test/CodeGen/sancov-new-pm.c
  llvm/include/llvm/Passes/PassBuilder.h
  llvm/lib/Passes/PassBuilder.cpp
  llvm/tools/opt/NewPMDriver.cpp

Index: llvm/tools/opt/NewPMDriver.cpp
===
--- llvm/tools/opt/NewPMDriver.cpp
+++ llvm/tools/opt/NewPMDriver.cpp
@@ -194,7 +194,7 @@
 });
   if (tryParsePipelineText(PB, OptimizerLastEPPipeline))
 PB.registerOptimizerLastEPCallback(
-[, VerifyEachPass, DebugLogging](FunctionPassManager ,
+[, VerifyEachPass, DebugLogging](ModulePassManager ,
 PassBuilder::OptimizationLevel) {
   ExitOnError Err("Unable to parse OptimizerLastEP pipeline: ");
   Err(PB.parsePassPipeline(PM, OptimizerLastEPPipeline, VerifyEachPass,
Index: llvm/lib/Passes/PassBuilder.cpp
===
--- llvm/lib/Passes/PassBuilder.cpp
+++ llvm/lib/Passes/PassBuilder.cpp
@@ -1073,12 +1073,12 @@
   if (PTO.Coroutines)
 OptimizePM.addPass(CoroCleanupPass());
 
-  for (auto  : OptimizerLastEPCallbacks)
-C(OptimizePM, Level);
-
   // Add the core optimizing pipeline.
   MPM.addPass(createModuleToFunctionPassAdaptor(std::move(OptimizePM)));
 
+  for (auto  : OptimizerLastEPCallbacks)
+C(MPM, Level);
+
   if (PTO.CallGraphProfile)
 MPM.addPass(CGProfilePass());
 
Index: llvm/include/llvm/Passes/PassBuilder.h
===
--- llvm/include/llvm/Passes/PassBuilder.h
+++ llvm/include/llvm/Passes/PassBuilder.h
@@ -600,7 +600,7 @@
   /// is not triggered at O0. Extensions to the O0 pipeline should append their
   /// passes to the end of the overall pipeline.
   void registerOptimizerLastEPCallback(
-  const std::function ) {
+  const std::function ) {
 OptimizerLastEPCallbacks.push_back(C);
   }
 
@@ -728,7 +728,7 @@
   CGSCCOptimizerLateEPCallbacks;
   SmallVector, 2>
   VectorizerStartEPCallbacks;
-  SmallVector, 2>
+  SmallVector, 2>
   OptimizerLastEPCallbacks;
   // Module callbacks
   SmallVector, 2>
Index: clang/test/CodeGen/sancov-new-pm.c
===
--- clang/test/CodeGen/sancov-new-pm.c
+++ clang/test/CodeGen/sancov-new-pm.c
@@ -1,10 +1,6 @@
 // Test that SanitizerCoverage works under the new pass manager.
 // RUN: %clang -target x86_64-linux-gnu -fsanitize=fuzzer %s -fexperimental-new-pass-manager -S -emit-llvm -o - | FileCheck %s --check-prefixes=CHECK,CHECK-O0
 // RUN: %clang -target x86_64-linux-gnu -fsanitize=fuzzer %s -fexperimental-new-pass-manager -O2 -S -emit-llvm -o - | FileCheck %s --check-prefixes=CHECK,CHECK-O2
-// RUN: %clang -target x86_64-linux-gnu -fsanitize=fuzzer %s -fexperimental-new-pass-manager -flto -S -emit-llvm -o - | FileCheck %s --check-prefixes=CHECK,CHECK-O0
-// RUN: %clang -target x86_64-linux-gnu -fsanitize=fuzzer %s -fexperimental-new-pass-manager -flto -O2 -S -emit-llvm -o - | FileCheck %s --check-prefixes=CHECK,CHECK-O2
-// RUN: %clang -target x86_64-linux-gnu -fsanitize=fuzzer %s -fexperimental-new-pass-manager -flto=thin -S -emit-llvm -o - | FileCheck %s --check-prefixes=CHECK,CHECK-O0
-// RUN: %clang -target x86_64-linux-gnu -fsanitize=fuzzer %s -fexperimental-new-pass-manager -flto=thin -O2 -S -emit-llvm -o - | FileCheck %s --check-prefixes=CHECK,CHECK-O2,CHECK-O2-THINLTO
 
 extern void *memcpy(void *, const void *, unsigned long);
 extern int printf(const char *restrict, ...);
Index: clang/lib/CodeGen/BackendUtil.cpp
===
--- clang/lib/CodeGen/BackendUtil.cpp
+++ clang/lib/CodeGen/BackendUtil.cpp
@@ -32,6 +32,7 @@
 #include "llvm/IR/LegacyPassManager.h"
 #include "llvm/IR/Module.h"
 #include "llvm/IR/ModuleSummaryIndex.h"
+#include "llvm/IR/PassManager.h"
 #include "llvm/IR/Verifier.h"
 #include "llvm/LTO/LTOBackend.h"
 #include "llvm/MC/MCAsmInfo.h"
@@ -1001,6 +1002,15 @@
   const Triple ,
   const LangOptions ,
   const CodeGenOptions ) {
+  if (CodeGenOpts.SanitizeCoverageType ||
+  

[PATCH] D80692: Run Coverage pass before other *San passes under new pass manager, round 2

2020-05-28 Thread Arthur Eubanks via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rG922fa2fce38b: Run Coverage pass before other *San passes 
under new pass manager, round 2 (authored by aeubanks).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80692

Files:
  clang/lib/CodeGen/BackendUtil.cpp
  llvm/include/llvm/Passes/PassBuilder.h
  llvm/lib/Passes/PassBuilder.cpp
  llvm/tools/opt/NewPMDriver.cpp

Index: llvm/tools/opt/NewPMDriver.cpp
===
--- llvm/tools/opt/NewPMDriver.cpp
+++ llvm/tools/opt/NewPMDriver.cpp
@@ -194,7 +194,7 @@
 });
   if (tryParsePipelineText(PB, OptimizerLastEPPipeline))
 PB.registerOptimizerLastEPCallback(
-[, VerifyEachPass, DebugLogging](FunctionPassManager ,
+[, VerifyEachPass, DebugLogging](ModulePassManager ,
 PassBuilder::OptimizationLevel) {
   ExitOnError Err("Unable to parse OptimizerLastEP pipeline: ");
   Err(PB.parsePassPipeline(PM, OptimizerLastEPPipeline, VerifyEachPass,
Index: llvm/lib/Passes/PassBuilder.cpp
===
--- llvm/lib/Passes/PassBuilder.cpp
+++ llvm/lib/Passes/PassBuilder.cpp
@@ -1073,12 +1073,12 @@
   if (PTO.Coroutines)
 OptimizePM.addPass(CoroCleanupPass());
 
-  for (auto  : OptimizerLastEPCallbacks)
-C(OptimizePM, Level);
-
   // Add the core optimizing pipeline.
   MPM.addPass(createModuleToFunctionPassAdaptor(std::move(OptimizePM)));
 
+  for (auto  : OptimizerLastEPCallbacks)
+C(MPM, Level);
+
   if (PTO.CallGraphProfile)
 MPM.addPass(CGProfilePass());
 
Index: llvm/include/llvm/Passes/PassBuilder.h
===
--- llvm/include/llvm/Passes/PassBuilder.h
+++ llvm/include/llvm/Passes/PassBuilder.h
@@ -600,7 +600,7 @@
   /// is not triggered at O0. Extensions to the O0 pipeline should append their
   /// passes to the end of the overall pipeline.
   void registerOptimizerLastEPCallback(
-  const std::function ) {
+  const std::function ) {
 OptimizerLastEPCallbacks.push_back(C);
   }
 
@@ -728,7 +728,7 @@
   CGSCCOptimizerLateEPCallbacks;
   SmallVector, 2>
   VectorizerStartEPCallbacks;
-  SmallVector, 2>
+  SmallVector, 2>
   OptimizerLastEPCallbacks;
   // Module callbacks
   SmallVector, 2>
Index: clang/lib/CodeGen/BackendUtil.cpp
===
--- clang/lib/CodeGen/BackendUtil.cpp
+++ clang/lib/CodeGen/BackendUtil.cpp
@@ -32,6 +32,7 @@
 #include "llvm/IR/LegacyPassManager.h"
 #include "llvm/IR/Module.h"
 #include "llvm/IR/ModuleSummaryIndex.h"
+#include "llvm/IR/PassManager.h"
 #include "llvm/IR/Verifier.h"
 #include "llvm/LTO/LTOBackend.h"
 #include "llvm/MC/MCAsmInfo.h"
@@ -1001,6 +1002,15 @@
   const Triple ,
   const LangOptions ,
   const CodeGenOptions ) {
+  if (CodeGenOpts.SanitizeCoverageType ||
+  CodeGenOpts.SanitizeCoverageIndirectCalls ||
+  CodeGenOpts.SanitizeCoverageTraceCmp) {
+auto SancovOpts = getSancovOptsFromCGOpts(CodeGenOpts);
+MPM.addPass(ModuleSanitizerCoveragePass(
+SancovOpts, CodeGenOpts.SanitizeCoverageWhitelistFiles,
+CodeGenOpts.SanitizeCoverageBlacklistFiles));
+  }
+
   auto ASanPass = [&](SanitizerMask Mask, bool CompileKernel) {
 MPM.addPass(RequireAnalysisPass());
 bool Recover = CodeGenOpts.SanitizeRecover.has(Mask);
@@ -1249,6 +1259,20 @@
 [](FunctionPassManager , PassBuilder::OptimizationLevel Level) {
   FPM.addPass(BoundsCheckingPass());
 });
+
+  if (CodeGenOpts.SanitizeCoverageType ||
+  CodeGenOpts.SanitizeCoverageIndirectCalls ||
+  CodeGenOpts.SanitizeCoverageTraceCmp) {
+PB.registerOptimizerLastEPCallback(
+[this](ModulePassManager ,
+   PassBuilder::OptimizationLevel Level) {
+  auto SancovOpts = getSancovOptsFromCGOpts(CodeGenOpts);
+  MPM.addPass(ModuleSanitizerCoveragePass(
+  SancovOpts, CodeGenOpts.SanitizeCoverageWhitelistFiles,
+  CodeGenOpts.SanitizeCoverageBlacklistFiles));
+});
+  }
+
   if (LangOpts.Sanitize.has(SanitizerKind::Memory)) {
 int TrackOrigins = CodeGenOpts.SanitizeMemoryTrackOrigins;
 bool Recover = CodeGenOpts.SanitizeRecover.has(SanitizerKind::Memory);
@@ -1257,17 +1281,19 @@
   MPM.addPass(MemorySanitizerPass({TrackOrigins, Recover, false}));
 });
 PB.registerOptimizerLastEPCallback(
-[TrackOrigins, Recover](FunctionPassManager ,
+[TrackOrigins, Recover](ModulePassManager ,
 PassBuilder::OptimizationLevel Level) {
-  

[PATCH] D80692: Run Coverage pass before other *San passes under new pass manager, round 2

2020-05-28 Thread Arthur Eubanks via Phabricator via cfe-commits
aeubanks added a comment.

In D80692#2061219 , @leonardchan wrote:

> Would be worthwhile to write a small test that asserts Sancov runs before the 
> other sanitizers under the new PM?
>
> Aside from this, LGTM.


Asserting that sancov runs before other sanitizers seems like overspecification 
to me. Really all we want to know is that something like msan + sancov work 
together, and there are tests for that which we are fixing with this.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80692



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


[PATCH] D80692: Run Coverage pass before other *San passes under new pass manager, round 2

2020-05-28 Thread Leonard Chan via Phabricator via cfe-commits
leonardchan accepted this revision.
leonardchan added a comment.
This revision is now accepted and ready to land.

Would be worthwhile to write a small test that asserts Sancov runs before the 
other sanitizers under the new PM?

Aside from this, LGTM.




Comment at: llvm/include/llvm/Passes/PassBuilder.h:597-598
   ///
   /// This extension point allows adding optimizations at the very end of the
   /// function optimization pipeline. A key difference between this and the
   /// legacy PassManager's OptimizerLast callback is that this extension point

aeubanks wrote:
> leonardchan wrote:
> > Will need to change the wording on this if this doesn't handle function 
> > passes anymore.
> This doesn't say that `registerOptimizerLastEPCallback()` handles function 
> passes, it says it handles passes that will run right after all the default 
> function optimization passes. I believe that's still true. The passes were 
> previously added at the end of `OptimizePM`, now they're added right after 
> `OptimizePM`, which is the same thing.
Ah I see. You're right. I misread.



Comment at: llvm/lib/Passes/PassBuilder.cpp:1078
 
+  for (auto  : OptimizerLastEPCallbacks)
+C(MPM, Level);

aeubanks wrote:
> leonardchan wrote:
> > Would it be better to add another `SmallVector` and `register*Calback()` 
> > for modules in in `PassBuilder` that we could loop through here instead of 
> > changing how these extension points work?
> > 
> > I imagine it would still be meaningful in the future to be able to add 
> > function passes at the end of the function optimization pipeline.
> You can still add function passes via `createModuleToFunctionPassAdaptor()`, 
> which is what I've done here for the existing usages. Given that the number 
> of calls to `registerOptimizerLastEPCallback()` is small, I don't see a huge 
> benefit to create a both a vector of module passes and function passes that 
> run at the same place. If in the future we end up calling 
> `registerOptimizerLastEPCallback()` with lots of function passes we can 
> revisit this but for now it seems unnecessary.
> 
> This change doesn't change the ordering of any passes aside from sanitizer 
> coverage (I believe), all usages of `registerOptimizerLastEPCallback()` will 
> still end up putting the passes in the same place in the pipeline.
I see. Makes sense.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80692



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


[PATCH] D80692: Run Coverage pass before other *San passes under new pass manager, round 2

2020-05-28 Thread Arthur Eubanks via Phabricator via cfe-commits
aeubanks marked 2 inline comments as done.
aeubanks added inline comments.



Comment at: llvm/include/llvm/Passes/PassBuilder.h:597-598
   ///
   /// This extension point allows adding optimizations at the very end of the
   /// function optimization pipeline. A key difference between this and the
   /// legacy PassManager's OptimizerLast callback is that this extension point

leonardchan wrote:
> Will need to change the wording on this if this doesn't handle function 
> passes anymore.
This doesn't say that `registerOptimizerLastEPCallback()` handles function 
passes, it says it handles passes that will run right after all the default 
function optimization passes. I believe that's still true. The passes were 
previously added at the end of `OptimizePM`, now they're added right after 
`OptimizePM`, which is the same thing.



Comment at: llvm/lib/Passes/PassBuilder.cpp:1078
 
+  for (auto  : OptimizerLastEPCallbacks)
+C(MPM, Level);

leonardchan wrote:
> Would it be better to add another `SmallVector` and `register*Calback()` for 
> modules in in `PassBuilder` that we could loop through here instead of 
> changing how these extension points work?
> 
> I imagine it would still be meaningful in the future to be able to add 
> function passes at the end of the function optimization pipeline.
You can still add function passes via `createModuleToFunctionPassAdaptor()`, 
which is what I've done here for the existing usages. Given that the number of 
calls to `registerOptimizerLastEPCallback()` is small, I don't see a huge 
benefit to create a both a vector of module passes and function passes that run 
at the same place. If in the future we end up calling 
`registerOptimizerLastEPCallback()` with lots of function passes we can revisit 
this but for now it seems unnecessary.

This change doesn't change the ordering of any passes aside from sanitizer 
coverage (I believe), all usages of `registerOptimizerLastEPCallback()` will 
still end up putting the passes in the same place in the pipeline.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80692



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


[PATCH] D80692: Run Coverage pass before other *San passes under new pass manager, round 2

2020-05-28 Thread Leonard Chan via Phabricator via cfe-commits
leonardchan added inline comments.



Comment at: llvm/include/llvm/Passes/PassBuilder.h:597-598
   ///
   /// This extension point allows adding optimizations at the very end of the
   /// function optimization pipeline. A key difference between this and the
   /// legacy PassManager's OptimizerLast callback is that this extension point

Will need to change the wording on this if this doesn't handle function passes 
anymore.



Comment at: llvm/lib/Passes/PassBuilder.cpp:1078
 
+  for (auto  : OptimizerLastEPCallbacks)
+C(MPM, Level);

Would it be better to add another `SmallVector` and `register*Calback()` for 
modules in in `PassBuilder` that we could loop through here instead of changing 
how these extension points work?

I imagine it would still be meaningful in the future to be able to add function 
passes at the end of the function optimization pipeline.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80692



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


[PATCH] D80692: Run Coverage pass before other *San passes under new pass manager, round 2

2020-05-28 Thread Arthur Eubanks via Phabricator via cfe-commits
aeubanks created this revision.
Herald added subscribers: llvm-commits, cfe-commits, hiraditya.
Herald added projects: clang, LLVM.
aeubanks added reviewers: vitalybuka, leonardchan.

This was attempted once before in https://reviews.llvm.org/D79698, but
was reverted due to the coverage pass running in the wrong part of the
pipeline. This commit puts it in the same place as the other sanitizers.

This changes PassBuilder.OptimizerLastEPCallbacks to work on a
ModulePassManager instead of a FunctionPassManager. That is because
SanitizerCoverage cannot (easily) be split into a module pass and a
function pass like some of the other sanitizers since in its current
implementation it conditionally inserts module constructors based on
whether or not it successfully modified functions.

This fixes compiler-rt/test/msan/coverage-levels.cpp under the new pass
manager (last check-msan test).


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D80692

Files:
  clang/lib/CodeGen/BackendUtil.cpp
  llvm/include/llvm/Passes/PassBuilder.h
  llvm/lib/Passes/PassBuilder.cpp
  llvm/tools/opt/NewPMDriver.cpp

Index: llvm/tools/opt/NewPMDriver.cpp
===
--- llvm/tools/opt/NewPMDriver.cpp
+++ llvm/tools/opt/NewPMDriver.cpp
@@ -194,7 +194,7 @@
 });
   if (tryParsePipelineText(PB, OptimizerLastEPPipeline))
 PB.registerOptimizerLastEPCallback(
-[, VerifyEachPass, DebugLogging](FunctionPassManager ,
+[, VerifyEachPass, DebugLogging](ModulePassManager ,
 PassBuilder::OptimizationLevel) {
   ExitOnError Err("Unable to parse OptimizerLastEP pipeline: ");
   Err(PB.parsePassPipeline(PM, OptimizerLastEPPipeline, VerifyEachPass,
Index: llvm/lib/Passes/PassBuilder.cpp
===
--- llvm/lib/Passes/PassBuilder.cpp
+++ llvm/lib/Passes/PassBuilder.cpp
@@ -1072,12 +1072,12 @@
   if (PTO.Coroutines)
 OptimizePM.addPass(CoroCleanupPass());
 
-  for (auto  : OptimizerLastEPCallbacks)
-C(OptimizePM, Level);
-
   // Add the core optimizing pipeline.
   MPM.addPass(createModuleToFunctionPassAdaptor(std::move(OptimizePM)));
 
+  for (auto  : OptimizerLastEPCallbacks)
+C(MPM, Level);
+
   if (PTO.CallGraphProfile)
 MPM.addPass(CGProfilePass());
 
Index: llvm/include/llvm/Passes/PassBuilder.h
===
--- llvm/include/llvm/Passes/PassBuilder.h
+++ llvm/include/llvm/Passes/PassBuilder.h
@@ -600,7 +600,7 @@
   /// is not triggered at O0. Extensions to the O0 pipeline should append their
   /// passes to the end of the overall pipeline.
   void registerOptimizerLastEPCallback(
-  const std::function ) {
+  const std::function ) {
 OptimizerLastEPCallbacks.push_back(C);
   }
 
@@ -728,7 +728,7 @@
   CGSCCOptimizerLateEPCallbacks;
   SmallVector, 2>
   VectorizerStartEPCallbacks;
-  SmallVector, 2>
+  SmallVector, 2>
   OptimizerLastEPCallbacks;
   // Module callbacks
   SmallVector, 2>
Index: clang/lib/CodeGen/BackendUtil.cpp
===
--- clang/lib/CodeGen/BackendUtil.cpp
+++ clang/lib/CodeGen/BackendUtil.cpp
@@ -32,6 +32,7 @@
 #include "llvm/IR/LegacyPassManager.h"
 #include "llvm/IR/Module.h"
 #include "llvm/IR/ModuleSummaryIndex.h"
+#include "llvm/IR/PassManager.h"
 #include "llvm/IR/Verifier.h"
 #include "llvm/LTO/LTOBackend.h"
 #include "llvm/MC/MCAsmInfo.h"
@@ -1001,6 +1002,15 @@
   const Triple ,
   const LangOptions ,
   const CodeGenOptions ) {
+  if (CodeGenOpts.SanitizeCoverageType ||
+  CodeGenOpts.SanitizeCoverageIndirectCalls ||
+  CodeGenOpts.SanitizeCoverageTraceCmp) {
+auto SancovOpts = getSancovOptsFromCGOpts(CodeGenOpts);
+MPM.addPass(ModuleSanitizerCoveragePass(
+SancovOpts, CodeGenOpts.SanitizeCoverageWhitelistFiles,
+CodeGenOpts.SanitizeCoverageBlacklistFiles));
+  }
+
   auto ASanPass = [&](SanitizerMask Mask, bool CompileKernel) {
 MPM.addPass(RequireAnalysisPass());
 bool Recover = CodeGenOpts.SanitizeRecover.has(Mask);
@@ -1249,6 +1259,20 @@
 [](FunctionPassManager , PassBuilder::OptimizationLevel Level) {
   FPM.addPass(BoundsCheckingPass());
 });
+
+  if (CodeGenOpts.SanitizeCoverageType ||
+  CodeGenOpts.SanitizeCoverageIndirectCalls ||
+  CodeGenOpts.SanitizeCoverageTraceCmp) {
+PB.registerOptimizerLastEPCallback(
+[this](ModulePassManager ,
+   PassBuilder::OptimizationLevel Level) {
+  auto SancovOpts = getSancovOptsFromCGOpts(CodeGenOpts);
+  MPM.addPass(ModuleSanitizerCoveragePass(
+  SancovOpts, CodeGenOpts.SanitizeCoverageWhitelistFiles,
+