Skip to content

Commit 43882b1

Browse files
committedMay 23, 2019
[MergeICmps] Make the pass compatible with the new pass manager.
Reviewers: gchatelet, spatel Subscribers: hiraditya, llvm-commits Tags: #llvm Differential Revision: https://reviews.llvm.org/D62287 llvm-svn: 361490
1 parent 903f5b0 commit 43882b1

File tree

10 files changed

+110
-77
lines changed

10 files changed

+110
-77
lines changed
 

‎llvm/include/llvm/InitializePasses.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -279,7 +279,7 @@ void initializeMemorySSAPrinterLegacyPassPass(PassRegistry&);
279279
void initializeMemorySSAWrapperPassPass(PassRegistry&);
280280
void initializeMemorySanitizerLegacyPassPass(PassRegistry&);
281281
void initializeMergeFunctionsPass(PassRegistry&);
282-
void initializeMergeICmpsPass(PassRegistry&);
282+
void initializeMergeICmpsLegacyPassPass(PassRegistry &);
283283
void initializeMergedLoadStoreMotionLegacyPassPass(PassRegistry&);
284284
void initializeMetaRenamerPass(PassRegistry&);
285285
void initializeModuleDebugInfoPrinterPass(PassRegistry&);

‎llvm/include/llvm/LinkAllPasses.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -191,7 +191,7 @@ namespace {
191191
(void) llvm::createPostOrderFunctionAttrsLegacyPass();
192192
(void) llvm::createReversePostOrderFunctionAttrsPass();
193193
(void) llvm::createMergeFunctionsPass();
194-
(void) llvm::createMergeICmpsPass();
194+
(void) llvm::createMergeICmpsLegacyPass();
195195
(void) llvm::createExpandMemCmpPass();
196196
std::string buf;
197197
llvm::raw_string_ostream os(buf);

‎llvm/include/llvm/Transforms/Scalar.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -371,7 +371,7 @@ Pass *createLowerWidenableConditionPass();
371371
//
372372
// MergeICmps - Merge integer comparison chains into a memcmp
373373
//
374-
Pass *createMergeICmpsPass();
374+
Pass *createMergeICmpsLegacyPass();
375375

376376
//===----------------------------------------------------------------------===//
377377
//
Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,25 @@
1+
//===- MergeICmps.h -----------------------------*- C++ -*-===//
2+
//
3+
// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
4+
// See https://llvm.org/LICENSE.txt for license information.
5+
// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
6+
//
7+
//===----------------------------------------------------------------------===//
8+
9+
#ifndef LLVM_TRANSFORMS_SCALAR_MERGEICMPS_H
10+
#define LLVM_TRANSFORMS_SCALAR_MERGEICMPS_H
11+
12+
#include "llvm/IR/PassManager.h"
13+
14+
namespace llvm {
15+
16+
class Function;
17+
18+
struct MergeICmpsPass
19+
: PassInfoMixin<MergeICmpsPass> {
20+
PreservedAnalyses run(Function &F, FunctionAnalysisManager &AM);
21+
};
22+
23+
} // end namespace llvm
24+
25+
#endif // LLVM_TRANSFORMS_SCALAR_MERGEICMPS_H

‎llvm/lib/CodeGen/TargetPassConfig.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -646,7 +646,7 @@ void TargetPassConfig::addIRPasses() {
646646
// into optimally-sized loads and compares. The transforms are enabled by a
647647
// target lowering hook.
648648
if (!DisableMergeICmps)
649-
addPass(createMergeICmpsPass());
649+
addPass(createMergeICmpsLegacyPass());
650650
addPass(createExpandMemCmpPass());
651651
}
652652

‎llvm/lib/Passes/PassBuilder.cpp

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -142,6 +142,7 @@
142142
#include "llvm/Transforms/Scalar/MakeGuardsExplicit.h"
143143
#include "llvm/Transforms/Scalar/MemCpyOptimizer.h"
144144
#include "llvm/Transforms/Scalar/MergedLoadStoreMotion.h"
145+
#include "llvm/Transforms/Scalar/MergeICmps.h"
145146
#include "llvm/Transforms/Scalar/NaryReassociate.h"
146147
#include "llvm/Transforms/Scalar/NewGVN.h"
147148
#include "llvm/Transforms/Scalar/PartiallyInlineLibCalls.h"

‎llvm/lib/Passes/PassRegistry.def

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -190,6 +190,7 @@ FUNCTION_PASS("loop-sink", LoopSinkPass())
190190
FUNCTION_PASS("lowerinvoke", LowerInvokePass())
191191
FUNCTION_PASS("mem2reg", PromotePass())
192192
FUNCTION_PASS("memcpyopt", MemCpyOptPass())
193+
FUNCTION_PASS("mergeicmps", MergeICmpsPass())
193194
FUNCTION_PASS("mldst-motion", MergedLoadStoreMotionPass())
194195
FUNCTION_PASS("nary-reassociate", NaryReassociatePass())
195196
FUNCTION_PASS("newgvn", NewGVNPass())

‎llvm/lib/Transforms/Scalar/MergeICmps.cpp

Lines changed: 77 additions & 71 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,7 @@
4141
//
4242
//===----------------------------------------------------------------------===//
4343

44+
#include "llvm/Transforms/Scalar/MergeICmps.h"
4445
#include "llvm/Analysis/DomTreeUpdater.h"
4546
#include "llvm/Analysis/GlobalsModRef.h"
4647
#include "llvm/Analysis/Loads.h"
@@ -214,19 +215,19 @@ class BCECmpBlock {
214215

215216
// Returns true if the non-BCE-cmp instructions can be separated from BCE-cmp
216217
// instructions in the block.
217-
bool canSplit(AliasAnalysis *AA) const;
218+
bool canSplit(AliasAnalysis &AA) const;
218219

219220
// Return true if this all the relevant instructions in the BCE-cmp-block can
220221
// be sunk below this instruction. By doing this, we know we can separate the
221222
// BCE-cmp-block instructions from the non-BCE-cmp-block instructions in the
222223
// block.
223224
bool canSinkBCECmpInst(const Instruction *, DenseSet<Instruction *> &,
224-
AliasAnalysis *AA) const;
225+
AliasAnalysis &AA) const;
225226

226227
// We can separate the BCE-cmp-block instructions and the non-BCE-cmp-block
227228
// instructions. Split the old block and move all non-BCE-cmp-insts into the
228229
// new parent block.
229-
void split(BasicBlock *NewParent, AliasAnalysis *AA) const;
230+
void split(BasicBlock *NewParent, AliasAnalysis &AA) const;
230231

231232
// The basic block where this comparison happens.
232233
BasicBlock *BB = nullptr;
@@ -245,7 +246,7 @@ class BCECmpBlock {
245246

246247
bool BCECmpBlock::canSinkBCECmpInst(const Instruction *Inst,
247248
DenseSet<Instruction *> &BlockInsts,
248-
AliasAnalysis *AA) const {
249+
AliasAnalysis &AA) const {
249250
// If this instruction has side effects and its in middle of the BCE cmp block
250251
// instructions, then bail for now.
251252
if (Inst->mayHaveSideEffects()) {
@@ -255,9 +256,9 @@ bool BCECmpBlock::canSinkBCECmpInst(const Instruction *Inst,
255256
// Disallow stores that might alias the BCE operands
256257
MemoryLocation LLoc = MemoryLocation::get(Lhs_.LoadI);
257258
MemoryLocation RLoc = MemoryLocation::get(Rhs_.LoadI);
258-
if (isModSet(AA->getModRefInfo(Inst, LLoc)) ||
259-
isModSet(AA->getModRefInfo(Inst, RLoc)))
260-
return false;
259+
if (isModSet(AA.getModRefInfo(Inst, LLoc)) ||
260+
isModSet(AA.getModRefInfo(Inst, RLoc)))
261+
return false;
261262
}
262263
// Make sure this instruction does not use any of the BCE cmp block
263264
// instructions as operand.
@@ -268,7 +269,7 @@ bool BCECmpBlock::canSinkBCECmpInst(const Instruction *Inst,
268269
return true;
269270
}
270271

271-
void BCECmpBlock::split(BasicBlock *NewParent, AliasAnalysis *AA) const {
272+
void BCECmpBlock::split(BasicBlock *NewParent, AliasAnalysis &AA) const {
272273
DenseSet<Instruction *> BlockInsts(
273274
{Lhs_.GEP, Rhs_.GEP, Lhs_.LoadI, Rhs_.LoadI, CmpI, BranchI});
274275
llvm::SmallVector<Instruction *, 4> OtherInsts;
@@ -288,7 +289,7 @@ void BCECmpBlock::split(BasicBlock *NewParent, AliasAnalysis *AA) const {
288289
}
289290
}
290291

291-
bool BCECmpBlock::canSplit(AliasAnalysis *AA) const {
292+
bool BCECmpBlock::canSplit(AliasAnalysis &AA) const {
292293
DenseSet<Instruction *> BlockInsts(
293294
{Lhs_.GEP, Rhs_.GEP, Lhs_.LoadI, Rhs_.LoadI, CmpI, BranchI});
294295
for (Instruction &Inst : *BB) {
@@ -404,16 +405,16 @@ static inline void enqueueBlock(std::vector<BCECmpBlock> &Comparisons,
404405
// A chain of comparisons.
405406
class BCECmpChain {
406407
public:
407-
BCECmpChain(const std::vector<BasicBlock *> &Blocks, PHINode &Phi,
408-
AliasAnalysis *AA);
408+
BCECmpChain(const std::vector<BasicBlock *> &Blocks, PHINode &Phi,
409+
AliasAnalysis &AA);
409410

410-
int size() const { return Comparisons_.size(); }
411+
int size() const { return Comparisons_.size(); }
411412

412413
#ifdef MERGEICMPS_DOT_ON
413414
void dump() const;
414415
#endif // MERGEICMPS_DOT_ON
415416

416-
bool simplify(const TargetLibraryInfo *const TLI, AliasAnalysis *AA,
417+
bool simplify(const TargetLibraryInfo &TLI, AliasAnalysis &AA,
417418
DomTreeUpdater &DTU);
418419

419420
private:
@@ -432,7 +433,7 @@ class BCECmpChain {
432433
};
433434

434435
BCECmpChain::BCECmpChain(const std::vector<BasicBlock *> &Blocks, PHINode &Phi,
435-
AliasAnalysis *AA)
436+
AliasAnalysis &AA)
436437
: Phi_(Phi) {
437438
assert(!Blocks.empty() && "a chain should have at least one block");
438439
// Now look inside blocks to check for BCE comparisons.
@@ -604,9 +605,8 @@ class MergedBlockName {
604605
static BasicBlock *mergeComparisons(ArrayRef<BCECmpBlock> Comparisons,
605606
BasicBlock *const InsertBefore,
606607
BasicBlock *const NextCmpBlock,
607-
PHINode &Phi,
608-
const TargetLibraryInfo *const TLI,
609-
AliasAnalysis *AA, DomTreeUpdater &DTU) {
608+
PHINode &Phi, const TargetLibraryInfo &TLI,
609+
AliasAnalysis &AA, DomTreeUpdater &DTU) {
610610
assert(!Comparisons.empty() && "merging zero comparisons");
611611
LLVMContext &Context = NextCmpBlock->getContext();
612612
const BCECmpBlock &FirstCmp = Comparisons[0];
@@ -652,7 +652,7 @@ static BasicBlock *mergeComparisons(ArrayRef<BCECmpBlock> Comparisons,
652652
Value *const MemCmpCall = emitMemCmp(
653653
Lhs, Rhs,
654654
ConstantInt::get(DL.getIntPtrType(Context), TotalSizeBits / 8), Builder,
655-
DL, TLI);
655+
DL, &TLI);
656656
IsEqual = Builder.CreateICmpEQ(
657657
MemCmpCall, ConstantInt::get(Type::getInt32Ty(Context), 0));
658658
}
@@ -674,8 +674,8 @@ static BasicBlock *mergeComparisons(ArrayRef<BCECmpBlock> Comparisons,
674674
return BB;
675675
}
676676

677-
bool BCECmpChain::simplify(const TargetLibraryInfo *const TLI,
678-
AliasAnalysis *AA, DomTreeUpdater &DTU) {
677+
bool BCECmpChain::simplify(const TargetLibraryInfo &TLI, AliasAnalysis &AA,
678+
DomTreeUpdater &DTU) {
679679
assert(Comparisons_.size() >= 2 && "simplifying trivial BCECmpChain");
680680
// First pass to check if there is at least one merge. If not, we don't do
681681
// anything and we keep analysis passes intact.
@@ -694,9 +694,9 @@ bool BCECmpChain::simplify(const TargetLibraryInfo *const TLI,
694694

695695
// Effectively merge blocks. We go in the reverse direction from the phi block
696696
// so that the next block is always available to branch to.
697-
const auto mergeRange = [this, TLI, AA, &DTU](int I, int Num,
698-
BasicBlock *InsertBefore,
699-
BasicBlock *Next) {
697+
const auto mergeRange = [this, &TLI, &AA, &DTU](int I, int Num,
698+
BasicBlock *InsertBefore,
699+
BasicBlock *Next) {
700700
return mergeComparisons(makeArrayRef(Comparisons_).slice(I, Num),
701701
InsertBefore, Next, Phi_, TLI, AA, DTU);
702702
};
@@ -790,8 +790,8 @@ std::vector<BasicBlock *> getOrderedBlocks(PHINode &Phi,
790790
return Blocks;
791791
}
792792

793-
bool processPhi(PHINode &Phi, const TargetLibraryInfo *const TLI,
794-
AliasAnalysis *AA, DomTreeUpdater &DTU) {
793+
bool processPhi(PHINode &Phi, const TargetLibraryInfo &TLI, AliasAnalysis &AA,
794+
DomTreeUpdater &DTU) {
795795
LLVM_DEBUG(dbgs() << "processPhi()\n");
796796
if (Phi.getNumIncomingValues() <= 1) {
797797
LLVM_DEBUG(dbgs() << "skip: only one incoming value in phi\n");
@@ -859,12 +859,40 @@ bool processPhi(PHINode &Phi, const TargetLibraryInfo *const TLI,
859859
return CmpChain.simplify(TLI, AA, DTU);
860860
}
861861

862-
class MergeICmps : public FunctionPass {
863-
public:
862+
static bool runImpl(Function &F, const TargetLibraryInfo &TLI,
863+
const TargetTransformInfo &TTI, AliasAnalysis &AA,
864+
DominatorTree *DT) {
865+
LLVM_DEBUG(dbgs() << "MergeICmpsLegacyPass: " << F.getName() << "\n");
866+
867+
// We only try merging comparisons if the target wants to expand memcmp later.
868+
// The rationale is to avoid turning small chains into memcmp calls.
869+
if (!TTI.enableMemCmpExpansion(true))
870+
return false;
871+
872+
// If we don't have memcmp avaiable we can't emit calls to it.
873+
if (!TLI.has(LibFunc_memcmp))
874+
return false;
875+
876+
DomTreeUpdater DTU(DT, /*PostDominatorTree*/ nullptr,
877+
DomTreeUpdater::UpdateStrategy::Eager);
878+
879+
bool MadeChange = false;
880+
881+
for (auto BBIt = ++F.begin(); BBIt != F.end(); ++BBIt) {
882+
// A Phi operation is always first in a basic block.
883+
if (auto *const Phi = dyn_cast<PHINode>(&*BBIt->begin()))
884+
MadeChange |= processPhi(*Phi, TLI, AA, DTU);
885+
}
886+
887+
return MadeChange;
888+
}
889+
890+
class MergeICmpsLegacyPass : public FunctionPass {
891+
public:
864892
static char ID;
865893

866-
MergeICmps() : FunctionPass(ID) {
867-
initializeMergeICmpsPass(*PassRegistry::getPassRegistry());
894+
MergeICmpsLegacyPass() : FunctionPass(ID) {
895+
initializeMergeICmpsLegacyPassPass(*PassRegistry::getPassRegistry());
868896
}
869897

870898
bool runOnFunction(Function &F) override {
@@ -874,12 +902,8 @@ class MergeICmps : public FunctionPass {
874902
// MergeICmps does not need the DominatorTree, but we update it if it's
875903
// already available.
876904
auto *DTWP = getAnalysisIfAvailable<DominatorTreeWrapperPass>();
877-
DomTreeUpdater DTU(DTWP ? &DTWP->getDomTree() : nullptr,
878-
/*PostDominatorTree*/ nullptr,
879-
DomTreeUpdater::UpdateStrategy::Eager);
880-
AliasAnalysis *AA = &getAnalysis<AAResultsWrapperPass>().getAAResults();
881-
auto PA = runImpl(F, &TLI, &TTI, AA, DTU);
882-
return !PA.areAllPreserved();
905+
auto &AA = getAnalysis<AAResultsWrapperPass>().getAAResults();
906+
return runImpl(F, TLI, TTI, AA, DTWP ? &DTWP->getDomTree() : nullptr);
883907
}
884908

885909
private:
@@ -890,50 +914,32 @@ class MergeICmps : public FunctionPass {
890914
AU.addPreserved<GlobalsAAWrapperPass>();
891915
AU.addPreserved<DominatorTreeWrapperPass>();
892916
}
893-
894-
PreservedAnalyses runImpl(Function &F, const TargetLibraryInfo *TLI,
895-
const TargetTransformInfo *TTI, AliasAnalysis *AA,
896-
DomTreeUpdater &DTU);
897917
};
898918

899-
PreservedAnalyses MergeICmps::runImpl(Function &F, const TargetLibraryInfo *TLI,
900-
const TargetTransformInfo *TTI,
901-
AliasAnalysis *AA, DomTreeUpdater &DTU) {
902-
LLVM_DEBUG(dbgs() << "MergeICmpsPass: " << F.getName() << "\n");
903-
904-
// We only try merging comparisons if the target wants to expand memcmp later.
905-
// The rationale is to avoid turning small chains into memcmp calls.
906-
if (!TTI->enableMemCmpExpansion(true)) return PreservedAnalyses::all();
919+
} // namespace
907920

908-
// If we don't have memcmp avaiable we can't emit calls to it.
909-
if (!TLI->has(LibFunc_memcmp))
910-
return PreservedAnalyses::all();
921+
char MergeICmpsLegacyPass::ID = 0;
922+
INITIALIZE_PASS_BEGIN(MergeICmpsLegacyPass, "mergeicmps",
923+
"Merge contiguous icmps into a memcmp", false, false)
924+
INITIALIZE_PASS_DEPENDENCY(TargetLibraryInfoWrapperPass)
925+
INITIALIZE_PASS_DEPENDENCY(TargetTransformInfoWrapperPass)
926+
INITIALIZE_PASS_DEPENDENCY(AAResultsWrapperPass)
927+
INITIALIZE_PASS_END(MergeICmpsLegacyPass, "mergeicmps",
928+
"Merge contiguous icmps into a memcmp", false, false)
911929

912-
bool MadeChange = false;
930+
Pass *llvm::createMergeICmpsLegacyPass() { return new MergeICmpsLegacyPass(); }
913931

914-
for (auto BBIt = ++F.begin(); BBIt != F.end(); ++BBIt) {
915-
// A Phi operation is always first in a basic block.
916-
if (auto *const Phi = dyn_cast<PHINode>(&*BBIt->begin()))
917-
MadeChange |= processPhi(*Phi, TLI, AA, DTU);
918-
}
919-
920-
if (!MadeChange)
932+
PreservedAnalyses MergeICmpsPass::run(Function &F,
933+
FunctionAnalysisManager &AM) {
934+
auto &TLI = AM.getResult<TargetLibraryAnalysis>(F);
935+
auto &TTI = AM.getResult<TargetIRAnalysis>(F);
936+
auto &AA = AM.getResult<AAManager>(F);
937+
auto *DT = AM.getCachedResult<DominatorTreeAnalysis>(F);
938+
const bool MadeChanges = runImpl(F, TLI, TTI, AA, DT);
939+
if (!MadeChanges)
921940
return PreservedAnalyses::all();
922941
PreservedAnalyses PA;
923942
PA.preserve<GlobalsAA>();
924943
PA.preserve<DominatorTreeAnalysis>();
925944
return PA;
926945
}
927-
928-
} // namespace
929-
930-
char MergeICmps::ID = 0;
931-
INITIALIZE_PASS_BEGIN(MergeICmps, "mergeicmps",
932-
"Merge contiguous icmps into a memcmp", false, false)
933-
INITIALIZE_PASS_DEPENDENCY(TargetLibraryInfoWrapperPass)
934-
INITIALIZE_PASS_DEPENDENCY(TargetTransformInfoWrapperPass)
935-
INITIALIZE_PASS_DEPENDENCY(AAResultsWrapperPass)
936-
INITIALIZE_PASS_END(MergeICmps, "mergeicmps",
937-
"Merge contiguous icmps into a memcmp", false, false)
938-
939-
Pass *llvm::createMergeICmpsPass() { return new MergeICmps(); }

‎llvm/lib/Transforms/Scalar/Scalar.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -83,7 +83,7 @@ void llvm::initializeScalarOpts(PassRegistry &Registry) {
8383
initializeLowerGuardIntrinsicLegacyPassPass(Registry);
8484
initializeLowerWidenableConditionLegacyPassPass(Registry);
8585
initializeMemCpyOptLegacyPassPass(Registry);
86-
initializeMergeICmpsPass(Registry);
86+
initializeMergeICmpsLegacyPassPass(Registry);
8787
initializeMergedLoadStoreMotionLegacyPassPass(Registry);
8888
initializeNaryReassociateLegacyPassPass(Registry);
8989
initializePartiallyInlineLibCallsLegacyPassPass(Registry);

‎llvm/test/Transforms/MergeICmps/X86/pair-int32-int32.ll

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
; NOTE: Assertions have been autogenerated by utils/update_test_checks.py
2-
; RUN: opt < %s -mergeicmps -verify-dom-info -mtriple=x86_64-unknown-unknown -S | FileCheck %s --check-prefix=X86
2+
; RUN: opt < %s -passes='require<domtree>,mergeicmps,verify<domtree>' -mtriple=x86_64-unknown-unknown -S | FileCheck %s --check-prefix=X86
33
; RUN: opt < %s -mergeicmps -verify-dom-info -mtriple=x86_64-unknown-unknown -S -disable-simplify-libcalls | FileCheck %s --check-prefix=X86-NOBUILTIN
44

55
%S = type { i32, i32 }

0 commit comments

Comments
 (0)
Please sign in to comment.