[PATCH] D19938: [PM] Port ConstantMerge to the new pass manager

Justin Bogner via llvm-commits llvm-commits at lists.llvm.org
Wed May 4 17:02:38 PDT 2016


Davide Italiano <dccitaliano at gmail.com> writes:
> davide created this revision.
> davide added reviewers: bogner, chandlerc.
> davide added a subscriber: llvm-commits.
> Herald added a subscriber: joker.eph.
>
> This is a relatively straightforward port, but I'd love to get another
> pair of eyes on it in case I missed something

A couple of nitpicks below and this LGTM.

> http://reviews.llvm.org/D19938
>
> Files:
>   include/llvm/InitializePasses.h
>   include/llvm/Transforms/IPO/ConstantMerge.h
>   lib/LTO/LTOCodeGenerator.cpp
>   lib/Passes/PassBuilder.cpp
>   lib/Passes/PassRegistry.def
>   lib/Transforms/IPO/ConstantMerge.cpp
>   lib/Transforms/IPO/IPO.cpp
>   test/Transforms/ConstantMerge/merge-both.ll
>
> Index: test/Transforms/ConstantMerge/merge-both.ll
> ===================================================================
> --- test/Transforms/ConstantMerge/merge-both.ll
> +++ test/Transforms/ConstantMerge/merge-both.ll
> @@ -1,4 +1,4 @@
> -; RUN: opt -constmerge -S < %s | FileCheck %s
> +; RUN: opt -S < %s -passes=constmerge | FileCheck %s
>  ; Test that in one run var3 is merged into var2 and var1 into var4.
>  ; Test that we merge @var5 and @var6 into one with the higher alignment
>  
> Index: lib/Transforms/IPO/IPO.cpp
> ===================================================================
> --- lib/Transforms/IPO/IPO.cpp
> +++ lib/Transforms/IPO/IPO.cpp
> @@ -24,7 +24,7 @@
>  
>  void llvm::initializeIPO(PassRegistry &Registry) {
>    initializeArgPromotionPass(Registry);
> -  initializeConstantMergePass(Registry);
> +  initializeConstantMergeLegacyPassPass(Registry);
>    initializeCrossDSOCFIPass(Registry);
>    initializeDAEPass(Registry);
>    initializeDAHPass(Registry);
> Index: lib/Transforms/IPO/ConstantMerge.cpp
> ===================================================================
> --- lib/Transforms/IPO/ConstantMerge.cpp
> +++ lib/Transforms/IPO/ConstantMerge.cpp
> @@ -17,7 +17,7 @@
>  //
>  //===----------------------------------------------------------------------===//
>  
> -#include "llvm/Transforms/IPO.h"
> +#include "llvm/Transforms/IPO/ConstantMerge.h"
>  #include "llvm/ADT/DenseMap.h"
>  #include "llvm/ADT/PointerIntPair.h"
>  #include "llvm/ADT/SmallPtrSet.h"
> @@ -28,33 +28,13 @@
>  #include "llvm/IR/Module.h"
>  #include "llvm/IR/Operator.h"
>  #include "llvm/Pass.h"
> +#include "llvm/Transforms/IPO.h"
>  using namespace llvm;
>  
>  #define DEBUG_TYPE "constmerge"
>  
>  STATISTIC(NumMerged, "Number of global constants merged");
>  
> -namespace {
> -  struct ConstantMerge : public ModulePass {
> -    static char ID; // Pass identification, replacement for typeid
> -    ConstantMerge() : ModulePass(ID) {
> -      initializeConstantMergePass(*PassRegistry::getPassRegistry());
> -    }
> -
> -    // For this pass, process all of the globals in the module, eliminating
> -    // duplicate constants.
> -    bool runOnModule(Module &M) override;
> -  };
> -}
> -
> -char ConstantMerge::ID = 0;
> -INITIALIZE_PASS(ConstantMerge, "constmerge",
> -                "Merge Duplicate Global Constants", false, false)
> -
> -ModulePass *llvm::createConstantMergePass() { return new ConstantMerge(); }
> -
> -
> -
>  /// Find values that are marked as llvm.used.
>  static void FindUsedValues(GlobalVariable *LLVMUsed,
>                             SmallPtrSetImpl<const GlobalValue*> &UsedValues) {
> @@ -87,10 +67,7 @@
>    return GV->getParent()->getDataLayout().getPreferredAlignment(GV);
>  }
>  
> -bool ConstantMerge::runOnModule(Module &M) {
> -  if (skipModule(M))
> -    return false;
> -
> +static bool mergeConstants(Module &M) {
>    // Find all the globals that are marked "used".  These cannot be merged.
>    SmallPtrSet<const GlobalValue*, 8> UsedGlobals;
>    FindUsedValues(M.getGlobalVariable("llvm.used"), UsedGlobals);
> @@ -214,3 +191,35 @@
>      Replacements.clear();
>    }
>  }
> +
> +PreservedAnalyses ConstantMergePass::run(Module &M,
> +                                         AnalysisManager<Module> &AM) {

Might as well implement the version that doesn't even take an
AnalysisManager instead, like:

  PreservedAnalyses ConstantMergePass::run(Module &M) { ... }

> +  if (!mergeConstants(M))
> +    return PreservedAnalyses::all();
> +  return PreservedAnalyses::none();
> +}
> +
> +namespace {
> +struct ConstantMergeLegacyPass : public ModulePass {
> +  static char ID; // Pass identification, replacement for typeid
> +  ConstantMergeLegacyPass() : ModulePass(ID) {
> +    initializeConstantMergeLegacyPassPass(*PassRegistry::getPassRegistry());
> +  }
> +
> +  // For this pass, process all of the globals in the module, eliminating
> +  // duplicate constants.
> +  bool runOnModule(Module &M) {
> +    if (skipModule(M))
> +      return false;
> +    return mergeConstants(M);
> +  }
> +};
> +}
> +
> +char ConstantMergeLegacyPass::ID = 0;
> +INITIALIZE_PASS(ConstantMergeLegacyPass, "constmerge",
> +                "Merge Duplicate Global Constants", false, false)
> +
> +ModulePass *llvm::createConstantMergePass() {
> +  return new ConstantMergeLegacyPass();
> +}
> Index: lib/Passes/PassRegistry.def
> ===================================================================
> --- lib/Passes/PassRegistry.def
> +++ lib/Passes/PassRegistry.def
> @@ -35,6 +35,7 @@
>  #ifndef MODULE_PASS
>  #define MODULE_PASS(NAME, CREATE_PASS)
>  #endif
> +MODULE_PASS("constmerge", ConstantMergePass())
>  MODULE_PASS("forceattrs", ForceFunctionAttrsPass())
>  MODULE_PASS("globaldce", GlobalDCEPass())
>  MODULE_PASS("globalopt", GlobalOptPass())
> Index: lib/Passes/PassBuilder.cpp
> ===================================================================
> --- lib/Passes/PassBuilder.cpp
> +++ lib/Passes/PassBuilder.cpp
> @@ -45,6 +45,7 @@
>  #include "llvm/Support/Debug.h"
>  #include "llvm/Support/Regex.h"
>  #include "llvm/Target/TargetMachine.h"
> +#include "llvm/Transforms/IPO/ConstantMerge.h"
>  #include "llvm/Transforms/IPO/ForceFunctionAttrs.h"
>  #include "llvm/Transforms/IPO/FunctionAttrs.h"
>  #include "llvm/Transforms/IPO/GlobalDCE.h"
> @@ -57,10 +58,10 @@
>  #include "llvm/Transforms/Scalar/ADCE.h"
>  #include "llvm/Transforms/Scalar/DCE.h"
>  #include "llvm/Transforms/Scalar/EarlyCSE.h"
> +#include "llvm/Transforms/Scalar/GVN.h"
>  #include "llvm/Transforms/Scalar/LoopRotation.h"
>  #include "llvm/Transforms/Scalar/LoopSimplifyCFG.h"
>  #include "llvm/Transforms/Scalar/LowerExpectIntrinsic.h"
> -#include "llvm/Transforms/Scalar/GVN.h"
>  #include "llvm/Transforms/Scalar/Reassociate.h"
>  #include "llvm/Transforms/Scalar/SROA.h"
>  #include "llvm/Transforms/Scalar/SimplifyCFG.h"
> Index: lib/LTO/LTOCodeGenerator.cpp
> ===================================================================
> --- lib/LTO/LTOCodeGenerator.cpp
> +++ lib/LTO/LTOCodeGenerator.cpp
> @@ -99,7 +99,7 @@
>    initializeInternalizeLegacyPassPass(R);
>    initializeIPSCCPPass(R);
>    initializeGlobalOptLegacyPassPass(R);
> -  initializeConstantMergePass(R);
> +  initializeConstantMergeLegacyPassPass(R);
>    initializeDAHPass(R);
>    initializeInstructionCombiningPassPass(R);
>    initializeSimpleInlinerPass(R);
> Index: include/llvm/Transforms/IPO/ConstantMerge.h
> ===================================================================
> --- include/llvm/Transforms/IPO/ConstantMerge.h
> +++ include/llvm/Transforms/IPO/ConstantMerge.h
> @@ -0,0 +1,34 @@
> +//===--- ConstantMerge.h - Merge duplicate global constants ---------------===//

We usually put the little C++ marker in the header for editors to
magically work out the mode, ie:

//===- ConstantMerge.h - Merge duplicate global constants -------*- C++ -*-===//

> +//
> +//                     The LLVM Compiler Infrastructure
> +//
> +// This file is distributed under the University of Illinois Open Source
> +// License. See LICENSE.TXT for details.
> +//
> +//===----------------------------------------------------------------------===//
> +//
> +// This file defines the interface to a pass that merges duplicate global
> +// constants together into a single constant that is shared.  This is useful
> +// because some passes (ie TraceValues) insert a lot of string constants into
> +// the program, regardless of whether or not an existing string is available.
> +//
> +// Algorithm: ConstantMerge is designed to build up a map of available constants
> +// and eliminate duplicates when it is initialized.
> +//
> +//===----------------------------------------------------------------------===//
> +
> +#ifndef LLVM_TRANSFORMS_IPO_CONSTANTMERGE_H
> +#define LLVM_TRANSFORMS_IPO_CONSTANTMERGE_H
> +
> +#include "llvm/IR/Module.h"
> +#include "llvm/IR/PassManager.h"
> +
> +namespace llvm {
> +
> +class ConstantMergePass : public PassInfoMixin<ConstantMergePass> {

Throw a very basic doxygen comment on the class please.

> +public:
> +  PreservedAnalyses run(Module &M, AnalysisManager<Module> &AM);
> +};
> +}
> +
> +#endif // LLVM_TRANSFORMS_IPO_CONSTANTMERGE_H
> Index: include/llvm/InitializePasses.h
> ===================================================================
> --- include/llvm/InitializePasses.h
> +++ include/llvm/InitializePasses.h
> @@ -96,7 +96,7 @@
>  void initializeCFGViewerPass(PassRegistry&);
>  void initializeConstantHoistingPass(PassRegistry&);
>  void initializeCodeGenPreparePass(PassRegistry&);
> -void initializeConstantMergePass(PassRegistry&);
> +void initializeConstantMergeLegacyPassPass(PassRegistry &);
>  void initializeConstantPropagationPass(PassRegistry&);
>  void initializeMachineCopyPropagationPass(PassRegistry&);
>  void initializeCostModelAnalysisPass(PassRegistry&);


More information about the llvm-commits mailing list