[PATCH] D53428: Adjust cl::opt uses for opt_storage refactored

[Yevgeny Rouban via Phabricator via cfe-commits]

yrouban created this revision.
yrouban added reviewers: chandlerc, rsmith, reames, skatkov.
Herald added a subscriber: cfe-commits.

This patch fixes compilation errors that result from refactoring of LLVM 
cl::opt structures introduced by https://reviews.llvm.org/D53426.
Basically for class-based cl::opt to access to the instance of MyClass 
one of the overloadable operators should be used.

For example if we have an option defined as cl::opt MyStringOption;
Then the changes should look like the following:

Old: MyStringOption.c_str()
New: MyStringOption->c_str()

Old: cout << MyStringOption;
New: cout << *MyStringOption;


Repository:
  rC Clang

https://reviews.llvm.org/D53428

Files:
  lib/Tooling/Execution.cpp
  tools/arcmt-test/arcmt-test.cpp
  tools/clang-import-test/clang-import-test.cpp
  tools/clang-rename/ClangRename.cpp
  tools/diagtool/FindDiagnosticID.cpp
  unittests/Tooling/ExecutionTest.cpp


Index: unittests/Tooling/ExecutionTest.cpp
===
--- unittests/Tooling/ExecutionTest.cpp
+++ unittests/Tooling/ExecutionTest.cpp
@@ -158,7 +158,7 @@
   auto Executor = internal::createExecutorFromCommandLineArgsImpl(
   argc, &argv[0], TestCategory);
   ASSERT_TRUE((bool)Executor);
-  EXPECT_EQ(BeforeReset, "set");
+  EXPECT_EQ(*BeforeReset, "set");
   BeforeReset.removeArgument();
 }
 
Index: tools/diagtool/FindDiagnosticID.cpp
===
--- tools/diagtool/FindDiagnosticID.cpp
+++ tools/diagtool/FindDiagnosticID.cpp
@@ -66,7 +66,7 @@
   return 0;
 }
 
-llvm::errs() << "error: invalid diagnostic '" << DiagnosticName << "'\n";
+llvm::errs() << "error: invalid diagnostic '" << *DiagnosticName << "'\n";
 return 1;
   }
   OS << Diag->DiagID << "\n";
Index: tools/clang-rename/ClangRename.cpp
===
--- tools/clang-rename/ClangRename.cpp
+++ tools/clang-rename/ClangRename.cpp
@@ -104,9 +104,9 @@
   if (!Input.empty()) {
 // Populate QualifiedNames and NewNames from a YAML file.
 ErrorOr> Buffer =
-llvm::MemoryBuffer::getFile(Input);
+llvm::MemoryBuffer::getFile(*Input);
 if (!Buffer) {
-  errs() << "clang-rename: failed to read " << Input << ": "
+  errs() << "clang-rename: failed to read " << *Input << ": "
  << Buffer.getError().message() << "\n";
   return 1;
 }
Index: tools/clang-import-test/clang-import-test.cpp
===
--- tools/clang-import-test/clang-import-test.cpp
+++ tools/clang-import-test/clang-import-test.cpp
@@ -174,7 +174,7 @@
 
   {
 using namespace driver::types;
-ID Id = lookupTypeForTypeSpecifier(Input.c_str());
+ID Id = lookupTypeForTypeSpecifier(Input->c_str());
 assert(Id != TY_INVALID);
 if (isCXX(Id)) {
   Inv->getLangOpts()->CPlusPlus = true;
Index: tools/arcmt-test/arcmt-test.cpp
===
--- tools/arcmt-test/arcmt-test.cpp
+++ tools/arcmt-test/arcmt-test.cpp
@@ -243,7 +243,7 @@
   if (RemappingsFile.empty())
 inputBuf = MemoryBuffer::getSTDIN();
   else
-inputBuf = MemoryBuffer::getFile(RemappingsFile);
+inputBuf = MemoryBuffer::getFile(*RemappingsFile);
   if (!inputBuf) {
 errs() << "error: could not read remappings input\n";
 return true;
Index: lib/Tooling/Execution.cpp
===
--- lib/Tooling/Execution.cpp
+++ lib/Tooling/Execution.cpp
@@ -82,7 +82,7 @@
 return std::move(*Executor);
   }
   return llvm::make_error(
-  llvm::Twine("Executor \"") + ExecutorName + "\" is not registered.",
+  llvm::Twine("Executor \"") + *ExecutorName + "\" is not registered.",
   llvm::inconvertibleErrorCode());
 }
 } // end namespace internal


Index: unittests/Tooling/ExecutionTest.cpp
===
--- unittests/Tooling/ExecutionTest.cpp
+++ unittests/Tooling/ExecutionTest.cpp
@@ -158,7 +158,7 @@
   auto Executor = internal::createExecutorFromCommandLineArgsImpl(
   argc, &argv[0], TestCategory);
   ASSERT_TRUE((bool)Executor);
-  EXPECT_EQ(BeforeReset, "set");
+  EXPECT_EQ(*BeforeReset, "set");
   BeforeReset.removeArgument();
 }
 
Index: tools/diagtool/FindDiagnosticID.cpp
===
--- tools/diagtool/FindDiagnosticID.cpp
+++ tools/diagtool/FindDiagnosticID.cpp
@@ -66,7 +66,7 @@
   return 0;
 }
 
-llvm::errs() << "error: invalid diagnostic '" << DiagnosticName << "'\n";
+llvm::errs() << "error: invalid diagnostic '" << *DiagnosticName << "'\n";
 return 1;
   }
   OS << Diag->DiagID << "\n";
Index: tools/clang-rename/ClangRename.cpp
===
--- tools/clang-rename/ClangRename.cpp
+++ tools/clang-rename/ClangR

[PATCH] D57220: Test fix for isViableInline remark message

[Yevgeny Rouban via Phabricator via cfe-commits]

yrouban created this revision.
yrouban added reviewers: xbolva00, anemet.
Herald added a subscriber: cfe-commits.
yrouban added a parent revision: D57089: Provide reason messages for unviable 
inlining.

This patch adjusts the test remark message that is changed by D57089 
.


Repository:
  rC Clang

https://reviews.llvm.org/D57220

Files:
  test/Frontend/optimization-remark-with-hotness.c


Index: test/Frontend/optimization-remark-with-hotness.c
===
--- test/Frontend/optimization-remark-with-hotness.c
+++ test/Frontend/optimization-remark-with-hotness.c
@@ -66,7 +66,7 @@
 
 int main(int argc, const char *argv[]) {
   for (int i = 0; i < 30; i++)
-// expected-remark@+1 {{bar not inlined into main because it should never 
be inlined (cost=never): always inliner (hotness:}}
+// expected-remark@+1 {{bar not inlined into main because it should never 
be inlined (cost=never): no alwaysinline attribute (hotness:}}
 bar(argc);
   return sum;
 }


Index: test/Frontend/optimization-remark-with-hotness.c
===
--- test/Frontend/optimization-remark-with-hotness.c
+++ test/Frontend/optimization-remark-with-hotness.c
@@ -66,7 +66,7 @@
 
 int main(int argc, const char *argv[]) {
   for (int i = 0; i < 30; i++)
-// expected-remark@+1 {{bar not inlined into main because it should never be inlined (cost=never): always inliner (hotness:}}
+// expected-remark@+1 {{bar not inlined into main because it should never be inlined (cost=never): no alwaysinline attribute (hotness:}}
 bar(argc);
   return sum;
 }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D57220: Test fix for isViableInline remark message

[Yevgeny Rouban via Phabricator via cfe-commits]

yrouban marked an inline comment as done.
yrouban added inline comments.



Comment at: test/Frontend/optimization-remark-with-hotness.c:63
   // NO_PGO: '-fdiagnostics-hotness-threshold=' requires profile-guided 
optimization information
   // expected-remark@+1 {{foo inlined into bar with (cost=always): always 
inliner (hotness:}}
   sum += foo(x, x - 2);

xbolva00 wrote:
> What about this line?
positive inlining remarks did not change. I did not see tests failure, did you?


Repository:
  rC Clang

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

https://reviews.llvm.org/D57220



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

[PATCH] D57220: Test fix for isViableInline remark message

[Yevgeny Rouban via Phabricator via cfe-commits]

yrouban closed this revision.
yrouban added a comment.

Landed along with D57089  (see rC352849 
).


Repository:
  rC Clang

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

https://reviews.llvm.org/D57220



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

[PATCH] D120713: [clangd] Make dexp command line options sticky

[Yevgeny Rouban via Phabricator via cfe-commits]

yrouban closed this revision.
yrouban added a comment.

Closed by commit 
https://reviews.llvm.org/rG7fb39fb6d6665cd469557b43eb205cc32b0a7ac3.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D120713

___
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);
   FAM.registerPass([&] { return PassInstrumentationAnalysis(&PIC); });
   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);
   FAM.registerPass([&] { return PassInstrumentationAnalysis(&PIC); });
   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);
   FAM.registerPass([&] { return PassInstrumentationAnalysis(&PIC); });
   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, &FAM);
+
   // 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
 ;

[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);
   FAM.registerPass([&] { return PassInstrumentationAnalysis(&PIC); });
   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);
   FAM.registerPass([&] { return PassInstrumentationAnalysis(&PIC); });
   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);
   FAM.registerPass([&] { return PassInstrumentationAnalysis(&PIC); });
   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, &FAM);
   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 t

[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 &PassPA) {
-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

[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:
> 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] D146003: [StandardInstrumentations] Check that the number of instructions doesn't change if analyses are preserved

[Yevgeny Rouban via Phabricator via cfe-commits]

yrouban added a comment.

Looks good in general, but it would be better to have this patch split into 
several smaller patches: renaming NFC, InstructionCounter intro, one for each 
particular pass fix.




Comment at: llvm/lib/Passes/StandardInstrumentations.cpp:1057
+  struct InstructionCount {
+unsigned Count;
+  };

may be rename to //Value//?



Comment at: llvm/lib/Passes/StandardInstrumentations.cpp:2208
   // to ensure that other callbacks are not included in the timings.
   TimeProfilingPasses.registerCallbacks(PIC);
+

I think the time profiling pass has to be the last one.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146003

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

[PATCH] D120713: [clangd] Make dexp command line options sticky

[Yevgeny Rouban via Phabricator via cfe-commits]

yrouban created this revision.
yrouban added reviewers: jhenderson, lattner, jpienaar, mehdi_amini.
yrouban added a project: clang.
Herald added subscribers: usaxena95, kadircet, arphaman.
yrouban requested review of this revision.
Herald added subscribers: cfe-commits, MaskRay, ilya-biryukov.
Herald added a project: clang-tools-extra.

Preparing for the cl::opt reset fix proposed on D115433 
 this patch fixes the dexp tool to preserve 
its three command line options (IndexLocation, ExecCommand, ProjectRoot) from 
reset that is done before parsing query options.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D120713

Files:
  clang-tools-extra/clangd/index/dex/dexp/Dexp.cpp


Index: clang-tools-extra/clangd/index/dex/dexp/Dexp.cpp
===
--- clang-tools-extra/clangd/index/dex/dexp/Dexp.cpp
+++ clang-tools-extra/clangd/index/dex/dexp/Dexp.cpp
@@ -413,6 +413,12 @@
   using namespace clang::clangd;
 
   llvm::cl::ParseCommandLineOptions(argc, argv, Overview);
+
+  // Protect IndexLocation, ExecCommand and ProjectRoot from being reset.
+  IndexLocation.setValue(IndexLocation, true /* initial */);
+  ExecCommand.setValue(ExecCommand, true /* initial */);
+  ProjectRoot.setValue(ProjectRoot, true /* initial */);
+
   llvm::cl::ResetCommandLineParser(); // We reuse it for REPL commands.
   llvm::sys::PrintStackTraceOnErrorSignal(argv[0]);
 


Index: clang-tools-extra/clangd/index/dex/dexp/Dexp.cpp
===
--- clang-tools-extra/clangd/index/dex/dexp/Dexp.cpp
+++ clang-tools-extra/clangd/index/dex/dexp/Dexp.cpp
@@ -413,6 +413,12 @@
   using namespace clang::clangd;
 
   llvm::cl::ParseCommandLineOptions(argc, argv, Overview);
+
+  // Protect IndexLocation, ExecCommand and ProjectRoot from being reset.
+  IndexLocation.setValue(IndexLocation, true /* initial */);
+  ExecCommand.setValue(ExecCommand, true /* initial */);
+  ProjectRoot.setValue(ProjectRoot, true /* initial */);
+
   llvm::cl::ResetCommandLineParser(); // We reuse it for REPL commands.
   llvm::sys::PrintStackTraceOnErrorSignal(argv[0]);
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D120713: [clangd] Make dexp command line options sticky

[Yevgeny Rouban via Phabricator via cfe-commits]

yrouban added subscribers: jhenderson, lattner.
yrouban added a comment.

In D120713#3353481 , @lattner wrote:

> I'm not a competent reviewer for clangd
>
>> ! In D120713#3353532 , @jhenderson 
>> wrote:
>
> I don't know anything about clangd either. Not sure why I was added as a 
> reviewer :)

I took your names from https://reviews.llvm.org/D105959 which affected Dexp.cpp 
and was related to cl::opt.
Ok, I will shoot in the dark once more :)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D120713

___
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);
   FAM.registerPass([&] { return PassInstrumentationAnalysis(&PIC); });
   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);
   FAM.registerPass([&] { return PassInstrumentationAnalysis(&PIC); });
   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);
   FAM.registerPass([&] { return PassInstrumentationAnalysis(&PIC); });
   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, &FAM);
   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 t

[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

[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);
   FAM.registerPass([&] { return PassInstrumentationAnalysis(&PIC); });
   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);
   FAM.registerPass([&] { return PassInstrumentationAnalysis(&PIC); });
   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);
   FAM.registerPass([&] { return PassInstrumentationAnalysis(&PIC); });
   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, &FAM);
   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 w

[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 &PIC,

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 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 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);
   FAM.registerPass([&] { return PassInstrumentationAnalysis(&PIC); });
   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);
   FAM.registerPass([&] { return PassInstrumentationAnalysis(&PIC); });
   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);
   FAM.registerPass([&] { return PassInstrumentationAnalysis(&PIC); });
   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, &FAM);
   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: | Fi

[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&id=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);
   FAM.registerPass([&] { return PassInstrumentationAnalysis(&PIC); });
   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);
   FAM.registerPass([&] { return PassInstrumentationAnalysis(&PIC); });
   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);
   FAM.registerPass([&] { return PassInstrumentationAnalysis(&PIC); });
   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, &FAM);
   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: