[NewPM] Remove LowerWidenableConditionLegacyPass

[boomanaiden154]

This legacy pass isn't used anywhere upstream and thus has no test coverage, so remove it.

[llvmbot]

@llvm/pr-subscribers-llvm-transforms

Author: Aiden Grossman (boomanaiden154)

Changes

This legacy pass isn't used anywhere upstream and thus has no test coverage, so remove it.

---

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

4 Files Affected:

• (modified) llvm/include/llvm/InitializePasses.h (-1)
• (modified) llvm/include/llvm/Transforms/Scalar.h (-6)
• (modified) llvm/lib/Transforms/Scalar/LowerWidenableCondition.cpp (-27)
• (modified) llvm/lib/Transforms/Scalar/Scalar.cpp (-1)

diff --git a/llvm/include/llvm/InitializePasses.h b/llvm/include/llvm/InitializePasses.h
index da38a38b87ebc4d..9299b8e3c7183bd 100644
--- a/llvm/include/llvm/InitializePasses.h
+++ b/llvm/include/llvm/InitializePasses.h
@@ -189,7 +189,6 @@ void initializeLowerAtomicLegacyPassPass(PassRegistry&);
 void initializeLowerConstantIntrinsicsPass(PassRegistry&);
 void initializeLowerEmuTLSPass(PassRegistry&);
 void initializeLowerGlobalDtorsLegacyPassPass(PassRegistry &);
-void initializeLowerWidenableConditionLegacyPassPass(PassRegistry&);
 void initializeLowerIntrinsicsPass(PassRegistry&);
 void initializeLowerInvokeLegacyPassPass(PassRegistry&);
 void initializeLowerSwitchLegacyPassPass(PassRegistry &);
diff --git a/llvm/include/llvm/Transforms/Scalar.h b/llvm/include/llvm/Transforms/Scalar.h
index 0676f485f3fbcf4..a036314b382ceeb 100644
--- a/llvm/include/llvm/Transforms/Scalar.h
+++ b/llvm/include/llvm/Transforms/Scalar.h
@@ -195,12 +195,6 @@ FunctionPass *createSinkingPass();
 //
 Pass *createLowerAtomicPass();
 
-//===----------------------------------------------------------------------===//
-//
-// LowerWidenableCondition - Lower widenable condition to i1 true.
-//
-Pass *createLowerWidenableConditionPass();
-
 //===----------------------------------------------------------------------===//
 //
 // MergeICmps - Merge integer comparison chains into a memcmp
diff --git a/llvm/lib/Transforms/Scalar/LowerWidenableCondition.cpp b/llvm/lib/Transforms/Scalar/LowerWidenableCondition.cpp
index e2de322933bc1ba..3c977b816a05067 100644
--- a/llvm/lib/Transforms/Scalar/LowerWidenableCondition.cpp
+++ b/llvm/lib/Transforms/Scalar/LowerWidenableCondition.cpp
@@ -19,24 +19,10 @@
 #include "llvm/IR/Intrinsics.h"
 #include "llvm/IR/Module.h"
 #include "llvm/IR/PatternMatch.h"
-#include "llvm/InitializePasses.h"
-#include "llvm/Pass.h"
 #include "llvm/Transforms/Scalar.h"
 
 using namespace llvm;
 
-namespace {
-struct LowerWidenableConditionLegacyPass : public FunctionPass {
-  static char ID;
-  LowerWidenableConditionLegacyPass() : FunctionPass(ID) {
-    initializeLowerWidenableConditionLegacyPassPass(
-        *PassRegistry::getPassRegistry());
-  }
-
-  bool runOnFunction(Function &F) override;
-};
-}
-
 static bool lowerWidenableCondition(Function &F) {
   // Check if we can cheaply rule out the possibility of not having any work to
   // do.
@@ -65,19 +51,6 @@ static bool lowerWidenableCondition(Function &F) {
   return true;
 }
 
-bool LowerWidenableConditionLegacyPass::runOnFunction(Function &F) {
-  return lowerWidenableCondition(F);
-}
-
-char LowerWidenableConditionLegacyPass::ID = 0;
-INITIALIZE_PASS(LowerWidenableConditionLegacyPass, "lower-widenable-condition",
-                "Lower the widenable condition to default true value", false,
-                false)
-
-Pass *llvm::createLowerWidenableConditionPass() {
-  return new LowerWidenableConditionLegacyPass();
-}
-
 PreservedAnalyses LowerWidenableConditionPass::run(Function &F,
                                                FunctionAnalysisManager &AM) {
   if (lowerWidenableCondition(F))
diff --git a/llvm/lib/Transforms/Scalar/Scalar.cpp b/llvm/lib/Transforms/Scalar/Scalar.cpp
index 11bb8e936b74e7c..d3bede56cd5e088 100644
--- a/llvm/lib/Transforms/Scalar/Scalar.cpp
+++ b/llvm/lib/Transforms/Scalar/Scalar.cpp
@@ -41,7 +41,6 @@ void llvm::initializeScalarOpts(PassRegistry &Registry) {
   initializeLoopUnrollPass(Registry);
   initializeLowerAtomicLegacyPassPass(Registry);
   initializeLowerConstantIntrinsicsPass(Registry);
-  initializeLowerWidenableConditionLegacyPassPass(Registry);
   initializeMergeICmpsLegacyPassPass(Registry);
   initializeMergedLoadStoreMotionLegacyPassPass(Registry);
   initializeNaryReassociateLegacyPassPass(Registry);

[boomanaiden154]

Not sure if this one would be good to land as the Lower* passes are used quite often in the Backend, and even though it's used upstream, it might b be used somewhere downstream?

[boomanaiden154]

This should be the last removal in the current iteration of legacy pass removals (other than maybe LoopGuardWideningLegacy). There are some other passes that look like they could be removed:

• CFGOnlyPrinterLegacy
• CFGOnlyViewerLegacy
• CFGViewerLegacy
• CallGraphPrinterLegacy
• InstCountLegacy
• LoopExtractorLegacy
• PlaceBackedgeSafepointsLegacy
• PredicateInfoPrinterLegacy
• RegToMemLegacy
• StripGCRelocatesLegacy

A lot of these are utility functions used manually with opt, so I'm not sure we should look into removing most of them yet. There are a couple in there that look like they just need a bit more untangling to remove.

[nikic]

LGTM

Not sure if this one would be good to land as the Lower* passes are used quite often in the Backend, and even though it's used upstream, it might b be used somewhere downstream?

Azul confirmed in https://reviews.llvm.org/D153677#4452433 that they're not using this in the backend, so I think we can go ahead and remove this. If it turns out that someone else is using it in the backend, we should revert the patch later.

[nikic]

A lot of these are utility functions used manually with opt, so I'm not sure we should look into removing most of them yet. There are a couple in there that look like they just need a bit more untangling to remove.

I think at least the printing passes should get migrated to print<foo> NewPM passes.

[boomanaiden154]

Azul confirmed in https://reviews.llvm.org/D153677#4452433 that they're not using this in the backend, so I think we can go ahead and remove this. If it turns out that someone else is using it in the backend, we should revert the patch later.

Sounds good. One of the main reasons I kept everything split up was to make reverts easy for cases like that.

I think at least the printing passes should get migrated to print<foo> NewPM passes.

Can you clarify what you mean here? Are you referring to adjusting PassRegistry.def to switch the print-* names to print<*> names?

And then the plan right now would be to keep the legacy versions of all these utility passes? Or should we look at removing them too?

[nikic]

Can you clarify what you mean here? Are you referring to adjusting PassRegistry.def to switch the print-* names to print<*> names?

And then the plan right now would be to keep the legacy versions of all these utility passes? Or should we look at removing them too?

Ah, if these already have NewPM variants, we should remove the legacy PM ones. I thought that some of the printing passes hadn't been ported to NewPM yet.

[boomanaiden154]

Ah, if these already have NewPM variants, we should remove the legacy PM ones. I thought that some of the printing passes hadn't been ported to NewPM yet.

Sounds good. I'll work on getting patches up to remove those as well.