[PATCH] D72893: [NewPassManager] Add assertions when getting statefull cached analysis.

2020-04-21 Thread Chandler Carruth via Phabricator via cfe-commits 
chandlerc added a comment.

Really like the approach now. Pretty minor code nits below only. =D




Comment at: llvm/include/llvm/Analysis/CGSCCPassManager.h:856-858
+auto *ResultFAMCP =
+&CGAM.getResult(*C, CG);
+ResultFAMCP->updateFAM(FAM);

Check that it doesn't hit an assert failure, but I think you can remove this 
one.



Comment at: llvm/include/llvm/IR/PassManager.h:812
+auto *RetRes = &static_cast(ResultConcept)->Result;
+if (calledFromProxy) {
+  PreservedAnalyses PA = PreservedAnalyses::none();

asbirlea wrote:
> chandlerc wrote:
> > Could this logic be moved into the callers that pass true here to avoid the 
> > need for a book parameter?
> I haven't addressed this. Yes, I can move the logic in to the caller but I 
> need to make the AnalysisManagerT::Invalidator constructor public.
> Is there another way to do this?
Ah, good point.

How about making this a `verifyNotInvalidated` method separate from 
`getCachedResult`? That still seems a bit cleaner to me than a bool parameter. 
What do you think?



Comment at: llvm/lib/Analysis/CGSCCPassManager.cpp:249-251
+  // Create a new empty Result. This needs to have the FunctionAnalysisManager
+  // inside through the updateFAM() API whenever the FAM's module proxy goes
   // away.

Don't fully understand this comments wording... Maybe something more along the 
lines of:

> We just return an empty result. The caller will use the `updateFAM` interface 
> to correctly register the relevant `FunctionAnalysisManager` based on the 
> context in which this proxy is run.



Comment at: llvm/lib/Analysis/CGSCCPassManager.cpp:341
+ FunctionAnalysisManager &FAM) {
+  auto *ResultFAMCP = &AM.getResult(C, G);
+  ResultFAMCP->updateFAM(FAM);

Omit the variable?



Comment at: llvm/lib/Analysis/CGSCCPassManager.cpp:406-411
   bool NeedFAMProxy =
   AM.getCachedResult(*OldC) != nullptr;
+  FunctionAnalysisManager *FAM = nullptr;
+  if (NeedFAMProxy)
+FAM =
+&AM.getResult(*OldC, 
G).getManager();

Can this be simplified to:

```
FunctionAnalysisManager *FAM = nullptr;
if (auto *FAMProxy = 
AM.getCachedResultgetManager();
```

And then use `FAM` being non-null below instead of the `NeedFAMProxy` bool?




Comment at: llvm/lib/Analysis/CGSCCPassManager.cpp:705
+  if (HasFunctionAnalysisProxy) {
+auto *ResultFAMCP =
+&AM.getResult(*C, G);

Could omit the variable?



Comment at: llvm/lib/Transforms/IPO/Inliner.cpp:1172
 // re-use the exact same logic for updating the call graph to reflect the
 // change.
+// Inside the update, we also update the FunctionAnalysisManager in the

Add a blank line between the paragraphs?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72893



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

[PATCH] D72893: [NewPassManager] Add assertions when getting statefull cached analysis.

2020-04-22 Thread Alina Sbirlea via Phabricator via cfe-commits 
asbirlea added inline comments.



Comment at: llvm/include/llvm/Analysis/CGSCCPassManager.h:856-858
+auto *ResultFAMCP =
+&CGAM.getResult(*C, CG);
+ResultFAMCP->updateFAM(FAM);

chandlerc wrote:
> Check that it doesn't hit an assert failure, but I think you can remove this 
> one.
Removing this hits the assertion.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72893



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

[PATCH] D72893: [NewPassManager] Add assertions when getting statefull cached analysis.

2020-04-22 Thread Alina Sbirlea via Phabricator via cfe-commits 
asbirlea updated this revision to Diff 259450.
asbirlea marked 8 inline comments as done.
asbirlea added a comment.

Address comments.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72893

Files:
  clang/test/CodeGen/thinlto-distributed-newpm.ll
  llvm/include/llvm/Analysis/AliasAnalysis.h
  llvm/include/llvm/Analysis/CGSCCPassManager.h
  llvm/include/llvm/IR/PassManager.h
  llvm/include/llvm/Transforms/Utils/CallGraphUpdater.h
  llvm/lib/Analysis/CGSCCPassManager.cpp
  llvm/lib/Transforms/Coroutines/CoroSplit.cpp
  llvm/lib/Transforms/IPO/Inliner.cpp
  llvm/lib/Transforms/InstCombine/InstructionCombining.cpp
  llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp
  llvm/lib/Transforms/Instrumentation/ControlHeightReduction.cpp
  llvm/lib/Transforms/Scalar/ConstantHoisting.cpp
  llvm/lib/Transforms/Scalar/LoopLoadElimination.cpp
  llvm/lib/Transforms/Scalar/LoopUnrollPass.cpp
  llvm/lib/Transforms/Utils/CallGraphUpdater.cpp
  llvm/lib/Transforms/Vectorize/LoopVectorize.cpp
  llvm/unittests/Analysis/CGSCCPassManagerTest.cpp
  llvm/unittests/IR/PassManagerTest.cpp
  llvm/unittests/Transforms/Scalar/LoopPassManagerTest.cpp

Index: llvm/unittests/Transforms/Scalar/LoopPassManagerTest.cpp
===
--- llvm/unittests/Transforms/Scalar/LoopPassManagerTest.cpp
+++ llvm/unittests/Transforms/Scalar/LoopPassManagerTest.cpp
@@ -779,9 +779,11 @@
   .WillByDefault(Invoke([&](Loop &L, LoopAnalysisManager &AM,
 LoopStandardAnalysisResults &AR) {
 auto &FAMP = AM.getResult(L, AR);
-auto &FAM = FAMP.getManager();
 Function &F = *L.getHeader()->getParent();
-if (FAM.getCachedResult(F))
+// This call will assert when trying to get the actual analysis if the
+// FunctionAnalysis can be invalidated. Only check its existence.
+// Alternatively, use FAM above, for the purposes of this unittest.
+if (FAMP.cachedResultExists(F))
   FAMP.registerOuterAnalysisInvalidation();
 return MLAHandle.getResult();
Index: llvm/unittests/IR/PassManagerTest.cpp
===
--- llvm/unittests/IR/PassManagerTest.cpp
+++ llvm/unittests/IR/PassManagerTest.cpp
@@ -107,20 +107,23 @@
 
 struct TestFunctionPass : PassInfoMixin {
   TestFunctionPass(int &RunCount, int &AnalyzedInstrCount,
-   int &AnalyzedFunctionCount,
+   int &AnalyzedFunctionCount, ModuleAnalysisManager &MAM,
bool OnlyUseCachedResults = false)
   : RunCount(RunCount), AnalyzedInstrCount(AnalyzedInstrCount),
-AnalyzedFunctionCount(AnalyzedFunctionCount),
+AnalyzedFunctionCount(AnalyzedFunctionCount), MAM(MAM),
 OnlyUseCachedResults(OnlyUseCachedResults) {}
 
   PreservedAnalyses run(Function &F, FunctionAnalysisManager &AM) {
 ++RunCount;
 
-const ModuleAnalysisManager &MAM =
-AM.getResult(F).getManager();
+// Getting a cached result that isn't stateless through the proxy will
+// trigger an assert:
+// auto &ModuleProxy = AM.getResult(F);
+// Use MAM, for the purposes of this unittest.
 if (TestModuleAnalysis::Result *TMA =
-MAM.getCachedResult(*F.getParent()))
+MAM.getCachedResult(*F.getParent())) {
   AnalyzedFunctionCount += TMA->FunctionCount;
+}
 
 if (OnlyUseCachedResults) {
   // Hack to force the use of the cached interface.
@@ -139,6 +142,7 @@
   int &RunCount;
   int &AnalyzedInstrCount;
   int &AnalyzedFunctionCount;
+  ModuleAnalysisManager &MAM;
   bool OnlyUseCachedResults;
 };
 
@@ -436,8 +440,9 @@
 {
   // Pointless scope to test move assignment.
   FunctionPassManager NestedFPM(/*DebugLogging*/ true);
-  NestedFPM.addPass(TestFunctionPass(
-  FunctionPassRunCount1, AnalyzedInstrCount1, AnalyzedFunctionCount1));
+  NestedFPM.addPass(TestFunctionPass(FunctionPassRunCount1,
+ AnalyzedInstrCount1,
+ AnalyzedFunctionCount1, MAM));
   FPM = std::move(NestedFPM);
 }
 NestedMPM.addPass(createModuleToFunctionPassAdaptor(std::move(FPM)));
@@ -455,7 +460,7 @@
   {
 FunctionPassManager FPM(/*DebugLogging*/ true);
 FPM.addPass(TestFunctionPass(FunctionPassRunCount2, AnalyzedInstrCount2,
- AnalyzedFunctionCount2));
+ AnalyzedFunctionCount2, MAM));
 MPM.addPass(createModuleToFunctionPassAdaptor(std::move(FPM)));
   }
 
@@ -468,7 +473,7 @@
   {
 FunctionPassManager FPM(/*DebugLogging*/ true);
 FPM.addPass(TestFunctionPass(FunctionPassRunCount3, AnalyzedInstrCount3,
- AnalyzedFunctionCount3));
+ AnalyzedFunctionCount3, MAM));
 FPM.addPass(TestInvalida

[PATCH] D72893: [NewPassManager] Add assertions when getting statefull cached analysis.

2020-04-30 Thread Alina Sbirlea via Phabricator via cfe-commits 
asbirlea added a comment.

Gentle ping for more comments :-)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72893



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

[PATCH] D72893: [NewPassManager] Add assertions when getting statefull cached analysis.

2020-05-07 Thread Alina Sbirlea via Phabricator via cfe-commits 
asbirlea added a comment.

Weekly re-ping.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72893



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

[PATCH] D72893: [NewPassManager] Add assertions when getting statefull cached analysis.

2020-05-12 Thread Chandler Carruth via Phabricator via cfe-commits 
chandlerc accepted this revision.
chandlerc added a comment.
This revision is now accepted and ready to land.

LGTM other than two nits here, this is really awesome!




Comment at: llvm/include/llvm/Analysis/CGSCCPassManager.h:856-858
+auto *ResultFAMCP =
+&CGAM.getResult(*C, CG);
+ResultFAMCP->updateFAM(FAM);

asbirlea wrote:
> chandlerc wrote:
> > Check that it doesn't hit an assert failure, but I think you can remove 
> > this one.
> Removing this hits the assertion.
Dunno what I was thinking. Of course it does.

Anyways, skip the variable and just update the result directly?



Comment at: llvm/include/llvm/Analysis/CGSCCPassManager.h:908
+// result.
+ResultFAMCP =
+&CGAM.getResult(*C, CG);

Similar to above, skip the variable?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72893



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

[PATCH] D72893: [NewPassManager] Add assertions when getting statefull cached analysis.

2020-05-12 Thread Alina Sbirlea via Phabricator via cfe-commits 
asbirlea updated this revision to Diff 263590.
asbirlea marked 3 inline comments as done.
asbirlea added a comment.

Address comments.
Thank you for the review!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72893

Files:
  clang/test/CodeGen/thinlto-distributed-newpm.ll
  llvm/include/llvm/Analysis/AliasAnalysis.h
  llvm/include/llvm/Analysis/CGSCCPassManager.h
  llvm/include/llvm/IR/PassManager.h
  llvm/include/llvm/Transforms/Utils/CallGraphUpdater.h
  llvm/lib/Analysis/CGSCCPassManager.cpp
  llvm/lib/Transforms/Coroutines/CoroSplit.cpp
  llvm/lib/Transforms/IPO/Inliner.cpp
  llvm/lib/Transforms/InstCombine/InstructionCombining.cpp
  llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp
  llvm/lib/Transforms/Instrumentation/ControlHeightReduction.cpp
  llvm/lib/Transforms/Scalar/ConstantHoisting.cpp
  llvm/lib/Transforms/Scalar/LoopLoadElimination.cpp
  llvm/lib/Transforms/Scalar/LoopUnrollPass.cpp
  llvm/lib/Transforms/Utils/CallGraphUpdater.cpp
  llvm/lib/Transforms/Vectorize/LoopVectorize.cpp
  llvm/unittests/Analysis/CGSCCPassManagerTest.cpp
  llvm/unittests/IR/PassManagerTest.cpp
  llvm/unittests/Transforms/Scalar/LoopPassManagerTest.cpp

Index: llvm/unittests/Transforms/Scalar/LoopPassManagerTest.cpp
===
--- llvm/unittests/Transforms/Scalar/LoopPassManagerTest.cpp
+++ llvm/unittests/Transforms/Scalar/LoopPassManagerTest.cpp
@@ -779,9 +779,11 @@
   .WillByDefault(Invoke([&](Loop &L, LoopAnalysisManager &AM,
 LoopStandardAnalysisResults &AR) {
 auto &FAMP = AM.getResult(L, AR);
-auto &FAM = FAMP.getManager();
 Function &F = *L.getHeader()->getParent();
-if (FAM.getCachedResult(F))
+// This call will assert when trying to get the actual analysis if the
+// FunctionAnalysis can be invalidated. Only check its existence.
+// Alternatively, use FAM above, for the purposes of this unittest.
+if (FAMP.cachedResultExists(F))
   FAMP.registerOuterAnalysisInvalidation();
 return MLAHandle.getResult();
Index: llvm/unittests/IR/PassManagerTest.cpp
===
--- llvm/unittests/IR/PassManagerTest.cpp
+++ llvm/unittests/IR/PassManagerTest.cpp
@@ -107,20 +107,23 @@
 
 struct TestFunctionPass : PassInfoMixin {
   TestFunctionPass(int &RunCount, int &AnalyzedInstrCount,
-   int &AnalyzedFunctionCount,
+   int &AnalyzedFunctionCount, ModuleAnalysisManager &MAM,
bool OnlyUseCachedResults = false)
   : RunCount(RunCount), AnalyzedInstrCount(AnalyzedInstrCount),
-AnalyzedFunctionCount(AnalyzedFunctionCount),
+AnalyzedFunctionCount(AnalyzedFunctionCount), MAM(MAM),
 OnlyUseCachedResults(OnlyUseCachedResults) {}
 
   PreservedAnalyses run(Function &F, FunctionAnalysisManager &AM) {
 ++RunCount;
 
-const ModuleAnalysisManager &MAM =
-AM.getResult(F).getManager();
+// Getting a cached result that isn't stateless through the proxy will
+// trigger an assert:
+// auto &ModuleProxy = AM.getResult(F);
+// Use MAM, for the purposes of this unittest.
 if (TestModuleAnalysis::Result *TMA =
-MAM.getCachedResult(*F.getParent()))
+MAM.getCachedResult(*F.getParent())) {
   AnalyzedFunctionCount += TMA->FunctionCount;
+}
 
 if (OnlyUseCachedResults) {
   // Hack to force the use of the cached interface.
@@ -139,6 +142,7 @@
   int &RunCount;
   int &AnalyzedInstrCount;
   int &AnalyzedFunctionCount;
+  ModuleAnalysisManager &MAM;
   bool OnlyUseCachedResults;
 };
 
@@ -436,8 +440,9 @@
 {
   // Pointless scope to test move assignment.
   FunctionPassManager NestedFPM(/*DebugLogging*/ true);
-  NestedFPM.addPass(TestFunctionPass(
-  FunctionPassRunCount1, AnalyzedInstrCount1, AnalyzedFunctionCount1));
+  NestedFPM.addPass(TestFunctionPass(FunctionPassRunCount1,
+ AnalyzedInstrCount1,
+ AnalyzedFunctionCount1, MAM));
   FPM = std::move(NestedFPM);
 }
 NestedMPM.addPass(createModuleToFunctionPassAdaptor(std::move(FPM)));
@@ -455,7 +460,7 @@
   {
 FunctionPassManager FPM(/*DebugLogging*/ true);
 FPM.addPass(TestFunctionPass(FunctionPassRunCount2, AnalyzedInstrCount2,
- AnalyzedFunctionCount2));
+ AnalyzedFunctionCount2, MAM));
 MPM.addPass(createModuleToFunctionPassAdaptor(std::move(FPM)));
   }
 
@@ -468,7 +473,7 @@
   {
 FunctionPassManager FPM(/*DebugLogging*/ true);
 FPM.addPass(TestFunctionPass(FunctionPassRunCount3, AnalyzedInstrCount3,
- AnalyzedFunctionCount3));
+ AnalyzedFunctionCount3, MAM));

[PATCH] D72893: [NewPassManager] Add assertions when getting statefull cached analysis.

2020-05-13 Thread Alina Sbirlea via Phabricator via cfe-commits 
This revision was automatically updated to reflect the committed changes.
Closed by commit rGbd541b217f4d: [NewPassManager] Add assertions when getting 
statefull cached analysis. (authored by asbirlea).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72893

Files:
  clang/test/CodeGen/thinlto-distributed-newpm.ll
  llvm/include/llvm/Analysis/AliasAnalysis.h
  llvm/include/llvm/Analysis/CGSCCPassManager.h
  llvm/include/llvm/IR/PassManager.h
  llvm/include/llvm/Transforms/Utils/CallGraphUpdater.h
  llvm/lib/Analysis/CGSCCPassManager.cpp
  llvm/lib/Transforms/Coroutines/CoroSplit.cpp
  llvm/lib/Transforms/IPO/Inliner.cpp
  llvm/lib/Transforms/InstCombine/InstructionCombining.cpp
  llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp
  llvm/lib/Transforms/Instrumentation/ControlHeightReduction.cpp
  llvm/lib/Transforms/Scalar/ConstantHoisting.cpp
  llvm/lib/Transforms/Scalar/LoopLoadElimination.cpp
  llvm/lib/Transforms/Scalar/LoopUnrollPass.cpp
  llvm/lib/Transforms/Utils/CallGraphUpdater.cpp
  llvm/lib/Transforms/Vectorize/LoopVectorize.cpp
  llvm/unittests/Analysis/CGSCCPassManagerTest.cpp
  llvm/unittests/IR/PassManagerTest.cpp
  llvm/unittests/Transforms/Scalar/LoopPassManagerTest.cpp

Index: llvm/unittests/Transforms/Scalar/LoopPassManagerTest.cpp
===
--- llvm/unittests/Transforms/Scalar/LoopPassManagerTest.cpp
+++ llvm/unittests/Transforms/Scalar/LoopPassManagerTest.cpp
@@ -779,9 +779,11 @@
   .WillByDefault(Invoke([&](Loop &L, LoopAnalysisManager &AM,
 LoopStandardAnalysisResults &AR) {
 auto &FAMP = AM.getResult(L, AR);
-auto &FAM = FAMP.getManager();
 Function &F = *L.getHeader()->getParent();
-if (FAM.getCachedResult(F))
+// This call will assert when trying to get the actual analysis if the
+// FunctionAnalysis can be invalidated. Only check its existence.
+// Alternatively, use FAM above, for the purposes of this unittest.
+if (FAMP.cachedResultExists(F))
   FAMP.registerOuterAnalysisInvalidation();
 return MLAHandle.getResult();
Index: llvm/unittests/IR/PassManagerTest.cpp
===
--- llvm/unittests/IR/PassManagerTest.cpp
+++ llvm/unittests/IR/PassManagerTest.cpp
@@ -107,20 +107,23 @@
 
 struct TestFunctionPass : PassInfoMixin {
   TestFunctionPass(int &RunCount, int &AnalyzedInstrCount,
-   int &AnalyzedFunctionCount,
+   int &AnalyzedFunctionCount, ModuleAnalysisManager &MAM,
bool OnlyUseCachedResults = false)
   : RunCount(RunCount), AnalyzedInstrCount(AnalyzedInstrCount),
-AnalyzedFunctionCount(AnalyzedFunctionCount),
+AnalyzedFunctionCount(AnalyzedFunctionCount), MAM(MAM),
 OnlyUseCachedResults(OnlyUseCachedResults) {}
 
   PreservedAnalyses run(Function &F, FunctionAnalysisManager &AM) {
 ++RunCount;
 
-const ModuleAnalysisManager &MAM =
-AM.getResult(F).getManager();
+// Getting a cached result that isn't stateless through the proxy will
+// trigger an assert:
+// auto &ModuleProxy = AM.getResult(F);
+// Use MAM, for the purposes of this unittest.
 if (TestModuleAnalysis::Result *TMA =
-MAM.getCachedResult(*F.getParent()))
+MAM.getCachedResult(*F.getParent())) {
   AnalyzedFunctionCount += TMA->FunctionCount;
+}
 
 if (OnlyUseCachedResults) {
   // Hack to force the use of the cached interface.
@@ -139,6 +142,7 @@
   int &RunCount;
   int &AnalyzedInstrCount;
   int &AnalyzedFunctionCount;
+  ModuleAnalysisManager &MAM;
   bool OnlyUseCachedResults;
 };
 
@@ -436,8 +440,9 @@
 {
   // Pointless scope to test move assignment.
   FunctionPassManager NestedFPM(/*DebugLogging*/ true);
-  NestedFPM.addPass(TestFunctionPass(
-  FunctionPassRunCount1, AnalyzedInstrCount1, AnalyzedFunctionCount1));
+  NestedFPM.addPass(TestFunctionPass(FunctionPassRunCount1,
+ AnalyzedInstrCount1,
+ AnalyzedFunctionCount1, MAM));
   FPM = std::move(NestedFPM);
 }
 NestedMPM.addPass(createModuleToFunctionPassAdaptor(std::move(FPM)));
@@ -455,7 +460,7 @@
   {
 FunctionPassManager FPM(/*DebugLogging*/ true);
 FPM.addPass(TestFunctionPass(FunctionPassRunCount2, AnalyzedInstrCount2,
- AnalyzedFunctionCount2));
+ AnalyzedFunctionCount2, MAM));
 MPM.addPass(createModuleToFunctionPassAdaptor(std::move(FPM)));
   }
 
@@ -468,7 +473,7 @@
   {
 FunctionPassManager FPM(/*DebugLogging*/ true);
 FPM.addPass(TestFunctionPass(FunctionPassRunCount3, AnalyzedInstrCount3,
- AnalyzedFunctionCount3));
+

[PATCH] D72893: [NewPassManager] Add assertions when getting statefull cached analysis.

2020-04-01 Thread Alina Sbirlea via Phabricator via cfe-commits 
asbirlea updated this revision to Diff 254378.
asbirlea added a comment.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Update clang test.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72893

Files:
  clang/test/CodeGen/thinlto-distributed-newpm.ll
  llvm/include/llvm/Analysis/AliasAnalysis.h
  llvm/include/llvm/Analysis/CGSCCPassManager.h
  llvm/include/llvm/IR/PassManager.h
  llvm/include/llvm/Transforms/Utils/CallGraphUpdater.h
  llvm/lib/Analysis/CGSCCPassManager.cpp
  llvm/lib/Transforms/Coroutines/CoroSplit.cpp
  llvm/lib/Transforms/IPO/Inliner.cpp
  llvm/lib/Transforms/InstCombine/InstructionCombining.cpp
  llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp
  llvm/lib/Transforms/Instrumentation/ControlHeightReduction.cpp
  llvm/lib/Transforms/Scalar/ConstantHoisting.cpp
  llvm/lib/Transforms/Scalar/LoopLoadElimination.cpp
  llvm/lib/Transforms/Scalar/LoopUnrollPass.cpp
  llvm/lib/Transforms/Utils/CallGraphUpdater.cpp
  llvm/lib/Transforms/Vectorize/LoopVectorize.cpp
  llvm/unittests/Analysis/CGSCCPassManagerTest.cpp
  llvm/unittests/IR/PassManagerTest.cpp
  llvm/unittests/Transforms/Scalar/LoopPassManagerTest.cpp

Index: llvm/unittests/Transforms/Scalar/LoopPassManagerTest.cpp
===
--- llvm/unittests/Transforms/Scalar/LoopPassManagerTest.cpp
+++ llvm/unittests/Transforms/Scalar/LoopPassManagerTest.cpp
@@ -779,9 +779,11 @@
   .WillByDefault(Invoke([&](Loop &L, LoopAnalysisManager &AM,
 LoopStandardAnalysisResults &AR) {
 auto &FAMP = AM.getResult(L, AR);
-auto &FAM = FAMP.getManager();
 Function &F = *L.getHeader()->getParent();
-if (FAM.getCachedResult(F))
+// This call will assert when trying to get the actual analysis if the
+// FunctionAnalysis can be invalidated. Only check its existence.
+// Alternatively, use FAM above, for the purposes of this unittest.
+if (FAMP.cachedResultExists(F))
   FAMP.registerOuterAnalysisInvalidation();
 return MLAHandle.getResult();
Index: llvm/unittests/IR/PassManagerTest.cpp
===
--- llvm/unittests/IR/PassManagerTest.cpp
+++ llvm/unittests/IR/PassManagerTest.cpp
@@ -107,20 +107,23 @@
 
 struct TestFunctionPass : PassInfoMixin {
   TestFunctionPass(int &RunCount, int &AnalyzedInstrCount,
-   int &AnalyzedFunctionCount,
+   int &AnalyzedFunctionCount, ModuleAnalysisManager &MAM,
bool OnlyUseCachedResults = false)
   : RunCount(RunCount), AnalyzedInstrCount(AnalyzedInstrCount),
-AnalyzedFunctionCount(AnalyzedFunctionCount),
+AnalyzedFunctionCount(AnalyzedFunctionCount), MAM(MAM),
 OnlyUseCachedResults(OnlyUseCachedResults) {}
 
   PreservedAnalyses run(Function &F, FunctionAnalysisManager &AM) {
 ++RunCount;
 
-const ModuleAnalysisManager &MAM =
-AM.getResult(F).getManager();
+// Getting a cached result that isn't stateless through the proxy will
+// trigger an assert:
+// auto &ModuleProxy = AM.getResult(F);
+// Use MAM, for the purposes of this unittest.
 if (TestModuleAnalysis::Result *TMA =
-MAM.getCachedResult(*F.getParent()))
+MAM.getCachedResult(*F.getParent())) {
   AnalyzedFunctionCount += TMA->FunctionCount;
+}
 
 if (OnlyUseCachedResults) {
   // Hack to force the use of the cached interface.
@@ -139,6 +142,7 @@
   int &RunCount;
   int &AnalyzedInstrCount;
   int &AnalyzedFunctionCount;
+  ModuleAnalysisManager &MAM;
   bool OnlyUseCachedResults;
 };
 
@@ -436,8 +440,9 @@
 {
   // Pointless scope to test move assignment.
   FunctionPassManager NestedFPM(/*DebugLogging*/ true);
-  NestedFPM.addPass(TestFunctionPass(
-  FunctionPassRunCount1, AnalyzedInstrCount1, AnalyzedFunctionCount1));
+  NestedFPM.addPass(TestFunctionPass(FunctionPassRunCount1,
+ AnalyzedInstrCount1,
+ AnalyzedFunctionCount1, MAM));
   FPM = std::move(NestedFPM);
 }
 NestedMPM.addPass(createModuleToFunctionPassAdaptor(std::move(FPM)));
@@ -455,7 +460,7 @@
   {
 FunctionPassManager FPM(/*DebugLogging*/ true);
 FPM.addPass(TestFunctionPass(FunctionPassRunCount2, AnalyzedInstrCount2,
- AnalyzedFunctionCount2));
+ AnalyzedFunctionCount2, MAM));
 MPM.addPass(createModuleToFunctionPassAdaptor(std::move(FPM)));
   }
 
@@ -468,7 +473,7 @@
   {
 FunctionPassManager FPM(/*DebugLogging*/ true);
 FPM.addPass(TestFunctionPass(FunctionPassRunCount3, AnalyzedInstrCount3,
- AnalyzedFunctionCount3));
+ AnalyzedFunctionCount3, MAM));