[LegacyPM] Remove RewriteSymbolsLegacyPass

[nikic]

LGTM

[srj]

Is there any way to get this functionality without the LegacyPassManager?

Halide still relies on the LegacyPassManager for certain codegen paths, because (AFAIK) there is still no complete API to replace it with the newer PassManager. Our team would very much appreciate any pointers to how to modernize our usage in advance of Legacy API removal.

[nikic]

@srj Could you please share how you're currently using the pass? It's possible we just have to revert this, but hard to say without code.

[srj]

In D153679#4450033, @nikic wrote:

@srj Could you please share how you're currently using the pass? It's possible we just have to revert this, but hard to say without code.

See the function emit_file here: https://github.com/halide/Halide/blob/main/src/LLVM_Output.cpp#L341

[nikic]

In D153679#4450135, @srj wrote:

In D153679#4450033, @nikic wrote:

@srj Could you please share how you're currently using the pass? It's possible we just have to revert this, but hard to say without code.

See the function emit_file here: https://github.com/halide/Halide/blob/main/src/LLVM_Output.cpp#L341

Thanks! You could run just that one pass separately with the NewPM, but that probably doesn't make a lot of sense.

I think we should revert this change for now and keep the pass until we have NewPM support for the codegen pipeline. @aeubanks What do you think?

[srj]

In D153679#4450286, @nikic wrote:

I think we should revert this change for now and keep the pass until we have NewPM support for the codegen pipeline. @aeubanks What do you think?

Halide contributor Andrew Adams commented elsewhere: "It's an old commit. Hard to say if anyone is using the feature that this PR removes." I suspect that we can live without it until the entire API migration is done. (Is there an ETA for that? We've been hearing for years that it will happen...)

[aeubanks]

we could revert, but as you've alluded to I doubt that halide users are actually using this pass, for example I removed it from LLVM in https://reviews.llvm.org/D99707 without complaint

I'd remove the halide use and see if anybody complains, and if somebody does we can revert this patch

you could also add the corresponding new PM pass to the optimization pipeline that halide runs

llvm/include/llvm/InitializePasses.h

 void initializeRegionOnlyViewerPass(PassRegistry&);
 void initializeRegionPrinterPass(PassRegistry&);
 void initializeRegionViewerPass(PassRegistry&);
 void initializeRegisterCoalescerPass(PassRegistry&);
 void initializeRemoveRedundantDebugValuesPass(PassRegistry&);
 void initializeRenameIndependentSubregsPass(PassRegistry&);
 void initializeReplaceWithVeclibLegacyPass(PassRegistry &);
 void initializeResetMachineFunctionPass(PassRegistry&);
-void initializeRewriteSymbolsLegacyPassPass(PassRegistry&);
 void initializeSCEVAAWrapperPassPass(PassRegistry&);
 void initializeSROALegacyPassPass(PassRegistry&);
 void initializeSafeStackLegacyPassPass(PassRegistry&);
 void initializeSafepointIRVerifierPass(PassRegistry&);
 void initializeSelectOptimizePass(PassRegistry &);
 void initializeScalarEvolutionWrapperPassPass(PassRegistry&);
 void initializeScalarizeMaskedMemIntrinLegacyPassPass(PassRegistry &);
 void initializeScalarizerLegacyPassPass(PassRegistry&);

llvm/include/llvm/LinkAllPasses.h

ForcePassLinking() {
       (void) llvm::createSinkingPass();
       (void) llvm::createLowerAtomicPass();
       (void) llvm::createLoadStoreVectorizerPass();
       (void) llvm::createPartiallyInlineLibCallsPass();
       (void) llvm::createScalarizerPass();
       (void) llvm::createSeparateConstOffsetFromGEPPass();
       (void) llvm::createSpeculativeExecutionPass();
       (void) llvm::createSpeculativeExecutionIfHasBranchDivergencePass();
-      (void) llvm::createRewriteSymbolsPass();
       (void) llvm::createStraightLineStrengthReducePass();
       (void)llvm::createScalarizeMaskedMemIntrinLegacyPass();
       (void) llvm::createHardwareLoopsLegacyPass();
       (void) llvm::createUnifyLoopExitsPass();
       (void) llvm::createFixIrreduciblePass();
       (void)llvm::createSelectOptimizePass();
 
       (void)new llvm::IntervalPartition();

llvm/include/llvm/Transforms/Utils/SymbolRewriter.h

bool parseRewriteGlobalVariableDescriptor(yaml::Stream &Stream,
                                             RewriteDescriptorList *DL);
   bool parseRewriteGlobalAliasDescriptor(yaml::Stream &YS, yaml::ScalarNode *K,
                                          yaml::MappingNode *V,
                                          RewriteDescriptorList *DL);
 };
 
 } // end namespace SymbolRewriter
 
-ModulePass *createRewriteSymbolsPass();
-ModulePass *createRewriteSymbolsPass(SymbolRewriter::RewriteDescriptorList &);
-
 class RewriteSymbolPass : public PassInfoMixin<RewriteSymbolPass> {
 public:
   RewriteSymbolPass() { loadAndParseMapFiles(); }
 
   RewriteSymbolPass(SymbolRewriter::RewriteDescriptorList &DL) {
     Descriptors.splice(Descriptors.begin(), DL);
   }
 

llvm/lib/Transforms/Utils/SymbolRewriter.cpp

DL->push_back(std::make_unique<ExplicitRewriteNamedAliasDescriptor>(
         /*Naked*/ false));
   else
     DL->push_back(std::make_unique<PatternRewriteNamedAliasDescriptor>(
         Source, Transform));
 
   return true;
 }
 
-namespace {
-
-class RewriteSymbolsLegacyPass : public ModulePass {
-public:
-  static char ID; // Pass identification, replacement for typeid
-
-  RewriteSymbolsLegacyPass();
-  RewriteSymbolsLegacyPass(SymbolRewriter::RewriteDescriptorList &DL);
-
-  bool runOnModule(Module &M) override;
-
-private:
-  RewriteSymbolPass Impl;
-};
-
-} // end anonymous namespace
-
-char RewriteSymbolsLegacyPass::ID = 0;
-
-RewriteSymbolsLegacyPass::RewriteSymbolsLegacyPass() : ModulePass(ID) {
-  initializeRewriteSymbolsLegacyPassPass(*PassRegistry::getPassRegistry());
-}
-
-RewriteSymbolsLegacyPass::RewriteSymbolsLegacyPass(
-    SymbolRewriter::RewriteDescriptorList &DL)
-    : ModulePass(ID), Impl(DL) {}
-
-bool RewriteSymbolsLegacyPass::runOnModule(Module &M) {
-  return Impl.runImpl(M);
-}
-
 PreservedAnalyses RewriteSymbolPass::run(Module &M, ModuleAnalysisManager &AM) {
   if (!runImpl(M))
     return PreservedAnalyses::all();
 
   return PreservedAnalyses::none();
 }
 
 bool RewriteSymbolPass::runImpl(Module &M) {
   bool Changed;
 
   Changed = false;
   for (auto &Descriptor : Descriptors)
     Changed |= Descriptor->performOnModule(M);
 
   return Changed;
 }
 
 void RewriteSymbolPass::loadAndParseMapFiles() {
   const std::vector<std::string> MapFiles(RewriteMapFiles);
   SymbolRewriter::RewriteMapParser Parser;
 
   for (const auto &MapFile : MapFiles)
     Parser.parse(MapFile, &Descriptors);
 }
-
-INITIALIZE_PASS(RewriteSymbolsLegacyPass, "rewrite-symbols", "Rewrite Symbols",
-                false, false)
-
-ModulePass *llvm::createRewriteSymbolsPass() {
-  return new RewriteSymbolsLegacyPass();
-}
-
-ModulePass *
-llvm::createRewriteSymbolsPass(SymbolRewriter::RewriteDescriptorList &DL) {
-  return new RewriteSymbolsLegacyPass(DL);
-}

llvm/tools/opt/opt.cpp

int main(int argc, char **argv) {
   initializeExpandLargeDivRemLegacyPassPass(Registry);
   initializeExpandLargeFpConvertLegacyPassPass(Registry);
   initializeExpandMemCmpPassPass(Registry);
   initializeScalarizeMaskedMemIntrinLegacyPassPass(Registry);
   initializeSelectOptimizePass(Registry);
   initializeCallBrPreparePass(Registry);
   initializeCodeGenPreparePass(Registry);
   initializeAtomicExpandPass(Registry);
-  initializeRewriteSymbolsLegacyPassPass(Registry);
   initializeWinEHPreparePass(Registry);
   initializeDwarfEHPrepareLegacyPassPass(Registry);
   initializeSafeStackLegacyPassPass(Registry);
   initializeSjLjEHPreparePass(Registry);
   initializePreISelIntrinsicLoweringLegacyPassPass(Registry);
   initializeGlobalMergePass(Registry);
   initializeIndirectBrExpandPassPass(Registry);
   initializeInterleavedLoadCombinePass(Registry);