[PM] Port SymbolRewriter to the new PM

[mkuper]

Summary

There are two things that potentially worry me about this port:

1. Is this still the way we want to handle non-default constructors?
2. I've converted the iplist into a std::list<unique_ptr>, because iplist is not moveable. Does this seem reasonable?

[davide]

+ Duncan for the iplist bits.

[mkuper]

lib/Transforms/Utils/SymbolRewriter.cpp

72

We don't actually need this include, please ignore.

[mkuper]

Thanks, Duncan!

Committed the iplist change separately, so the only thing that I'm concerned about now is the handling of --rewrite-map-file.
Is this still the way we want this to work with the new PM?

[davide]

LGTM

[davide]

If we want to change the handling of -rewrite-file , fine, but that can (and should) be done separately.

include/llvm/InitializePasses.h

 void initializeRegionOnlyPrinterPass(PassRegistry&);
 void initializeRegionOnlyViewerPass(PassRegistry&);
 void initializeRegionPrinterPass(PassRegistry&);
 void initializeRegionViewerPass(PassRegistry&);
 void initializeRegisterCoalescerPass(PassRegistry&);
 void initializeRenameIndependentSubregsPass(PassRegistry&);
 void initializeReversePostOrderFunctionAttrsLegacyPassPass(PassRegistry&);
 void initializeRewriteStatepointsForGCPass(PassRegistry&);
-void initializeRewriteSymbolsPass(PassRegistry&);
+void initializeRewriteSymbolsLegacyPassPass(PassRegistry&);
 void initializeSCCPLegacyPassPass(PassRegistry &);
 void initializeSCEVAAWrapperPassPass(PassRegistry&);
 void initializeSLPVectorizerPass(PassRegistry&);
 void initializeSROALegacyPassPass(PassRegistry&);
 void initializeSafeStackPass(PassRegistry&);
 void initializeSampleProfileLoaderLegacyPassPass(PassRegistry&);
 void initializeSanitizerCoverageModulePass(PassRegistry&);
 void initializeScalarEvolutionWrapperPassPass(PassRegistry&);

include/llvm/Transforms/Utils/SymbolRewriter.h

 //
 // This makes it possible to cleanly transform symbols without resorting to
 // overly-complex macro tricks and the pre-processor.  An example of where this
 // is useful is the sanitizers where we would like to intercept a well-defined
 // set of functions across the module.
 //
 //===----------------------------------------------------------------------===//
 
-#ifndef LLVM_TRANSFORMS_UTILS_SYMBOL_REWRITER_H
-#define LLVM_TRANSFORMS_UTILS_SYMBOL_REWRITER_H
+#ifndef LLVM_TRANSFORMS_UTILS_SYMBOLREWRITER_H
+#define LLVM_TRANSFORMS_UTILS_SYMBOLREWRITER_H
 
-#include "llvm/ADT/ilist.h"
-#include "llvm/ADT/ilist_node.h"
 #include "llvm/IR/Module.h"
+#include "llvm/IR/PassManager.h"
+#include <list>
 
 namespace llvm {
 class MemoryBuffer;
 
 namespace yaml {
 class KeyValueNode;
 class MappingNode;
 class ScalarNode;
 class Stream;
 }
 
 namespace SymbolRewriter {
 /// The basic entity representing a rewrite operation.  It serves as the base
 /// class for any rewrite descriptor.  It has a certain set of specializations
 /// which describe a particular rewrite.
 ///
 /// The RewriteMapParser can be used to parse a mapping file that provides the
 /// mapping for rewriting the symbols.  The descriptors individually describe
 /// whether to rewrite a function, global variable, or global alias.  Each of
 /// these can be selected either by explicitly providing a name for the ones to
 /// be rewritten or providing a (posix compatible) regular expression that will
 /// select the symbols to rewrite.  This descriptor list is passed to the
 /// SymbolRewriter pass.
-class RewriteDescriptor : public ilist_node<RewriteDescriptor> {
+class RewriteDescriptor {
   RewriteDescriptor(const RewriteDescriptor &) = delete;
 
   const RewriteDescriptor &
   operator=(const RewriteDescriptor &) = delete;
 
 public:
   enum class Type {
     Invalid,        /// invalid

 
 protected:
   explicit RewriteDescriptor(Type T) : Kind(T) {}
 
 private:
   const Type Kind;
 };
 
-typedef iplist<RewriteDescriptor> RewriteDescriptorList;
+typedef std::list<std::unique_ptr<RewriteDescriptor>> RewriteDescriptorList;
 
 class RewriteMapParser {
 public:
   bool parse(const std::string &MapFile, RewriteDescriptorList *Descriptors);
 
 private:
   bool parse(std::unique_ptr<MemoryBuffer> &MapFile, RewriteDescriptorList *DL);
   bool parseEntry(yaml::Stream &Stream, yaml::KeyValueNode &Entry,
                   RewriteDescriptorList *DL);
   bool parseRewriteFunctionDescriptor(yaml::Stream &Stream,
                                       yaml::ScalarNode *Key,
                                       yaml::MappingNode *Value,
                                       RewriteDescriptorList *DL);
   bool parseRewriteGlobalVariableDescriptor(yaml::Stream &Stream,
                                             yaml::ScalarNode *Key,
                                             yaml::MappingNode *Value,
                                             RewriteDescriptorList *DL);
   bool parseRewriteGlobalAliasDescriptor(yaml::Stream &YS, yaml::ScalarNode *K,
                                          yaml::MappingNode *V,
                                          RewriteDescriptorList *DL);
 };
 }
 
-template <>
-struct ilist_traits<SymbolRewriter::RewriteDescriptor>
-    : public ilist_default_traits<SymbolRewriter::RewriteDescriptor> {
-  mutable ilist_half_node<SymbolRewriter::RewriteDescriptor> Sentinel;
+ModulePass *createRewriteSymbolsPass();
+ModulePass *createRewriteSymbolsPass(SymbolRewriter::RewriteDescriptorList &);
 
+class RewriteSymbolPass : public PassInfoMixin<RewriteSymbolPass> {
 public:
-  // createSentinel is used to get a reference to a node marking the end of
-  // the list.  Because the sentinel is relative to this instance, use a
-  // non-static method.
-  SymbolRewriter::RewriteDescriptor *createSentinel() const {
-    // since i[p] lists always publicly derive from the corresponding
-    // traits, placing a data member in this class will augment the
-    // i[p]list.  Since the NodeTy is expected to publicly derive from
-    // ilist_node<NodeTy>, there is a legal viable downcast from it to
-    // NodeTy.  We use this trick to superpose i[p]list with a "ghostly"
-    // NodeTy, which becomes the sentinel.  Dereferencing the sentinel is
-    // forbidden (save the ilist_node<NodeTy>) so no one will ever notice
-    // the superposition.
-    return static_cast<SymbolRewriter::RewriteDescriptor *>(&Sentinel);
+  RewriteSymbolPass() { loadAndParseMapFiles(); }
+  RewriteSymbolPass(SymbolRewriter::RewriteDescriptorList &DL) {
+    Descriptors.splice(Descriptors.begin(), DL);
   }
-  void destroySentinel(SymbolRewriter::RewriteDescriptor *) {}
 
-  SymbolRewriter::RewriteDescriptor *provideInitialHead() const {
-    return createSentinel();
-  }
+  PreservedAnalyses run(Module &M, ModuleAnalysisManager &AM);
 
-  SymbolRewriter::RewriteDescriptor *
-  ensureHead(SymbolRewriter::RewriteDescriptor *&) const {
-    return createSentinel();
-  }
+  // Glue for old PM
+  bool runImpl(Module &M);
 
-  static void noteHead(SymbolRewriter::RewriteDescriptor *,
-                       SymbolRewriter::RewriteDescriptor *) {}
+private:
+  void loadAndParseMapFiles();
+
+  SymbolRewriter::RewriteDescriptorList Descriptors;  
 };
 
-ModulePass *createRewriteSymbolsPass();
-ModulePass *createRewriteSymbolsPass(SymbolRewriter::RewriteDescriptorList &);
 }
 
-#endif
+#endif //LLVM_TRANSFORMS_UTILS_SYMBOLREWRITER_H

lib/Passes/PassBuilder.cpp

 #include "llvm/Transforms/Scalar/Sink.h"
 #include "llvm/Transforms/Scalar/TailRecursionElimination.h"
 #include "llvm/Transforms/Utils/AddDiscriminators.h"
 #include "llvm/Transforms/Utils/LCSSA.h"
 #include "llvm/Transforms/Utils/LoopSimplify.h"
 #include "llvm/Transforms/Utils/Mem2Reg.h"
 #include "llvm/Transforms/Utils/MemorySSA.h"
 #include "llvm/Transforms/Utils/SimplifyInstructions.h"
+#include "llvm/Transforms/Utils/SymbolRewriter.h"
 #include "llvm/Transforms/Vectorize/LoopVectorize.h"
 #include "llvm/Transforms/Vectorize/SLPVectorizer.h"
 
 #include <type_traits>
 
 using namespace llvm;
 
 static Regex DefaultAliasRegex("^(default|lto-pre-link|lto)<(O[0123sz])>$");

lib/Passes/PassRegistry.def

 MODULE_PASS("pgo-instr-gen", PGOInstrumentationGen())
 MODULE_PASS("pgo-instr-use", PGOInstrumentationUse())
 MODULE_PASS("pre-isel-intrinsic-lowering", PreISelIntrinsicLoweringPass())
 MODULE_PASS("print-profile-summary", ProfileSummaryPrinterPass(dbgs()))
 MODULE_PASS("print-callgraph", CallGraphPrinterPass(dbgs()))
 MODULE_PASS("print", PrintModulePass(dbgs()))
 MODULE_PASS("print-lcg", LazyCallGraphPrinterPass(dbgs()))
 MODULE_PASS("print-lcg-dot", LazyCallGraphDOTPrinterPass(dbgs()))
+MODULE_PASS("rewrite-symbols", RewriteSymbolPass())
 MODULE_PASS("rpo-functionattrs", ReversePostOrderFunctionAttrsPass())
 MODULE_PASS("sample-profile", SampleProfileLoaderPass())
 MODULE_PASS("strip-dead-prototypes", StripDeadPrototypesPass())
 MODULE_PASS("wholeprogramdevirt", WholeProgramDevirtPass())
 MODULE_PASS("verify", VerifierPass())
 #undef MODULE_PASS
 
 #ifndef CGSCC_ANALYSIS

lib/Transforms/Utils/SymbolRewriter.cpp

 //
 //  Specify to rewrite the symbols using the `-rewrite-symbols` option, and
 //  specify the map file to use for the rewriting via the `-rewrite-map-file`
 //  option.
 //
 //===----------------------------------------------------------------------===//
 
 #define DEBUG_TYPE "symbol-rewriter"
+#include "llvm/Transforms/Utils/SymbolRewriter.h"
 #include "llvm/Pass.h"
 #include "llvm/ADT/SmallString.h"
 #include "llvm/IR/LegacyPassManager.h"
 #include "llvm/Support/CommandLine.h"
 #include "llvm/Support/Debug.h"
 #include "llvm/Support/MemoryBuffer.h"
 #include "llvm/Support/Regex.h"
 #include "llvm/Support/SourceMgr.h"
 #include "llvm/Support/YAMLParser.h"
 #include "llvm/Support/raw_ostream.h"
-#include "llvm/Transforms/Utils/SymbolRewriter.h"
+#include <memory>
 
 using namespace llvm;
 using namespace SymbolRewriter;
 
 static cl::list<std::string> RewriteMapFiles("rewrite-map-file",
                                              cl::desc("Symbol Rewrite Map"),
                                              cl::value_desc("filename"));
 
if (Transform.empty() == Target.empty()) {
     YS.printError(Descriptor,
                   "exactly one of transform or target must be specified");
     return false;
   }
 
   // TODO see if there is a more elegant solution to selecting the rewrite
   // descriptor type
   if (!Target.empty())
-    DL->push_back(new ExplicitRewriteFunctionDescriptor(Source, Target, Naked));
+    DL->push_back(
+        make_unique<ExplicitRewriteFunctionDescriptor>(Source, Target, Naked));
   else
-    DL->push_back(new PatternRewriteFunctionDescriptor(Source, Transform));
+    DL->push_back(
+        make_unique<PatternRewriteFunctionDescriptor>(Source, Transform));
 
   return true;
 }
 
 bool RewriteMapParser::
 parseRewriteGlobalVariableDescriptor(yaml::Stream &YS, yaml::ScalarNode *K,
                                      yaml::MappingNode *Descriptor,
                                      RewriteDescriptorList *DL) {
parseRewriteGlobalVariableDescriptor(yaml::Stream &YS, yaml::ScalarNode *K,
 
   if (Transform.empty() == Target.empty()) {
     YS.printError(Descriptor,
                   "exactly one of transform or target must be specified");
     return false;
   }
 
   if (!Target.empty())
-    DL->push_back(new ExplicitRewriteGlobalVariableDescriptor(Source, Target,
-                                                              /*Naked*/false));
+    DL->push_back(
+        make_unique<ExplicitRewriteGlobalVariableDescriptor>(Source, Target,
+                                                             /*Naked*/ false));
   else
-    DL->push_back(new PatternRewriteGlobalVariableDescriptor(Source,
-                                                             Transform));
+    DL->push_back(
+        make_unique<PatternRewriteGlobalVariableDescriptor>(Source, Transform));
 
   return true;
 }
 
 bool RewriteMapParser::
 parseRewriteGlobalAliasDescriptor(yaml::Stream &YS, yaml::ScalarNode *K,
                                   yaml::MappingNode *Descriptor,
                                   RewriteDescriptorList *DL) {
parseRewriteGlobalAliasDescriptor(yaml::Stream &YS, yaml::ScalarNode *K,
 
   if (Transform.empty() == Target.empty()) {
     YS.printError(Descriptor,
                   "exactly one of transform or target must be specified");
     return false;
   }
 
   if (!Target.empty())
-    DL->push_back(new ExplicitRewriteNamedAliasDescriptor(Source, Target,
-                                                          /*Naked*/false));
+    DL->push_back(
+        make_unique<ExplicitRewriteNamedAliasDescriptor>(Source, Target,
+                                                         /*Naked*/ false));
   else
-    DL->push_back(new PatternRewriteNamedAliasDescriptor(Source, Transform));
+    DL->push_back(
+        make_unique<PatternRewriteNamedAliasDescriptor>(Source, Transform));
 
   return true;
 }
 
 namespace {
-class RewriteSymbols : public ModulePass {
+class RewriteSymbolsLegacyPass : public ModulePass {
 public:
   static char ID; // Pass identification, replacement for typeid
 
-  RewriteSymbols();
-  RewriteSymbols(SymbolRewriter::RewriteDescriptorList &DL);
+  RewriteSymbolsLegacyPass();
+  RewriteSymbolsLegacyPass(SymbolRewriter::RewriteDescriptorList &DL);
 
   bool runOnModule(Module &M) override;
 
 private:
-  void loadAndParseMapFiles();
-
-  SymbolRewriter::RewriteDescriptorList Descriptors;
+  RewriteSymbolPass Impl;
 };
 
-char RewriteSymbols::ID = 0;
+char RewriteSymbolsLegacyPass::ID = 0;
 
-RewriteSymbols::RewriteSymbols() : ModulePass(ID) {
-  initializeRewriteSymbolsPass(*PassRegistry::getPassRegistry());
-  loadAndParseMapFiles();
+RewriteSymbolsLegacyPass::RewriteSymbolsLegacyPass() : ModulePass(ID), Impl() {
+  initializeRewriteSymbolsLegacyPassPass(*PassRegistry::getPassRegistry());  
 }
 
-RewriteSymbols::RewriteSymbols(SymbolRewriter::RewriteDescriptorList &DL)
-    : ModulePass(ID) {
-  Descriptors.splice(Descriptors.begin(), DL);
+RewriteSymbolsLegacyPass::RewriteSymbolsLegacyPass(
+    SymbolRewriter::RewriteDescriptorList &DL)
+    : ModulePass(ID), Impl(DL) {}
+
+bool RewriteSymbolsLegacyPass::runOnModule(Module &M) {
+  return Impl.runImpl(M);
+}
 }
 
-bool RewriteSymbols::runOnModule(Module &M) {
+namespace llvm {
+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);
+    Changed |= Descriptor->performOnModule(M);
 
   return Changed;
 }
 
-void RewriteSymbols::loadAndParseMapFiles() {
+void RewriteSymbolPass::loadAndParseMapFiles() {
   const std::vector<std::string> MapFiles(RewriteMapFiles);
-  SymbolRewriter::RewriteMapParser parser;
+  SymbolRewriter::RewriteMapParser Parser;
 
   for (const auto &MapFile : MapFiles)
-    parser.parse(MapFile, &Descriptors);
+    Parser.parse(MapFile, &Descriptors);
 }
 }
 
-INITIALIZE_PASS(RewriteSymbols, "rewrite-symbols", "Rewrite Symbols", false,
-                false)
+INITIALIZE_PASS(RewriteSymbolsLegacyPass, "rewrite-symbols", "Rewrite Symbols",
+                false, false)
 
-ModulePass *llvm::createRewriteSymbolsPass() { return new RewriteSymbols(); }
+ModulePass *llvm::createRewriteSymbolsPass() {
+  return new RewriteSymbolsLegacyPass();
+}
 
 ModulePass *
 llvm::createRewriteSymbolsPass(SymbolRewriter::RewriteDescriptorList &DL) {
-  return new RewriteSymbols(DL);
+  return new RewriteSymbolsLegacyPass(DL);
 }

test/SymbolRewriter/rewrite.ll

 ; RUN: opt -mtriple i686-win32 -rewrite-symbols -rewrite-map-file %p/rewrite.map \
 ; RUN:   %s -o - | llvm-dis | FileCheck %s
+; RUN: opt -mtriple i686-win32 -passes='rewrite-symbols' -rewrite-map-file %p/rewrite.map \
+; RUN:   %s -o - | llvm-dis | FileCheck %s
 
 declare void @source_function()
 @source_variable = external global i32
 declare void @source_function_pattern_function()
 declare void @source_function_pattern_multiple_function_matches()
 @source_variable_pattern_variable = external global i32
 @source_variable_pattern_multiple_variable_matches = external global i32
 declare void @"\01naked_source_function"()

tools/opt/opt.cpp

int main(int argc, char **argv) {
   initializeTransformUtils(Registry);
   initializeInstCombine(Registry);
   initializeInstrumentation(Registry);
   initializeTarget(Registry);
   // For codegen passes, only passes that do IR to IR transformation are
   // supported.
   initializeCodeGenPreparePass(Registry);
   initializeAtomicExpandPass(Registry);
-  initializeRewriteSymbolsPass(Registry);
+  initializeRewriteSymbolsLegacyPassPass(Registry);
   initializeWinEHPreparePass(Registry);
   initializeDwarfEHPreparePass(Registry);
   initializeSafeStackPass(Registry);
   initializeSjLjEHPreparePass(Registry);
   initializePreISelIntrinsicLoweringLegacyPassPass(Registry);
   initializeGlobalMergePass(Registry);
   initializeInterleavedAccessPass(Registry);
   initializeUnreachableBlockElimLegacyPassPass(Registry);