[llvm] r226987 - [PM] Port instcombine to the new pass manager!
Aaron Ballman
aaron at aaronballman.com
Sat Jan 24 13:31:46 PST 2015
On Sat, Jan 24, 2015 at 3:50 PM, Patrik Hägglund H
<patrik.h.hagglund at ericsson.com> wrote:
> 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?
I think the correct fix may be to remove LLVM_LIBRARY_VISIBILITY from
InstCombinWorklist, but I was waiting for Chandler's input since he
understands the design here far better than I do.
~Aaron
>
> /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