[llvm] r226987 - [PM] Port instcombine to the new pass manager!
Patrik Hägglund H
patrik.h.hagglund at ericsson.com
Sat Jan 24 12:50:08 PST 2015
I added LLVM_LIBRARY_VISIBILITY to InstCombinePass in r227013, but revert it due to http://lab.llvm.org:8011/builders/clang-hexagon-elf/builds/21749. Benjamin pointed out the cause in another mail. Can any of you provide a more correct fix?
/Patrik Hägglund
-----Original Message-----
From: llvm-commits-bounces at cs.uiuc.edu [mailto:llvm-commits-bounces at cs.uiuc.edu] On Behalf Of Aaron Ballman
Sent: den 24 januari 2015 18:08
To: Chandler Carruth
Cc: llvm-commits
Subject: Re: [llvm] r226987 - [PM] Port instcombine to the new pass manager!
This commit is causing some new warnings to be triggered:
/opt/llvm/build-llvm/llvm/include/llvm/Transforms/InstCombine/InstCombine.h:27:7:
warning: ‘llvm::InstCombinePass’ declared with greater visibility than
the type of its field ‘llvm::InstCombinePass::Worklist’ [-Wattributes]
I noticed that InstCombineWorklist has LLVM_LIBRARY_VISIBILITY but
InstCombinePass does not. Perhaps InstcombinePass should receive that
attribute as well?
~Aaron
On Fri, Jan 23, 2015 at 11:19 PM, Chandler Carruth <chandlerc at gmail.com> wrote:
> Author: chandlerc
> Date: Fri Jan 23 22:19:17 2015
> New Revision: 226987
>
> URL: http://llvm.org/viewvc/llvm-project?rev=226987&view=rev
> Log:
> [PM] Port instcombine to the new pass manager!
>
> This is exciting as this is a much more involved port. This is
> a complex, existing transformation pass. All of the core logic is shared
> between both old and new pass managers. Only the access to the analyses
> is separate because the actual techniques are separate. This also uses
> a bunch of different and interesting analyses and is the first time
> where we need to use an analysis across an IR layer.
>
> This also paves the way to expose instcombine utility functions. I've
> got a static function that implements the core pass logic over
> a function which might be mildly interesting, but more interesting is
> likely exposing a routine which just uses instructions *already in* the
> worklist and combines until empty.
>
> I've switched one of my favorite instcombine tests to run with both as
> well to make sure this keeps working.
>
> Added:
> llvm/trunk/include/llvm/Transforms/InstCombine/
> llvm/trunk/include/llvm/Transforms/InstCombine/InstCombine.h
> llvm/trunk/include/llvm/Transforms/InstCombine/InstCombineWorklist.h
> - copied, changed from r226981, llvm/trunk/lib/Transforms/InstCombine/InstCombineWorklist.h
> Removed:
> llvm/trunk/lib/Transforms/InstCombine/InstCombineWorklist.h
> Modified:
> llvm/trunk/lib/Transforms/InstCombine/InstCombineInternal.h
> llvm/trunk/lib/Transforms/InstCombine/InstructionCombining.cpp
> llvm/trunk/test/Transforms/InstCombine/load.ll
> llvm/trunk/tools/opt/PassRegistry.def
> llvm/trunk/tools/opt/Passes.cpp
>
> Added: llvm/trunk/include/llvm/Transforms/InstCombine/InstCombine.h
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/Transforms/InstCombine/InstCombine.h?rev=226987&view=auto
> ==============================================================================
> --- llvm/trunk/include/llvm/Transforms/InstCombine/InstCombine.h (added)
> +++ llvm/trunk/include/llvm/Transforms/InstCombine/InstCombine.h Fri Jan 23 22:19:17 2015
> @@ -0,0 +1,46 @@
> +//===- InstCombine.h - InstCombine pass -------------------------*- C++ -*-===//
> +//
> +// The LLVM Compiler Infrastructure
> +//
> +// This file is distributed under the University of Illinois Open Source
> +// License. See LICENSE.TXT for details.
> +//
> +//===----------------------------------------------------------------------===//
> +/// \file
> +///
> +/// This file provides the primary interface to the instcombine pass. This pass
> +/// is suitable for use in the new pass manager. For a pass that works with the
> +/// legacy pass manager, please look for \c createInstructionCombiningPass() in
> +/// Scalar.h.
> +///
> +//===----------------------------------------------------------------------===//
> +
> +#ifndef LLVM_TRANSFORMS_INSTCOMBINE_INSTCOMBINE_H
> +#define LLVM_TRANSFORMS_INSTCOMBINE_INSTCOMBINE_H
> +
> +#include "llvm/IR/Function.h"
> +#include "llvm/IR/PassManager.h"
> +#include "llvm/Transforms/InstCombine/InstCombineWorklist.h"
> +
> +namespace llvm {
> +
> +class InstCombinePass {
> + InstCombineWorklist Worklist;
> +
> +public:
> + static StringRef name() { return "InstCombinePass"; }
> +
> + // Explicitly define constructors for MSVC.
> + InstCombinePass() {}
> + InstCombinePass(InstCombinePass &&Arg) : Worklist(std::move(Arg.Worklist)) {}
> + InstCombinePass &operator=(InstCombinePass &&RHS) {
> + Worklist = std::move(RHS.Worklist);
> + return *this;
> + }
> +
> + PreservedAnalyses run(Function &F, AnalysisManager<Function> *AM);
> +};
> +
> +}
> +
> +#endif
>
> Copied: llvm/trunk/include/llvm/Transforms/InstCombine/InstCombineWorklist.h (from r226981, llvm/trunk/lib/Transforms/InstCombine/InstCombineWorklist.h)
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/Transforms/InstCombine/InstCombineWorklist.h?p2=llvm/trunk/include/llvm/Transforms/InstCombine/InstCombineWorklist.h&p1=llvm/trunk/lib/Transforms/InstCombine/InstCombineWorklist.h&r1=226981&r2=226987&rev=226987&view=diff
> ==============================================================================
> --- llvm/trunk/lib/Transforms/InstCombine/InstCombineWorklist.h (original)
> +++ llvm/trunk/include/llvm/Transforms/InstCombine/InstCombineWorklist.h Fri Jan 23 22:19:17 2015
> @@ -7,8 +7,8 @@
> //
> //===----------------------------------------------------------------------===//
>
> -#ifndef LLVM_LIB_TRANSFORMS_INSTCOMBINE_INSTCOMBINEWORKLIST_H
> -#define LLVM_LIB_TRANSFORMS_INSTCOMBINE_INSTCOMBINEWORKLIST_H
> +#ifndef LLVM_TRANSFORMS_INSTCOMBINE_INSTCOMBINEWORKLIST_H
> +#define LLVM_TRANSFORMS_INSTCOMBINE_INSTCOMBINEWORKLIST_H
>
> #include "llvm/ADT/DenseMap.h"
> #include "llvm/ADT/SmallVector.h"
> @@ -32,6 +32,15 @@ class LLVM_LIBRARY_VISIBILITY InstCombin
> public:
> InstCombineWorklist() {}
>
> + InstCombineWorklist(InstCombineWorklist &&Arg)
> + : Worklist(std::move(Arg.Worklist)),
> + WorklistMap(std::move(Arg.WorklistMap)) {}
> + InstCombineWorklist &operator=(InstCombineWorklist &&RHS) {
> + Worklist = std::move(RHS.Worklist);
> + WorklistMap = std::move(RHS.WorklistMap);
> + return *this;
> + }
> +
> bool isEmpty() const { return Worklist.empty(); }
>
> /// Add - Add the specified instruction to the worklist if it isn't already
>
> Modified: llvm/trunk/lib/Transforms/InstCombine/InstCombineInternal.h
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/InstCombine/InstCombineInternal.h?rev=226987&r1=226986&r2=226987&view=diff
> ==============================================================================
> --- llvm/trunk/lib/Transforms/InstCombine/InstCombineInternal.h (original)
> +++ llvm/trunk/lib/Transforms/InstCombine/InstCombineInternal.h Fri Jan 23 22:19:17 2015
> @@ -15,7 +15,6 @@
> #ifndef LLVM_LIB_TRANSFORMS_INSTCOMBINE_INSTCOMBINEINTERNAL_H
> #define LLVM_LIB_TRANSFORMS_INSTCOMBINE_INSTCOMBINEINTERNAL_H
>
> -#include "InstCombineWorklist.h"
> #include "llvm/Analysis/AssumptionCache.h"
> #include "llvm/Analysis/LoopInfo.h"
> #include "llvm/Analysis/TargetFolder.h"
> @@ -27,6 +26,7 @@
> #include "llvm/IR/Operator.h"
> #include "llvm/IR/PatternMatch.h"
> #include "llvm/Pass.h"
> +#include "llvm/Transforms/InstCombine/InstCombineWorklist.h"
>
> #define DEBUG_TYPE "instcombine"
>
>
> Removed: llvm/trunk/lib/Transforms/InstCombine/InstCombineWorklist.h
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/InstCombine/InstCombineWorklist.h?rev=226986&view=auto
> ==============================================================================
> --- llvm/trunk/lib/Transforms/InstCombine/InstCombineWorklist.h (original)
> +++ llvm/trunk/lib/Transforms/InstCombine/InstCombineWorklist.h (removed)
> @@ -1,107 +0,0 @@
> -//===- InstCombineWorklist.h - Worklist for InstCombine pass ----*- C++ -*-===//
> -//
> -// The LLVM Compiler Infrastructure
> -//
> -// This file is distributed under the University of Illinois Open Source
> -// License. See LICENSE.TXT for details.
> -//
> -//===----------------------------------------------------------------------===//
> -
> -#ifndef LLVM_LIB_TRANSFORMS_INSTCOMBINE_INSTCOMBINEWORKLIST_H
> -#define LLVM_LIB_TRANSFORMS_INSTCOMBINE_INSTCOMBINEWORKLIST_H
> -
> -#include "llvm/ADT/DenseMap.h"
> -#include "llvm/ADT/SmallVector.h"
> -#include "llvm/IR/Instruction.h"
> -#include "llvm/Support/Compiler.h"
> -#include "llvm/Support/Debug.h"
> -#include "llvm/Support/raw_ostream.h"
> -
> -#define DEBUG_TYPE "instcombine"
> -
> -namespace llvm {
> -
> -/// InstCombineWorklist - This is the worklist management logic for
> -/// InstCombine.
> -class LLVM_LIBRARY_VISIBILITY InstCombineWorklist {
> - SmallVector<Instruction*, 256> Worklist;
> - DenseMap<Instruction*, unsigned> WorklistMap;
> -
> - void operator=(const InstCombineWorklist&RHS) LLVM_DELETED_FUNCTION;
> - InstCombineWorklist(const InstCombineWorklist&) LLVM_DELETED_FUNCTION;
> -public:
> - InstCombineWorklist() {}
> -
> - bool isEmpty() const { return Worklist.empty(); }
> -
> - /// Add - Add the specified instruction to the worklist if it isn't already
> - /// in it.
> - void Add(Instruction *I) {
> - if (WorklistMap.insert(std::make_pair(I, Worklist.size())).second) {
> - DEBUG(dbgs() << "IC: ADD: " << *I << '\n');
> - Worklist.push_back(I);
> - }
> - }
> -
> - void AddValue(Value *V) {
> - if (Instruction *I = dyn_cast<Instruction>(V))
> - Add(I);
> - }
> -
> - /// AddInitialGroup - Add the specified batch of stuff in reverse order.
> - /// which should only be done when the worklist is empty and when the group
> - /// has no duplicates.
> - void AddInitialGroup(Instruction *const *List, unsigned NumEntries) {
> - assert(Worklist.empty() && "Worklist must be empty to add initial group");
> - Worklist.reserve(NumEntries+16);
> - WorklistMap.resize(NumEntries);
> - DEBUG(dbgs() << "IC: ADDING: " << NumEntries << " instrs to worklist\n");
> - for (unsigned Idx = 0; NumEntries; --NumEntries) {
> - Instruction *I = List[NumEntries-1];
> - WorklistMap.insert(std::make_pair(I, Idx++));
> - Worklist.push_back(I);
> - }
> - }
> -
> - // Remove - remove I from the worklist if it exists.
> - void Remove(Instruction *I) {
> - DenseMap<Instruction*, unsigned>::iterator It = WorklistMap.find(I);
> - if (It == WorklistMap.end()) return; // Not in worklist.
> -
> - // Don't bother moving everything down, just null out the slot.
> - Worklist[It->second] = nullptr;
> -
> - WorklistMap.erase(It);
> - }
> -
> - Instruction *RemoveOne() {
> - Instruction *I = Worklist.pop_back_val();
> - WorklistMap.erase(I);
> - return I;
> - }
> -
> - /// AddUsersToWorkList - When an instruction is simplified, add all users of
> - /// the instruction to the work lists because they might get more simplified
> - /// now.
> - ///
> - void AddUsersToWorkList(Instruction &I) {
> - for (User *U : I.users())
> - Add(cast<Instruction>(U));
> - }
> -
> -
> - /// Zap - check that the worklist is empty and nuke the backing store for
> - /// the map if it is large.
> - void Zap() {
> - assert(WorklistMap.empty() && "Worklist empty, but map not?");
> -
> - // Do an explicit clear, this shrinks the map if needed.
> - WorklistMap.clear();
> - }
> -};
> -
> -} // end namespace llvm.
> -
> -#undef DEBUG_TYPE
> -
> -#endif
>
> Modified: llvm/trunk/lib/Transforms/InstCombine/InstructionCombining.cpp
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/InstCombine/InstructionCombining.cpp?rev=226987&r1=226986&r2=226987&view=diff
> ==============================================================================
> --- llvm/trunk/lib/Transforms/InstCombine/InstructionCombining.cpp (original)
> +++ llvm/trunk/lib/Transforms/InstCombine/InstructionCombining.cpp Fri Jan 23 22:19:17 2015
> @@ -33,7 +33,7 @@
> //
> //===----------------------------------------------------------------------===//
>
> -#include "llvm/Transforms/Scalar.h"
> +#include "llvm/Transforms/InstCombine/InstCombine.h"
> #include "InstCombineInternal.h"
> #include "llvm-c/Initialization.h"
> #include "llvm/ADT/SmallPtrSet.h"
> @@ -45,6 +45,7 @@
> #include "llvm/Analysis/InstructionSimplify.h"
> #include "llvm/Analysis/LoopInfo.h"
> #include "llvm/Analysis/MemoryBuiltins.h"
> +#include "llvm/Analysis/TargetLibraryInfo.h"
> #include "llvm/Analysis/ValueTracking.h"
> #include "llvm/IR/CFG.h"
> #include "llvm/IR/DataLayout.h"
> @@ -55,7 +56,7 @@
> #include "llvm/IR/ValueHandle.h"
> #include "llvm/Support/CommandLine.h"
> #include "llvm/Support/Debug.h"
> -#include "llvm/Analysis/TargetLibraryInfo.h"
> +#include "llvm/Transforms/Scalar.h"
> #include "llvm/Transforms/Utils/Local.h"
> #include <algorithm>
> #include <climits>
> @@ -2922,6 +2923,66 @@ static bool prepareICWorklistFromFunctio
> return MadeIRChange;
> }
>
> +static bool combineInstructionsOverFunction(
> + Function &F, InstCombineWorklist &Worklist, AssumptionCache &AC,
> + TargetLibraryInfo &TLI, DominatorTree &DT, const DataLayout *DL = nullptr,
> + LoopInfo *LI = nullptr) {
> + // Minimizing size?
> + bool MinimizeSize = F.getAttributes().hasAttribute(
> + AttributeSet::FunctionIndex, Attribute::MinSize);
> +
> + /// Builder - This is an IRBuilder that automatically inserts new
> + /// instructions into the worklist when they are created.
> + IRBuilder<true, TargetFolder, InstCombineIRInserter> Builder(
> + F.getContext(), TargetFolder(DL), InstCombineIRInserter(Worklist, &AC));
> +
> + // Lower dbg.declare intrinsics otherwise their value may be clobbered
> + // by instcombiner.
> + bool DbgDeclaresChanged = LowerDbgDeclare(F);
> +
> + // Iterate while there is work to do.
> + int Iteration = 0;
> + for (;;) {
> + ++Iteration;
> + DEBUG(dbgs() << "\n\nINSTCOMBINE ITERATION #" << Iteration << " on "
> + << F.getName() << "\n");
> +
> + bool Changed = false;
> + if (prepareICWorklistFromFunction(F, DL, &TLI, Worklist))
> + Changed = true;
> +
> + InstCombiner IC(Worklist, &Builder, MinimizeSize, &AC, &TLI, &DT, DL, LI);
> + if (IC.run())
> + Changed = true;
> +
> + if (!Changed)
> + break;
> + }
> +
> + return DbgDeclaresChanged || Iteration > 1;
> +}
> +
> +PreservedAnalyses InstCombinePass::run(Function &F,
> + AnalysisManager<Function> *AM) {
> + auto *DL = F.getParent()->getDataLayout();
> +
> + auto &AC = AM->getResult<AssumptionAnalysis>(F);
> + auto &DT = AM->getResult<DominatorTreeAnalysis>(F);
> + auto &TLI = AM->getResult<TargetLibraryAnalysis>(F);
> +
> + auto *LI = AM->getCachedResult<LoopAnalysis>(F);
> +
> + if (!combineInstructionsOverFunction(F, Worklist, AC, TLI, DT, DL, LI))
> + // No changes, all analyses are preserved.
> + return PreservedAnalyses::all();
> +
> + // Mark all the analyses that instcombine updates as preserved.
> + // FIXME: Need a way to preserve CFG analyses here!
> + PreservedAnalyses PA;
> + PA.preserve<DominatorTreeAnalysis>();
> + return PA;
> +}
> +
> namespace {
> /// \brief The legacy pass manager's instcombine pass.
> ///
> @@ -2954,10 +3015,6 @@ bool InstructionCombiningPass::runOnFunc
> if (skipOptnoneFunction(F))
> return false;
>
> - // Lower dbg.declare intrinsics otherwise their value may be clobbered
> - // by instcombiner.
> - bool DbgDeclaresChanged = LowerDbgDeclare(F);
> -
> // Required analyses.
> auto &AC = getAnalysis<AssumptionCacheTracker>().getAssumptionCache(F);
> auto &TLI = getAnalysis<TargetLibraryInfoWrapperPass>().getTLI();
> @@ -2969,35 +3026,7 @@ bool InstructionCombiningPass::runOnFunc
> auto *LIWP = getAnalysisIfAvailable<LoopInfoWrapperPass>();
> auto *LI = LIWP ? &LIWP->getLoopInfo() : nullptr;
>
> - // Minimizing size?
> - bool MinimizeSize = F.getAttributes().hasAttribute(
> - AttributeSet::FunctionIndex, Attribute::MinSize);
> -
> - /// Builder - This is an IRBuilder that automatically inserts new
> - /// instructions into the worklist when they are created.
> - IRBuilder<true, TargetFolder, InstCombineIRInserter> Builder(
> - F.getContext(), TargetFolder(DL), InstCombineIRInserter(Worklist, &AC));
> -
> - // Iterate while there is work to do.
> - int Iteration = 0;
> - for (;;) {
> - ++Iteration;
> - DEBUG(dbgs() << "\n\nINSTCOMBINE ITERATION #" << Iteration << " on "
> - << F.getName() << "\n");
> -
> - bool Changed = false;
> - if (prepareICWorklistFromFunction(F, DL, &TLI, Worklist))
> - Changed = true;
> -
> - InstCombiner IC(Worklist, &Builder, MinimizeSize, &AC, &TLI, &DT, DL, LI);
> - if (IC.run())
> - Changed = true;
> -
> - if (!Changed)
> - break;
> - }
> -
> - return DbgDeclaresChanged || Iteration > 1;
> + return combineInstructionsOverFunction(F, Worklist, AC, TLI, DT, DL, LI);
> }
>
> char InstructionCombiningPass::ID = 0;
>
> Modified: llvm/trunk/test/Transforms/InstCombine/load.ll
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/InstCombine/load.ll?rev=226987&r1=226986&r2=226987&view=diff
> ==============================================================================
> --- llvm/trunk/test/Transforms/InstCombine/load.ll (original)
> +++ llvm/trunk/test/Transforms/InstCombine/load.ll Fri Jan 23 22:19:17 2015
> @@ -1,4 +1,5 @@
> ; RUN: opt -instcombine -S < %s | FileCheck %s
> +; RUN: opt -passes=instcombine -S < %s | FileCheck %s
>
> ; This test makes sure that these instructions are properly eliminated.
>
>
> Modified: llvm/trunk/tools/opt/PassRegistry.def
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/tools/opt/PassRegistry.def?rev=226987&r1=226986&r2=226987&view=diff
> ==============================================================================
> --- llvm/trunk/tools/opt/PassRegistry.def (original)
> +++ llvm/trunk/tools/opt/PassRegistry.def Fri Jan 23 22:19:17 2015
> @@ -60,6 +60,7 @@ FUNCTION_ANALYSIS("targetlibinfo", Targe
> #ifndef FUNCTION_PASS
> #define FUNCTION_PASS(NAME, CREATE_PASS)
> #endif
> +FUNCTION_PASS("instcombine", InstCombinePass())
> FUNCTION_PASS("invalidate<all>", InvalidateAllAnalysesPass())
> FUNCTION_PASS("no-op-function", NoOpFunctionPass())
> FUNCTION_PASS("print", PrintFunctionPass(dbgs()))
>
> Modified: llvm/trunk/tools/opt/Passes.cpp
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/tools/opt/Passes.cpp?rev=226987&r1=226986&r2=226987&view=diff
> ==============================================================================
> --- llvm/trunk/tools/opt/Passes.cpp (original)
> +++ llvm/trunk/tools/opt/Passes.cpp Fri Jan 23 22:19:17 2015
> @@ -25,6 +25,7 @@
> #include "llvm/IR/PassManager.h"
> #include "llvm/IR/Verifier.h"
> #include "llvm/Support/Debug.h"
> +#include "llvm/Transforms/InstCombine/InstCombine.h"
>
> using namespace llvm;
>
>
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
_______________________________________________
llvm-commits mailing list
llvm-commits at cs.uiuc.edu
http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
More information about the llvm-commits
mailing list