Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[NewPM] Remove LazyValueInfoPrinter Pass #73408

Merged
merged 1 commit into from
Nov 27, 2023

Conversation

boomanaiden154
Copy link
Contributor

This pass isn't used anywhere upstream and thus has no test coverage. For these reasons, remove it.

@llvmbot
Copy link
Collaborator

llvmbot commented Nov 25, 2023

@llvm/pr-subscribers-llvm-analysis

Author: Aiden Grossman (boomanaiden154)

Changes

This pass isn't used anywhere upstream and thus has no test coverage. For these reasons, remove it.


Full diff: https://github.com/llvm/llvm-project/pull/73408.diff

3 Files Affected:

  • (modified) llvm/include/llvm/InitializePasses.h (-1)
  • (modified) llvm/lib/Analysis/Analysis.cpp (-1)
  • (modified) llvm/lib/Analysis/LazyValueInfo.cpp (-34)
diff --git a/llvm/include/llvm/InitializePasses.h b/llvm/include/llvm/InitializePasses.h
index 05122ea98d8ca52..9e9e0be5d0426c0 100644
--- a/llvm/include/llvm/InitializePasses.h
+++ b/llvm/include/llvm/InitializePasses.h
@@ -149,7 +149,6 @@ void initializeLCSSAWrapperPassPass(PassRegistry&);
 void initializeLazyBlockFrequencyInfoPassPass(PassRegistry&);
 void initializeLazyBranchProbabilityInfoPassPass(PassRegistry&);
 void initializeLazyMachineBlockFrequencyInfoPassPass(PassRegistry&);
-void initializeLazyValueInfoPrinterPass(PassRegistry&);
 void initializeLazyValueInfoWrapperPassPass(PassRegistry&);
 void initializeLegacyLICMPassPass(PassRegistry&);
 void initializeLegalizerPass(PassRegistry&);
diff --git a/llvm/lib/Analysis/Analysis.cpp b/llvm/lib/Analysis/Analysis.cpp
index 1fa14b005d37609..b670aa7e53511a2 100644
--- a/llvm/lib/Analysis/Analysis.cpp
+++ b/llvm/lib/Analysis/Analysis.cpp
@@ -48,7 +48,6 @@ void llvm::initializeAnalysis(PassRegistry &Registry) {
   initializeLazyBranchProbabilityInfoPassPass(Registry);
   initializeLazyBlockFrequencyInfoPassPass(Registry);
   initializeLazyValueInfoWrapperPassPass(Registry);
-  initializeLazyValueInfoPrinterPass(Registry);
   initializeLoopInfoWrapperPassPass(Registry);
   initializeMemoryDependenceWrapperPassPass(Registry);
   initializeModuleSummaryIndexWrapperPassPass(Registry);
diff --git a/llvm/lib/Analysis/LazyValueInfo.cpp b/llvm/lib/Analysis/LazyValueInfo.cpp
index 5cb207c8036d40a..d4677b8ed444398 100644
--- a/llvm/lib/Analysis/LazyValueInfo.cpp
+++ b/llvm/lib/Analysis/LazyValueInfo.cpp
@@ -2086,37 +2086,3 @@ void LazyValueInfoAnnotatedWriter::emitInstructionAnnot(
         printResult(UseI->getParent());
 
 }
-
-namespace {
-// Printer class for LazyValueInfo results.
-class LazyValueInfoPrinter : public FunctionPass {
-public:
-  static char ID; // Pass identification, replacement for typeid
-  LazyValueInfoPrinter() : FunctionPass(ID) {
-    initializeLazyValueInfoPrinterPass(*PassRegistry::getPassRegistry());
-  }
-
-  void getAnalysisUsage(AnalysisUsage &AU) const override {
-    AU.setPreservesAll();
-    AU.addRequired<LazyValueInfoWrapperPass>();
-    AU.addRequired<DominatorTreeWrapperPass>();
-  }
-
-  // Get the mandatory dominator tree analysis and pass this in to the
-  // LVIPrinter. We cannot rely on the LVI's DT, since it's optional.
-  bool runOnFunction(Function &F) override {
-    dbgs() << "LVI for function '" << F.getName() << "':\n";
-    auto &LVI = getAnalysis<LazyValueInfoWrapperPass>().getLVI();
-    auto &DTree = getAnalysis<DominatorTreeWrapperPass>().getDomTree();
-    LVI.printLVI(F, DTree, dbgs());
-    return false;
-  }
-};
-}
-
-char LazyValueInfoPrinter::ID = 0;
-INITIALIZE_PASS_BEGIN(LazyValueInfoPrinter, "print-lazy-value-info",
-                "Lazy Value Info Printer Pass", false, false)
-INITIALIZE_PASS_DEPENDENCY(LazyValueInfoWrapperPass)
-INITIALIZE_PASS_END(LazyValueInfoPrinter, "print-lazy-value-info",
-                "Lazy Value Info Printer Pass", false, false)

@boomanaiden154
Copy link
Contributor Author

From what I can tell, it doesn't look like this pass has a NewPM equivalent yet, so maybe we want to port it first before removing the legacy pass?

Copy link

github-actions bot commented Nov 25, 2023

⚠️ C/C++ code formatter, clang-format found issues in your code. ⚠️

You can test this locally with the following command:
git-clang-format --diff c0fe0719df457b0992606b0f2a235719a5bbfd54 29cd5cb3a3c8452bc68e03ca9a76d4250cc8b47a -- llvm/include/llvm/InitializePasses.h llvm/lib/Analysis/Analysis.cpp llvm/lib/Analysis/LazyValueInfo.cpp
View the diff from clang-format here.
diff --git a/llvm/include/llvm/InitializePasses.h b/llvm/include/llvm/InitializePasses.h
index 66177f9b9f..19800fc8db 100644
--- a/llvm/include/llvm/InitializePasses.h
+++ b/llvm/include/llvm/InitializePasses.h
@@ -143,7 +143,7 @@ void initializeLCSSAVerificationPassPass(PassRegistry&);
 void initializeLCSSAWrapperPassPass(PassRegistry&);
 void initializeLazyBlockFrequencyInfoPassPass(PassRegistry&);
 void initializeLazyBranchProbabilityInfoPassPass(PassRegistry&);
-void initializeLazyMachineBlockFrequencyInfoPassPass(PassRegistry&);
+void initializeLazyMachineBlockFrequencyInfoPassPass(PassRegistry &);
 void initializeLazyValueInfoWrapperPassPass(PassRegistry&);
 void initializeLegacyLICMPassPass(PassRegistry&);
 void initializeLegalizerPass(PassRegistry&);

@nikic
Copy link
Contributor

nikic commented Nov 25, 2023

From what I can tell, it doesn't look like this pass has a NewPM equivalent yet, so maybe we want to port it first before removing the legacy pass?

Yes, it would be good to port the pass first.

@boomanaiden154
Copy link
Contributor Author

Yes, it would be good to port the pass first.

Sounds good. I'll open a patch porting it to the NewPM and get that landed first.

Copy link
Contributor

@nikic nikic left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

This pass isn't used anywhere upstream and thus has no test coverage.
For these reasons, remove it.
@boomanaiden154 boomanaiden154 merged commit 0bdb9cb into llvm:main Nov 27, 2023
1 of 3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants