[PATCH] D91327: [NewPM] Redesign of PreserveCFG Checker

[Yevgeny Rouban via Phabricator via cfe-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG39e3e3aa51d6: [NewPM] Redesign of PreserveCFG Checker 
(authored by yrouban).

Changed prior to commit:
  https://reviews.llvm.org/D91327?vs=335221=335405#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91327

Files:
  clang/lib/CodeGen/BackendUtil.cpp
  llvm/include/llvm/Passes/StandardInstrumentations.h
  llvm/lib/LTO/LTOBackend.cpp
  llvm/lib/Passes/StandardInstrumentations.cpp
  llvm/test/Other/new-pass-manager.ll
  llvm/test/Other/new-pm-defaults.ll
  llvm/tools/opt/NewPMDriver.cpp
  llvm/unittests/IR/PassManagerTest.cpp

Index: llvm/unittests/IR/PassManagerTest.cpp
===
--- llvm/unittests/IR/PassManagerTest.cpp
+++ llvm/unittests/IR/PassManagerTest.cpp
@@ -827,7 +827,7 @@
   FunctionPassManager FPM(/*DebugLogging*/ true);
   PassInstrumentationCallbacks PIC;
   StandardInstrumentations SI(/*DebugLogging*/ true);
-  SI.registerCallbacks(PIC);
+  SI.registerCallbacks(PIC, );
   FAM.registerPass([&] { return PassInstrumentationAnalysis(); });
   FAM.registerPass([&] { return DominatorTreeAnalysis(); });
   FAM.registerPass([&] { return AssumptionAnalysis(); });
@@ -873,7 +873,7 @@
   FunctionPassManager FPM(/*DebugLogging*/ true);
   PassInstrumentationCallbacks PIC;
   StandardInstrumentations SI(/*DebugLogging*/ true);
-  SI.registerCallbacks(PIC);
+  SI.registerCallbacks(PIC, );
   FAM.registerPass([&] { return PassInstrumentationAnalysis(); });
   FAM.registerPass([&] { return DominatorTreeAnalysis(); });
   FAM.registerPass([&] { return AssumptionAnalysis(); });
@@ -938,7 +938,7 @@
   FunctionPassManager FPM(/*DebugLogging*/ true);
   PassInstrumentationCallbacks PIC;
   StandardInstrumentations SI(/*DebugLogging*/ true);
-  SI.registerCallbacks(PIC);
+  SI.registerCallbacks(PIC, );
   FAM.registerPass([&] { return PassInstrumentationAnalysis(); });
   FAM.registerPass([&] { return DominatorTreeAnalysis(); });
   FAM.registerPass([&] { return AssumptionAnalysis(); });
Index: llvm/tools/opt/NewPMDriver.cpp
===
--- llvm/tools/opt/NewPMDriver.cpp
+++ llvm/tools/opt/NewPMDriver.cpp
@@ -275,9 +275,14 @@
   P->CSAction = PGOOptions::CSIRUse;
 }
   }
+  LoopAnalysisManager LAM(DebugPM);
+  FunctionAnalysisManager FAM(DebugPM);
+  CGSCCAnalysisManager CGAM(DebugPM);
+  ModuleAnalysisManager MAM(DebugPM);
+
   PassInstrumentationCallbacks PIC;
   StandardInstrumentations SI(DebugPM, VerifyEachPass);
-  SI.registerCallbacks(PIC);
+  SI.registerCallbacks(PIC, );
   DebugifyEachInstrumentation Debugify;
   if (DebugifyEach)
 Debugify.registerCallbacks(PIC);
@@ -373,11 +378,6 @@
 }
   }
 
-  LoopAnalysisManager LAM(DebugPM);
-  FunctionAnalysisManager FAM(DebugPM);
-  CGSCCAnalysisManager CGAM(DebugPM);
-  ModuleAnalysisManager MAM(DebugPM);
-
   // Register the AA manager first so that our version is the one used.
   FAM.registerPass([&] { return std::move(AA); });
   // Register our TargetLibraryInfoImpl.
Index: llvm/test/Other/new-pm-defaults.ll
===
--- llvm/test/Other/new-pm-defaults.ll
+++ llvm/test/Other/new-pm-defaults.ll
@@ -7,62 +7,62 @@
 ; Any invalidation that shows up here is a bug, unless we started modifying
 ; the IR, in which case we need to make it immutable harder.
 
-; RUN: opt -disable-verify -debug-pass-manager \
+; RUN: opt -disable-verify -verify-cfg-preserved=1 -debug-pass-manager \
 ; RUN: -passes='default' -S %s 2>&1 \
 ; RUN: | FileCheck %s --check-prefixes=CHECK-O,CHECK-DEFAULT,CHECK-O1,%llvmcheckext
-; RUN: opt -disable-verify -debug-pass-manager \
+; RUN: opt -disable-verify -verify-cfg-preserved=1 -debug-pass-manager \
 ; RUN: -passes='default' -S  %s 2>&1 \
 ; RUN: | FileCheck %s --check-prefixes=CHECK-O,CHECK-DEFAULT,CHECK-O2,CHECK-O23SZ,%llvmcheckext
-; RUN: opt -disable-verify -debug-pass-manager \
+; RUN: opt -disable-verify -verify-cfg-preserved=1 -debug-pass-manager \
 ; RUN: -passes='default' -S  %s 2>&1 \
 ; RUN: | FileCheck %s --check-prefixes=CHECK-O,CHECK-DEFAULT,CHECK-O3,CHECK-O23SZ,%llvmcheckext
-; RUN: opt -disable-verify -debug-pass-manager \
+; RUN: opt -disable-verify -verify-cfg-preserved=1 -debug-pass-manager \
 ; RUN: -passes='default' -S %s 2>&1 \
 ; RUN: | FileCheck %s --check-prefixes=CHECK-O,CHECK-DEFAULT,CHECK-Os,CHECK-O23SZ,%llvmcheckext
-; RUN: opt -disable-verify -debug-pass-manager \
+; RUN: opt -disable-verify -verify-cfg-preserved=1 -debug-pass-manager \
 ; RUN: -passes='default' -S %s 2>&1 \
 ; RUN: | FileCheck %s --check-prefixes=CHECK-O,CHECK-DEFAULT,CHECK-Oz,CHECK-O23SZ,%llvmcheckext
-; RUN: opt -disable-verify -debug-pass-manager \
+; RUN: opt -disable-verify 

[PATCH] D91327: [NewPM] Redesign of PreserveCFG Checker

[Serguei Katkov via Phabricator via cfe-commits]

skatkov accepted this revision.
skatkov added a comment.
This revision is now accepted and ready to land.

with two nits.




Comment at: llvm/include/llvm/Passes/StandardInstrumentations.h:422
+  // Register all the standard instrumentation callbacks. If \p FAM is nullptr
+  // then PreservedCFGChecker is not registeredenabled.
+  void registerCallbacks(PassInstrumentationCallbacks ,

yrouban wrote:
> fix registeredenabled
Not Done.



Comment at: llvm/lib/Passes/StandardInstrumentations.cpp:1079
+const auto *F = any_cast(IR);
+FAM.getResult(*const_cast(F));
+  });

Add a comment that you caches the CFG before pass.


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

https://reviews.llvm.org/D91327

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

[PATCH] D91327: [NewPM] Redesign of PreserveCFG Checker

[Yevgeny Rouban via Phabricator via cfe-commits]

yrouban updated this revision to Diff 335221.
yrouban marked 2 inline comments as done.
yrouban added a comment.

Addressed comments.
Some tests were moved a separate patch (D99878 
).


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

https://reviews.llvm.org/D91327

Files:
  clang/lib/CodeGen/BackendUtil.cpp
  llvm/include/llvm/Passes/StandardInstrumentations.h
  llvm/lib/LTO/LTOBackend.cpp
  llvm/lib/Passes/StandardInstrumentations.cpp
  llvm/test/Other/new-pass-manager.ll
  llvm/test/Other/new-pm-defaults.ll
  llvm/tools/opt/NewPMDriver.cpp
  llvm/unittests/IR/PassManagerTest.cpp

Index: llvm/unittests/IR/PassManagerTest.cpp
===
--- llvm/unittests/IR/PassManagerTest.cpp
+++ llvm/unittests/IR/PassManagerTest.cpp
@@ -827,7 +827,7 @@
   FunctionPassManager FPM(/*DebugLogging*/ true);
   PassInstrumentationCallbacks PIC;
   StandardInstrumentations SI(/*DebugLogging*/ true);
-  SI.registerCallbacks(PIC);
+  SI.registerCallbacks(PIC, );
   FAM.registerPass([&] { return PassInstrumentationAnalysis(); });
   FAM.registerPass([&] { return DominatorTreeAnalysis(); });
   FAM.registerPass([&] { return AssumptionAnalysis(); });
@@ -873,7 +873,7 @@
   FunctionPassManager FPM(/*DebugLogging*/ true);
   PassInstrumentationCallbacks PIC;
   StandardInstrumentations SI(/*DebugLogging*/ true);
-  SI.registerCallbacks(PIC);
+  SI.registerCallbacks(PIC, );
   FAM.registerPass([&] { return PassInstrumentationAnalysis(); });
   FAM.registerPass([&] { return DominatorTreeAnalysis(); });
   FAM.registerPass([&] { return AssumptionAnalysis(); });
@@ -938,7 +938,7 @@
   FunctionPassManager FPM(/*DebugLogging*/ true);
   PassInstrumentationCallbacks PIC;
   StandardInstrumentations SI(/*DebugLogging*/ true);
-  SI.registerCallbacks(PIC);
+  SI.registerCallbacks(PIC, );
   FAM.registerPass([&] { return PassInstrumentationAnalysis(); });
   FAM.registerPass([&] { return DominatorTreeAnalysis(); });
   FAM.registerPass([&] { return AssumptionAnalysis(); });
Index: llvm/tools/opt/NewPMDriver.cpp
===
--- llvm/tools/opt/NewPMDriver.cpp
+++ llvm/tools/opt/NewPMDriver.cpp
@@ -275,9 +275,14 @@
   P->CSAction = PGOOptions::CSIRUse;
 }
   }
+  LoopAnalysisManager LAM(DebugPM);
+  FunctionAnalysisManager FAM(DebugPM);
+  CGSCCAnalysisManager CGAM(DebugPM);
+  ModuleAnalysisManager MAM(DebugPM);
+
   PassInstrumentationCallbacks PIC;
   StandardInstrumentations SI(DebugPM, VerifyEachPass);
-  SI.registerCallbacks(PIC);
+  SI.registerCallbacks(PIC, );
   DebugifyEachInstrumentation Debugify;
   if (DebugifyEach)
 Debugify.registerCallbacks(PIC);
@@ -373,11 +378,6 @@
 }
   }
 
-  LoopAnalysisManager LAM(DebugPM);
-  FunctionAnalysisManager FAM(DebugPM);
-  CGSCCAnalysisManager CGAM(DebugPM);
-  ModuleAnalysisManager MAM(DebugPM);
-
   // Register the AA manager first so that our version is the one used.
   FAM.registerPass([&] { return std::move(AA); });
   // Register our TargetLibraryInfoImpl.
Index: llvm/test/Other/new-pm-defaults.ll
===
--- llvm/test/Other/new-pm-defaults.ll
+++ llvm/test/Other/new-pm-defaults.ll
@@ -7,62 +7,62 @@
 ; Any invalidation that shows up here is a bug, unless we started modifying
 ; the IR, in which case we need to make it immutable harder.
 
-; RUN: opt -disable-verify -debug-pass-manager \
+; RUN: opt -disable-verify -verify-cfg-preserved=1 -debug-pass-manager \
 ; RUN: -passes='default' -S %s 2>&1 \
 ; RUN: | FileCheck %s --check-prefixes=CHECK-O,CHECK-DEFAULT,CHECK-O1,%llvmcheckext
-; RUN: opt -disable-verify -debug-pass-manager \
+; RUN: opt -disable-verify -verify-cfg-preserved=1 -debug-pass-manager \
 ; RUN: -passes='default' -S  %s 2>&1 \
 ; RUN: | FileCheck %s --check-prefixes=CHECK-O,CHECK-DEFAULT,CHECK-O2,CHECK-O23SZ,%llvmcheckext
-; RUN: opt -disable-verify -debug-pass-manager \
+; RUN: opt -disable-verify -verify-cfg-preserved=1 -debug-pass-manager \
 ; RUN: -passes='default' -S  %s 2>&1 \
 ; RUN: | FileCheck %s --check-prefixes=CHECK-O,CHECK-DEFAULT,CHECK-O3,CHECK-O23SZ,%llvmcheckext
-; RUN: opt -disable-verify -debug-pass-manager \
+; RUN: opt -disable-verify -verify-cfg-preserved=1 -debug-pass-manager \
 ; RUN: -passes='default' -S %s 2>&1 \
 ; RUN: | FileCheck %s --check-prefixes=CHECK-O,CHECK-DEFAULT,CHECK-Os,CHECK-O23SZ,%llvmcheckext
-; RUN: opt -disable-verify -debug-pass-manager \
+; RUN: opt -disable-verify -verify-cfg-preserved=1 -debug-pass-manager \
 ; RUN: -passes='default' -S %s 2>&1 \
 ; RUN: | FileCheck %s --check-prefixes=CHECK-O,CHECK-DEFAULT,CHECK-Oz,CHECK-O23SZ,%llvmcheckext
-; RUN: opt -disable-verify -debug-pass-manager \
+; RUN: opt -disable-verify -verify-cfg-preserved=1 -debug-pass-manager \
 ; RUN: -passes='lto-pre-link' -S %s 2>&1 \
 ; RUN: | FileCheck %s 

[PATCH] D91327: [NewPM] Redesign of PreserveCFG Checker

[Serguei Katkov via Phabricator via cfe-commits]

skatkov added a comment.

Consider landing tests update for "-verify-cfg-preserved=0" in a separate 
commit. This will significantly reduce the size of review.


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

https://reviews.llvm.org/D91327

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

[PATCH] D91327: [NewPM] Redesign of PreserveCFG Checker

[Serguei Katkov via Phabricator via cfe-commits]

skatkov added a comment.

First iteration: style comments.




Comment at: llvm/include/llvm/Passes/StandardInstrumentations.h:124
   if (BBGuards)
 for (auto  : *BBGuards) {
   if (BB.second.isPoisoned())

redundant {}
consider using any_of



Comment at: llvm/lib/Passes/StandardInstrumentations.cpp:1017
 
+struct PreservedCFGCheckerAnalysis
+: public AnalysisInfoMixin {

Describe the idea of this analisys


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

https://reviews.llvm.org/D91327

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

[PATCH] D91327: [NewPM] Redesign of PreserveCFG Checker

[Yevgeny Rouban via Phabricator via cfe-commits]

yrouban added a comment.

@skatkov, @fedor.sergeev, could you please review. During the long time this 
features is on the reconstruction we introduced a new bug in the LoopFlatten 
pass. See the comment https://reviews.llvm.org/D90940#inline-938253.


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

https://reviews.llvm.org/D91327

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

[PATCH] D91327: [NewPM] Redesign of PreserveCFG Checker

[Yevgeny Rouban via Phabricator via cfe-commits]

yrouban added inline comments.



Comment at: llvm/include/llvm/Passes/StandardInstrumentations.h:422
+  // Register all the standard instrumentation callbacks. If \p FAM is nullptr
+  // then PreservedCFGChecker is not registeredenabled.
+  void registerCallbacks(PassInstrumentationCallbacks ,

fix registeredenabled


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

https://reviews.llvm.org/D91327

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

[PATCH] D91327: [NewPM] Redesign of PreserveCFG Checker

[Yevgeny Rouban via Phabricator via cfe-commits]

yrouban updated this revision to Diff 334042.
yrouban added a comment.

fixed the nits


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

https://reviews.llvm.org/D91327

Files:
  clang/lib/CodeGen/BackendUtil.cpp
  llvm/include/llvm/Passes/StandardInstrumentations.h
  llvm/lib/LTO/LTOBackend.cpp
  llvm/lib/Passes/StandardInstrumentations.cpp
  llvm/test/Other/loop-pm-invalidation.ll
  llvm/test/Other/new-pass-manager.ll
  llvm/test/Other/new-pm-O0-defaults.ll
  llvm/test/Other/new-pm-defaults.ll
  llvm/test/Other/new-pm-lto-defaults.ll
  llvm/test/Other/new-pm-thinlto-defaults.ll
  llvm/test/Other/new-pm-thinlto-postlink-pgo-defaults.ll
  llvm/test/Other/new-pm-thinlto-postlink-samplepgo-defaults.ll
  llvm/test/Other/new-pm-thinlto-prelink-pgo-defaults.ll
  llvm/test/Other/new-pm-thinlto-prelink-samplepgo-defaults.ll
  llvm/test/Transforms/LoopRotate/pr35210.ll
  llvm/test/Transforms/SCCP/ipsccp-preserve-analysis.ll
  llvm/tools/opt/NewPMDriver.cpp
  llvm/unittests/IR/PassManagerTest.cpp

Index: llvm/unittests/IR/PassManagerTest.cpp
===
--- llvm/unittests/IR/PassManagerTest.cpp
+++ llvm/unittests/IR/PassManagerTest.cpp
@@ -827,7 +827,7 @@
   FunctionPassManager FPM(/*DebugLogging*/ true);
   PassInstrumentationCallbacks PIC;
   StandardInstrumentations SI(/*DebugLogging*/ true);
-  SI.registerCallbacks(PIC);
+  SI.registerCallbacks(PIC, );
   FAM.registerPass([&] { return PassInstrumentationAnalysis(); });
   FAM.registerPass([&] { return DominatorTreeAnalysis(); });
   FAM.registerPass([&] { return AssumptionAnalysis(); });
@@ -873,7 +873,7 @@
   FunctionPassManager FPM(/*DebugLogging*/ true);
   PassInstrumentationCallbacks PIC;
   StandardInstrumentations SI(/*DebugLogging*/ true);
-  SI.registerCallbacks(PIC);
+  SI.registerCallbacks(PIC, );
   FAM.registerPass([&] { return PassInstrumentationAnalysis(); });
   FAM.registerPass([&] { return DominatorTreeAnalysis(); });
   FAM.registerPass([&] { return AssumptionAnalysis(); });
@@ -938,7 +938,7 @@
   FunctionPassManager FPM(/*DebugLogging*/ true);
   PassInstrumentationCallbacks PIC;
   StandardInstrumentations SI(/*DebugLogging*/ true);
-  SI.registerCallbacks(PIC);
+  SI.registerCallbacks(PIC, );
   FAM.registerPass([&] { return PassInstrumentationAnalysis(); });
   FAM.registerPass([&] { return DominatorTreeAnalysis(); });
   FAM.registerPass([&] { return AssumptionAnalysis(); });
Index: llvm/tools/opt/NewPMDriver.cpp
===
--- llvm/tools/opt/NewPMDriver.cpp
+++ llvm/tools/opt/NewPMDriver.cpp
@@ -275,9 +275,14 @@
   P->CSAction = PGOOptions::CSIRUse;
 }
   }
+  LoopAnalysisManager LAM(DebugPM);
+  FunctionAnalysisManager FAM(DebugPM);
+  CGSCCAnalysisManager CGAM(DebugPM);
+  ModuleAnalysisManager MAM(DebugPM);
+
   PassInstrumentationCallbacks PIC;
   StandardInstrumentations SI(DebugPM, VerifyEachPass);
-  SI.registerCallbacks(PIC);
+  SI.registerCallbacks(PIC, );
   DebugifyEachInstrumentation Debugify;
   if (DebugifyEach)
 Debugify.registerCallbacks(PIC);
@@ -373,11 +378,6 @@
 }
   }
 
-  LoopAnalysisManager LAM(DebugPM);
-  FunctionAnalysisManager FAM(DebugPM);
-  CGSCCAnalysisManager CGAM(DebugPM);
-  ModuleAnalysisManager MAM(DebugPM);
-
   // Register the AA manager first so that our version is the one used.
   FAM.registerPass([&] { return std::move(AA); });
   // Register our TargetLibraryInfoImpl.
Index: llvm/test/Transforms/SCCP/ipsccp-preserve-analysis.ll
===
--- llvm/test/Transforms/SCCP/ipsccp-preserve-analysis.ll
+++ llvm/test/Transforms/SCCP/ipsccp-preserve-analysis.ll
@@ -16,7 +16,7 @@
 ; NEW-PM: Running pass: IPSCCPPass
 ; NEW-PM-DAG: Running analysis: AssumptionAnalysis on f1
 ; NEW-PM-DAG: Running analysis: AssumptionAnalysis on f2
-; NEW-PM-NOT: Running analysis:
+; NEW-PM-NOT: Running analysis: AssumptionAnalysis
 
 ; IR-LABEL: @f1
 ; IR-LABEL: entry:
Index: llvm/test/Transforms/LoopRotate/pr35210.ll
===
--- llvm/test/Transforms/LoopRotate/pr35210.ll
+++ llvm/test/Transforms/LoopRotate/pr35210.ll
@@ -1,5 +1,5 @@
-;RUN: opt %s -aa-pipeline= -passes='adce,loop(loop-rotate),adce' -S -debug-pass-manager -debug-only=loop-rotate 2>&1 | FileCheck %s
-;RUN: opt %s -aa-pipeline= -passes='adce,loop-mssa(loop-rotate),adce' -S -debug-pass-manager -debug-only=loop-rotate -verify-memoryssa 2>&1 | FileCheck %s --check-prefix=MSSA
+;RUN: opt %s -aa-pipeline= -passes='adce,loop(loop-rotate),adce' -S -verify-cfg-preserved=0 -debug-pass-manager -debug-only=loop-rotate 2>&1 | FileCheck %s
+;RUN: opt %s -aa-pipeline= -passes='adce,loop-mssa(loop-rotate),adce' -S -verify-cfg-preserved=0 -debug-pass-manager -debug-only=loop-rotate -verify-memoryssa 2>&1 | FileCheck %s --check-prefix=MSSA
 ;REQUIRES: asserts
 
 ; This test is to make sure we invalidate the post 

[PATCH] D91327: [NewPM] Redesign of PreserveCFG Checker

[Yevgeny Rouban via Phabricator via cfe-commits]

yrouban marked 2 inline comments as done.
yrouban added a comment.

In D91327#2655786 , @kuhar wrote:

> Just two nits from me. I think it looks fine, but I'm not familiar with the 
> new pass manager and don't feel confident enough to approve it.

Jakub, thank you for comments. I expect @skatkov or @fedor.sergeev to look from 
NewPM point of view.


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

https://reviews.llvm.org/D91327

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

[PATCH] D91327: [NewPM] Redesign of PreserveCFG Checker

[Jakub Kuderski via Phabricator via cfe-commits]

kuhar added a comment.

Just two nits from me. I think it looks fine, but I'm not familiar with the new 
pass manager and don't feel confident enough to approve it.




Comment at: llvm/include/llvm/Passes/StandardInstrumentations.h:134
 
 public:
   static cl::opt VerifyPreservedCFG;

yrouban wrote:
> kuhar wrote:
> > yrouban wrote:
> > > kuhar wrote:
> > > > not necessary anymore
> > > there can bee a need to disabled/enable (e.g. for some tests or for 
> > > debugging).
> > I meant the 'public:'. You made everything public at the very top of the 
> > class.
> sure
I don't think this got fixed.



Comment at: llvm/lib/Passes/StandardInstrumentations.cpp:1096
+  checkCFG(P, F->getName(), *GraphBefore,
+   CFG(F, false /* TrackBBLifetime */));
   });

nit: can you put the commented argument name before the argument?


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

https://reviews.llvm.org/D91327

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

[PATCH] D91327: [NewPM] Redesign of PreserveCFG Checker

[Yevgeny Rouban via Phabricator via cfe-commits]

yrouban updated this revision to Diff 333825.
yrouban added a comment.

just rebased. please review


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

https://reviews.llvm.org/D91327

Files:
  clang/lib/CodeGen/BackendUtil.cpp
  llvm/include/llvm/Passes/StandardInstrumentations.h
  llvm/lib/LTO/LTOBackend.cpp
  llvm/lib/Passes/StandardInstrumentations.cpp
  llvm/test/Other/loop-pm-invalidation.ll
  llvm/test/Other/new-pass-manager.ll
  llvm/test/Other/new-pm-O0-defaults.ll
  llvm/test/Other/new-pm-defaults.ll
  llvm/test/Other/new-pm-lto-defaults.ll
  llvm/test/Other/new-pm-thinlto-defaults.ll
  llvm/test/Other/new-pm-thinlto-postlink-pgo-defaults.ll
  llvm/test/Other/new-pm-thinlto-postlink-samplepgo-defaults.ll
  llvm/test/Other/new-pm-thinlto-prelink-pgo-defaults.ll
  llvm/test/Other/new-pm-thinlto-prelink-samplepgo-defaults.ll
  llvm/test/Transforms/LoopRotate/pr35210.ll
  llvm/test/Transforms/SCCP/ipsccp-preserve-analysis.ll
  llvm/tools/opt/NewPMDriver.cpp
  llvm/unittests/IR/PassManagerTest.cpp

Index: llvm/unittests/IR/PassManagerTest.cpp
===
--- llvm/unittests/IR/PassManagerTest.cpp
+++ llvm/unittests/IR/PassManagerTest.cpp
@@ -827,7 +827,7 @@
   FunctionPassManager FPM(/*DebugLogging*/ true);
   PassInstrumentationCallbacks PIC;
   StandardInstrumentations SI(/*DebugLogging*/ true);
-  SI.registerCallbacks(PIC);
+  SI.registerCallbacks(PIC, );
   FAM.registerPass([&] { return PassInstrumentationAnalysis(); });
   FAM.registerPass([&] { return DominatorTreeAnalysis(); });
   FAM.registerPass([&] { return AssumptionAnalysis(); });
@@ -873,7 +873,7 @@
   FunctionPassManager FPM(/*DebugLogging*/ true);
   PassInstrumentationCallbacks PIC;
   StandardInstrumentations SI(/*DebugLogging*/ true);
-  SI.registerCallbacks(PIC);
+  SI.registerCallbacks(PIC, );
   FAM.registerPass([&] { return PassInstrumentationAnalysis(); });
   FAM.registerPass([&] { return DominatorTreeAnalysis(); });
   FAM.registerPass([&] { return AssumptionAnalysis(); });
@@ -938,7 +938,7 @@
   FunctionPassManager FPM(/*DebugLogging*/ true);
   PassInstrumentationCallbacks PIC;
   StandardInstrumentations SI(/*DebugLogging*/ true);
-  SI.registerCallbacks(PIC);
+  SI.registerCallbacks(PIC, );
   FAM.registerPass([&] { return PassInstrumentationAnalysis(); });
   FAM.registerPass([&] { return DominatorTreeAnalysis(); });
   FAM.registerPass([&] { return AssumptionAnalysis(); });
Index: llvm/tools/opt/NewPMDriver.cpp
===
--- llvm/tools/opt/NewPMDriver.cpp
+++ llvm/tools/opt/NewPMDriver.cpp
@@ -275,9 +275,14 @@
   P->CSAction = PGOOptions::CSIRUse;
 }
   }
+  LoopAnalysisManager LAM(DebugPM);
+  FunctionAnalysisManager FAM(DebugPM);
+  CGSCCAnalysisManager CGAM(DebugPM);
+  ModuleAnalysisManager MAM(DebugPM);
+
   PassInstrumentationCallbacks PIC;
   StandardInstrumentations SI(DebugPM, VerifyEachPass);
-  SI.registerCallbacks(PIC);
+  SI.registerCallbacks(PIC, );
   DebugifyEachInstrumentation Debugify;
   if (DebugifyEach)
 Debugify.registerCallbacks(PIC);
@@ -373,11 +378,6 @@
 }
   }
 
-  LoopAnalysisManager LAM(DebugPM);
-  FunctionAnalysisManager FAM(DebugPM);
-  CGSCCAnalysisManager CGAM(DebugPM);
-  ModuleAnalysisManager MAM(DebugPM);
-
   // Register the AA manager first so that our version is the one used.
   FAM.registerPass([&] { return std::move(AA); });
   // Register our TargetLibraryInfoImpl.
Index: llvm/test/Transforms/SCCP/ipsccp-preserve-analysis.ll
===
--- llvm/test/Transforms/SCCP/ipsccp-preserve-analysis.ll
+++ llvm/test/Transforms/SCCP/ipsccp-preserve-analysis.ll
@@ -16,7 +16,7 @@
 ; NEW-PM: Running pass: IPSCCPPass
 ; NEW-PM-DAG: Running analysis: AssumptionAnalysis on f1
 ; NEW-PM-DAG: Running analysis: AssumptionAnalysis on f2
-; NEW-PM-NOT: Running analysis:
+; NEW-PM-NOT: Running analysis: AssumptionAnalysis
 
 ; IR-LABEL: @f1
 ; IR-LABEL: entry:
Index: llvm/test/Transforms/LoopRotate/pr35210.ll
===
--- llvm/test/Transforms/LoopRotate/pr35210.ll
+++ llvm/test/Transforms/LoopRotate/pr35210.ll
@@ -1,5 +1,5 @@
-;RUN: opt %s -aa-pipeline= -passes='adce,loop(loop-rotate),adce' -S -debug-pass-manager -debug-only=loop-rotate 2>&1 | FileCheck %s
-;RUN: opt %s -aa-pipeline= -passes='adce,loop-mssa(loop-rotate),adce' -S -debug-pass-manager -debug-only=loop-rotate -verify-memoryssa 2>&1 | FileCheck %s --check-prefix=MSSA
+;RUN: opt %s -aa-pipeline= -passes='adce,loop(loop-rotate),adce' -S -verify-cfg-preserved=0 -debug-pass-manager -debug-only=loop-rotate 2>&1 | FileCheck %s
+;RUN: opt %s -aa-pipeline= -passes='adce,loop-mssa(loop-rotate),adce' -S -verify-cfg-preserved=0 -debug-pass-manager -debug-only=loop-rotate -verify-memoryssa 2>&1 | FileCheck %s --check-prefix=MSSA
 ;REQUIRES: asserts
 
 ; This test is to make sure we invalidate 

[PATCH] D91327: [NewPM] Redesign of PreserveCFG Checker

[Yevgeny Rouban via Phabricator via cfe-commits]

yrouban added inline comments.



Comment at: llvm/include/llvm/Passes/StandardInstrumentations.h:134
 
 public:
   static cl::opt VerifyPreservedCFG;

kuhar wrote:
> yrouban wrote:
> > kuhar wrote:
> > > not necessary anymore
> > there can bee a need to disabled/enable (e.g. for some tests or for 
> > debugging).
> I meant the 'public:'. You made everything public at the very top of the 
> class.
sure


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

https://reviews.llvm.org/D91327

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

[PATCH] D91327: [NewPM] Redesign of PreserveCFG Checker

[Jakub Kuderski via Phabricator via cfe-commits]

kuhar added inline comments.



Comment at: llvm/include/llvm/Passes/StandardInstrumentations.h:134
 
 public:
   static cl::opt VerifyPreservedCFG;

yrouban wrote:
> kuhar wrote:
> > not necessary anymore
> there can bee a need to disabled/enable (e.g. for some tests or for 
> debugging).
I meant the 'public:'. You made everything public at the very top of the class.


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

https://reviews.llvm.org/D91327

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

[PATCH] D91327: [NewPM] Redesign of PreserveCFG Checker

[Yevgeny Rouban via Phabricator via cfe-commits]

yrouban added a comment.

In D91327#2408521 , @kuhar wrote:

> Looks fine to me, but I'm not confident enough to give an approval.

Thanks. I'm working closely with @skatkov and believe he have enough expertise 
to review fully.
The biggest issue with the current design is that we have to send FAM to the 
initialization of StandardInstrumentations/PreservedCFGCheckerInstrumentation. 
This looks asymmetric to the other analysis managers and it would be more 
reasonable to send FAM or all AMs to the instrumentation callbacks as 
parameters.


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

https://reviews.llvm.org/D91327

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

[PATCH] D91327: [NewPM] Redesign of PreserveCFG Checker

[Yevgeny Rouban via Phabricator via cfe-commits]

yrouban added inline comments.



Comment at: llvm/include/llvm/Passes/StandardInstrumentations.h:134
 
 public:
   static cl::opt VerifyPreservedCFG;

kuhar wrote:
> not necessary anymore
there can bee a need to disabled/enable (e.g. for some tests or for debugging).



Comment at: llvm/lib/Passes/StandardInstrumentations.cpp:759
+CFG::printDiff(dbgs(), GraphBefore, GraphAfter);
+report_fatal_error(Twine("CFG unexpectedly changed by ", Pass));
+  };

kuhar wrote:
> I think this will print with `errs()`. Would it make sense to flush `dbgs()` 
> ahead of printing with `errs()`?
I believe this comment is true for all callsites of  //report_fatal_error()//. 
If so it should be done inside.



Comment at: llvm/lib/Passes/StandardInstrumentations.cpp:771
+*const_cast(F)))
+  checkCFG(P, F->getName(), *Result, CFG(F, false /* TrackBBLifetime */));
 else

skatkov wrote:
> why do you check before pass runs?
removed



Comment at: llvm/lib/Passes/StandardInstrumentations.cpp:778
   [this](StringRef P, const PreservedAnalyses ) {
-auto Before = GraphStackBefore.pop_back_val();
-assert(Before.first == P &&
+assert(PassStack.pop_back_val() == P &&
"Before and After callbacks must correspond");

skatkov wrote:
> you should not PassStack.pop_back_val() for product build?
PassStack is used only inside asserts



Comment at: llvm/lib/Passes/StandardInstrumentations.cpp:796
+auto *F = any_cast(IR);
+if (auto *GraphBefore = FAM.getCachedResult(
+*const_cast(F)))

to @skatkov
if //AfterPassCallback// is called before //AM.invalidate()// then we check CFG 
only if the condition (PassPA.allAnalysesInSetPreserved() || 
PassPA.allAnalysesInSetPreserved>()) holds.
if //AfterPassCallback// is called after //AM.invalidate()// then we check CFG 
only if the same condition holds and the PreservedCFGCheckerAnalysis is not 
invalidated. But it is invalidated according to the same condition.
All in all it does not matter if //AfterPassCallback// is called before or 
after //AM.invalidate()//.


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

https://reviews.llvm.org/D91327

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

[PATCH] D91327: [NewPM] Redesign of PreserveCFG Checker

[Jakub Kuderski via Phabricator via cfe-commits]

kuhar added a comment.

Looks fine to me, but I'm not confident enough to give an approval.




Comment at: llvm/lib/Passes/StandardInstrumentations.cpp:733-735
+  static AnalysisKey Key;
+
+public:

This is already public


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

https://reviews.llvm.org/D91327

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

[PATCH] D91327: [NewPM] Redesign of PreserveCFG Checker

[Yevgeny Rouban via Phabricator via cfe-commits]

yrouban updated this revision to Diff 306604.
yrouban marked 4 inline comments as done.
yrouban edited the summary of this revision.
yrouban added a comment.

Addressed comments.


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

https://reviews.llvm.org/D91327

Files:
  clang/lib/CodeGen/BackendUtil.cpp
  llvm/include/llvm/Passes/StandardInstrumentations.h
  llvm/lib/LTO/LTOBackend.cpp
  llvm/lib/Passes/StandardInstrumentations.cpp
  llvm/test/Other/loop-pm-invalidation.ll
  llvm/test/Other/new-pass-manager.ll
  llvm/test/Other/new-pm-O0-defaults.ll
  llvm/test/Other/new-pm-defaults.ll
  llvm/test/Other/new-pm-lto-defaults.ll
  llvm/test/Other/new-pm-thinlto-defaults.ll
  llvm/test/Other/new-pm-thinlto-postlink-pgo-defaults.ll
  llvm/test/Other/new-pm-thinlto-postlink-samplepgo-defaults.ll
  llvm/test/Other/new-pm-thinlto-prelink-pgo-defaults.ll
  llvm/test/Other/new-pm-thinlto-prelink-samplepgo-defaults.ll
  llvm/test/Transforms/LoopRotate/pr35210.ll
  llvm/test/Transforms/SCCP/ipsccp-preserve-analysis.ll
  llvm/tools/opt/NewPMDriver.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 @@
   FunctionPassManager FPM(/*DebugLogging*/ true);
   PassInstrumentationCallbacks PIC;
   StandardInstrumentations SI(/*DebugLogging*/ true);
-  SI.registerCallbacks(PIC);
+  SI.registerCallbacks(PIC, );
   FAM.registerPass([&] { return PassInstrumentationAnalysis(); });
   FAM.registerPass([&] { return AssumptionAnalysis(); });
   FAM.registerPass([&] { return TargetIRAnalysis(); });
@@ -871,7 +871,7 @@
   FunctionPassManager FPM(/*DebugLogging*/ true);
   PassInstrumentationCallbacks PIC;
   StandardInstrumentations SI(/*DebugLogging*/ true);
-  SI.registerCallbacks(PIC);
+  SI.registerCallbacks(PIC, );
   FAM.registerPass([&] { return PassInstrumentationAnalysis(); });
   FAM.registerPass([&] { return AssumptionAnalysis(); });
   FAM.registerPass([&] { return TargetIRAnalysis(); });
@@ -935,7 +935,7 @@
   FunctionPassManager FPM(/*DebugLogging*/ true);
   PassInstrumentationCallbacks PIC;
   StandardInstrumentations SI(/*DebugLogging*/ true);
-  SI.registerCallbacks(PIC);
+  SI.registerCallbacks(PIC, );
   FAM.registerPass([&] { return PassInstrumentationAnalysis(); });
   FAM.registerPass([&] { return AssumptionAnalysis(); });
   FAM.registerPass([&] { return TargetIRAnalysis(); });
Index: llvm/tools/opt/NewPMDriver.cpp
===
--- llvm/tools/opt/NewPMDriver.cpp
+++ llvm/tools/opt/NewPMDriver.cpp
@@ -257,9 +257,14 @@
   P->CSAction = PGOOptions::CSIRUse;
 }
   }
+  LoopAnalysisManager LAM(DebugPM);
+  FunctionAnalysisManager FAM(DebugPM);
+  CGSCCAnalysisManager CGAM(DebugPM);
+  ModuleAnalysisManager MAM(DebugPM);
+
   PassInstrumentationCallbacks PIC;
   StandardInstrumentations SI(DebugPM, VerifyEachPass);
-  SI.registerCallbacks(PIC);
+  SI.registerCallbacks(PIC, );
   DebugifyEachInstrumentation Debugify;
   if (DebugifyEach)
 Debugify.registerCallbacks(PIC);
@@ -357,11 +362,6 @@
 }
   }
 
-  LoopAnalysisManager LAM(DebugPM);
-  FunctionAnalysisManager FAM(DebugPM);
-  CGSCCAnalysisManager CGAM(DebugPM);
-  ModuleAnalysisManager MAM(DebugPM);
-
   // Register the AA manager first so that our version is the one used.
   FAM.registerPass([&] { return std::move(AA); });
   // Register our TargetLibraryInfoImpl.
Index: llvm/test/Transforms/SCCP/ipsccp-preserve-analysis.ll
===
--- llvm/test/Transforms/SCCP/ipsccp-preserve-analysis.ll
+++ llvm/test/Transforms/SCCP/ipsccp-preserve-analysis.ll
@@ -16,7 +16,7 @@
 ; NEW-PM: Running pass: IPSCCPPass
 ; NEW-PM-DAG: Running analysis: AssumptionAnalysis on f1
 ; NEW-PM-DAG: Running analysis: AssumptionAnalysis on f2
-; NEW-PM-NOT: Running analysis:
+; NEW-PM-NOT: Running analysis: AssumptionAnalysis
 
 ; IR-LABEL: @f1
 ; IR-LABEL: entry:
Index: llvm/test/Transforms/LoopRotate/pr35210.ll
===
--- llvm/test/Transforms/LoopRotate/pr35210.ll
+++ llvm/test/Transforms/LoopRotate/pr35210.ll
@@ -1,5 +1,5 @@
-;RUN: opt %s -passes='adce,loop(loop-rotate),adce' -S -debug-pass-manager -debug-only=loop-rotate 2>&1 | FileCheck %s
-;RUN: opt %s -passes='adce,loop-mssa(loop-rotate),adce' -S -debug-pass-manager -debug-only=loop-rotate -verify-memoryssa 2>&1 | FileCheck %s --check-prefix=MSSA
+;RUN: opt %s -passes='adce,loop(loop-rotate),adce' -S -verify-cfg-preserved=0 -debug-pass-manager -debug-only=loop-rotate 2>&1 | FileCheck %s
+;RUN: opt %s -passes='adce,loop-mssa(loop-rotate),adce' -verify-cfg-preserved=0 -S -debug-pass-manager -debug-only=loop-rotate -verify-memoryssa 2>&1 | FileCheck %s --check-prefix=MSSA
 ;REQUIRES: asserts
 
 ; This test is to make sure we 

[PATCH] D91327: [NewPM] Redesign of PreserveCFG Checker

[Serguei Katkov via Phabricator via cfe-commits]

skatkov added inline comments.



Comment at: llvm/lib/Passes/StandardInstrumentations.cpp:769
+auto *F = any_cast(IR);
+if (auto *Result = FAM.getCachedResult(
+*const_cast(F)))

Result -> GraphBefore 



Comment at: llvm/lib/Passes/StandardInstrumentations.cpp:771
+*const_cast(F)))
+  checkCFG(P, F->getName(), *Result, CFG(F, false /* TrackBBLifetime */));
 else

why do you check before pass runs?



Comment at: llvm/lib/Passes/StandardInstrumentations.cpp:778
   [this](StringRef P, const PreservedAnalyses ) {
-auto Before = GraphStackBefore.pop_back_val();
-assert(Before.first == P &&
+assert(PassStack.pop_back_val() == P &&
"Before and After callbacks must correspond");

you should not PassStack.pop_back_val() for product build?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91327

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

[PATCH] D91327: [NewPM] Redesign of PreserveCFG Checker

[Jakub Kuderski via Phabricator via cfe-commits]

kuhar added a comment.

Found some cosmetics, but I'm not familiar enough with the NPM to do a full 
review.




Comment at: llvm/include/llvm/Passes/StandardInstrumentations.h:99
   struct CFG {
 struct BBGuard final : public CallbackVH {
   BBGuard(const BasicBlock *BB) : CallbackVH(BB) {}

Consider pulling this out of CFG. I don't see many reasons for having 3 levels 
of class nesting.



Comment at: llvm/include/llvm/Passes/StandardInstrumentations.h:134
 
 public:
   static cl::opt VerifyPreservedCFG;

not necessary anymore



Comment at: llvm/lib/Passes/StandardInstrumentations.cpp:728
+  Result run(Function , FunctionAnalysisManager ) {
+return Result(, true /* TrackBBLifetime */);
+  }

nit: move parameter name to the left?



Comment at: llvm/lib/Passes/StandardInstrumentations.cpp:759
+CFG::printDiff(dbgs(), GraphBefore, GraphAfter);
+report_fatal_error(Twine("CFG unexpectedly changed by ", Pass));
+  };

I think this will print with `errs()`. Would it make sense to flush `dbgs()` 
ahead of printing with `errs()`?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91327

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

[PATCH] D91327: [NewPM] Redesign of PreserveCFG Checker

[Yevgeny Rouban via Phabricator via cfe-commits]

yrouban created this revision.
yrouban added reviewers: skatkov, kuhar, asbirlea, fedor.sergeev.
Herald added subscribers: cfe-commits, wenlei, steven_wu, hiraditya.
Herald added projects: clang, LLVM.
yrouban requested review of this revision.

Please see the reason for this redesign in the patch D91324 
.
Basically the checker introduces an internal custom CFG analysis that tracks 
current up-to date CFG snapshot. The analysis is invalidated along with any 
other CFG related analysis (the key is //CFGAnalyses//). If the CFG analysis is 
not invalidated at a functional pass exit then the checker asserts that the CFG 
snapshot taken from this analysis is equals to a snapshot of the current CFG.

Along the way:

- the function //CFG::printDiff()// is simplified by removing function name 
calculation. The name is printed by the caller;
- fixed CFG invalidated condition (see //CFG::invalidate()//);
- //StandardInstrumentations::registerCallbacks()// gets additional optional 
parameter of type //FunctionAnalysisManager*//, which is needed by the checker 
to get the custom CFG analysis;
- several PM related tests updated to ignore lines related to running the 
custom CFG analysis.

This patch is safe to land as the CFGChecker is left switched off (the options 
//-verify-cfg-preserved// is false by default). It will be switched on by a 
separate patch to minimize possible reverts.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D91327

Files:
  clang/lib/CodeGen/BackendUtil.cpp
  llvm/include/llvm/Passes/StandardInstrumentations.h
  llvm/lib/LTO/LTOBackend.cpp
  llvm/lib/Passes/StandardInstrumentations.cpp
  llvm/test/Other/loop-pm-invalidation.ll
  llvm/test/Other/new-pass-manager.ll
  llvm/test/Other/new-pm-defaults.ll
  llvm/test/Other/new-pm-lto-defaults.ll
  llvm/test/Other/new-pm-thinlto-defaults.ll
  llvm/test/Other/new-pm-thinlto-postlink-pgo-defaults.ll
  llvm/test/Other/new-pm-thinlto-postlink-samplepgo-defaults.ll
  llvm/test/Other/new-pm-thinlto-prelink-pgo-defaults.ll
  llvm/test/Transforms/LoopRotate/pr35210.ll
  llvm/test/Transforms/SCCP/ipsccp-preserve-analysis.ll
  llvm/tools/opt/NewPMDriver.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 @@
   FunctionPassManager FPM(/*DebugLogging*/ true);
   PassInstrumentationCallbacks PIC;
   StandardInstrumentations SI(/*DebugLogging*/ true);
-  SI.registerCallbacks(PIC);
+  SI.registerCallbacks(PIC, );
   FAM.registerPass([&] { return PassInstrumentationAnalysis(); });
   FAM.registerPass([&] { return AssumptionAnalysis(); });
   FAM.registerPass([&] { return TargetIRAnalysis(); });
@@ -871,7 +871,7 @@
   FunctionPassManager FPM(/*DebugLogging*/ true);
   PassInstrumentationCallbacks PIC;
   StandardInstrumentations SI(/*DebugLogging*/ true);
-  SI.registerCallbacks(PIC);
+  SI.registerCallbacks(PIC, );
   FAM.registerPass([&] { return PassInstrumentationAnalysis(); });
   FAM.registerPass([&] { return AssumptionAnalysis(); });
   FAM.registerPass([&] { return TargetIRAnalysis(); });
@@ -935,7 +935,7 @@
   FunctionPassManager FPM(/*DebugLogging*/ true);
   PassInstrumentationCallbacks PIC;
   StandardInstrumentations SI(/*DebugLogging*/ true);
-  SI.registerCallbacks(PIC);
+  SI.registerCallbacks(PIC, );
   FAM.registerPass([&] { return PassInstrumentationAnalysis(); });
   FAM.registerPass([&] { return AssumptionAnalysis(); });
   FAM.registerPass([&] { return TargetIRAnalysis(); });
Index: llvm/tools/opt/NewPMDriver.cpp
===
--- llvm/tools/opt/NewPMDriver.cpp
+++ llvm/tools/opt/NewPMDriver.cpp
@@ -258,8 +258,6 @@
 }
   }
   PassInstrumentationCallbacks PIC;
-  StandardInstrumentations SI(DebugPM, VerifyEachPass);
-  SI.registerCallbacks(PIC);
   DebugifyEachInstrumentation Debugify;
   if (DebugifyEach)
 Debugify.registerCallbacks(PIC);
@@ -360,6 +358,9 @@
   CGSCCAnalysisManager CGAM(DebugPM);
   ModuleAnalysisManager MAM(DebugPM);
 
+  StandardInstrumentations SI(DebugPM, VerifyEachPass);
+  SI.registerCallbacks(PIC, );
+
   // Register the AA manager first so that our version is the one used.
   FAM.registerPass([&] { return std::move(AA); });
   // Register our TargetLibraryInfoImpl.
Index: llvm/test/Transforms/SCCP/ipsccp-preserve-analysis.ll
===
--- llvm/test/Transforms/SCCP/ipsccp-preserve-analysis.ll
+++ llvm/test/Transforms/SCCP/ipsccp-preserve-analysis.ll
@@ -16,7 +16,7 @@
 ; NEW-PM: Running pass: IPSCCPPass
 ; NEW-PM-DAG: Running analysis: AssumptionAnalysis on f1
 ; NEW-PM-DAG: Running analysis: AssumptionAnalysis on f2
-; NEW-PM-NOT: Running analysis:
+; NEW-PM-NOT: Running analysis: AssumptionAnalysis
 
 ; IR-LABEL: @f1
 ; IR-LABEL: entry:
Index: