Revert "Expand existing loopsink testing to also test loopsinking using new pass manager and fix LICM bug."

This reverts commit d4ba28bddc.
This commit is contained in:
Jamie Schmeiser 2020-11-18 15:17:53 -05:00
parent 87369c6261
commit 562addba65
6 changed files with 32 additions and 141 deletions

View File

@ -163,10 +163,8 @@ static bool pointerInvalidatedByLoop(MemoryLocation MemLoc,
AliasSetTracker *CurAST, Loop *CurLoop,
AAResults *AA);
static bool pointerInvalidatedByLoopWithMSSA(MemorySSA *MSSA, MemoryUse *MU,
Loop *CurLoop, Instruction &I,
Loop *CurLoop,
SinkAndHoistLICMFlags &Flags);
static bool pointerInvalidatedByBlockWithMSSA(BasicBlock &BB, MemorySSA &MSSA,
MemoryUse &MU);
static Instruction *cloneInstructionInExitBlock(
Instruction &I, BasicBlock &ExitBlock, PHINode &PN, const LoopInfo *LI,
const LoopSafetyInfo *SafetyInfo, MemorySSAUpdater *MSSAU);
@ -1157,7 +1155,7 @@ bool llvm::canSinkOrHoistInst(Instruction &I, AAResults *AA, DominatorTree *DT,
CurLoop, AA);
else
Invalidated = pointerInvalidatedByLoopWithMSSA(
MSSA, cast<MemoryUse>(MSSA->getMemoryAccess(LI)), CurLoop, I, *Flags);
MSSA, cast<MemoryUse>(MSSA->getMemoryAccess(LI)), CurLoop, *Flags);
// Check loop-invariant address because this may also be a sinkable load
// whose address is not necessarily loop-invariant.
if (ORE && Invalidated && CurLoop->isLoopInvariant(LI->getPointerOperand()))
@ -1213,7 +1211,7 @@ bool llvm::canSinkOrHoistInst(Instruction &I, AAResults *AA, DominatorTree *DT,
CurAST, CurLoop, AA);
else
Invalidated = pointerInvalidatedByLoopWithMSSA(
MSSA, cast<MemoryUse>(MSSA->getMemoryAccess(CI)), CurLoop, I,
MSSA, cast<MemoryUse>(MSSA->getMemoryAccess(CI)), CurLoop,
*Flags);
if (Invalidated)
return false;
@ -1314,6 +1312,7 @@ bool llvm::canSinkOrHoistInst(Instruction &I, AAResults *AA, DominatorTree *DT,
}
}
}
auto *Source = MSSA->getSkipSelfWalker()->getClobberingMemoryAccess(SI);
Flags->incrementClobberingCalls();
// If there are no clobbering Defs in the loop, store is safe to hoist.
@ -2286,7 +2285,7 @@ static bool pointerInvalidatedByLoop(MemoryLocation MemLoc,
}
bool pointerInvalidatedByLoopWithMSSA(MemorySSA *MSSA, MemoryUse *MU,
Loop *CurLoop, Instruction &I,
Loop *CurLoop,
SinkAndHoistLICMFlags &Flags) {
// For hoisting, use the walker to determine safety
if (!Flags.getIsSink()) {
@ -2322,22 +2321,12 @@ bool pointerInvalidatedByLoopWithMSSA(MemorySSA *MSSA, MemoryUse *MU,
if (Flags.tooManyMemoryAccesses())
return true;
for (auto *BB : CurLoop->getBlocks())
if (pointerInvalidatedByBlockWithMSSA(*BB, *MSSA, *MU))
return true;
// When sinking, the source block may not be part of the loop so check it.
if (!CurLoop->contains(&I))
return pointerInvalidatedByBlockWithMSSA(*I.getParent(), *MSSA, *MU);
return false;
}
bool pointerInvalidatedByBlockWithMSSA(BasicBlock &BB, MemorySSA &MSSA,
MemoryUse &MU) {
if (const auto *Accesses = MSSA.getBlockDefs(&BB))
for (const auto &MA : *Accesses)
if (const auto *MD = dyn_cast<MemoryDef>(&MA))
if (MU.getBlock() != MD->getBlock() || !MSSA.locallyDominates(MD, &MU))
return true;
if (auto *Accesses = MSSA->getBlockDefs(BB))
for (const auto &MA : *Accesses)
if (const auto *MD = dyn_cast<MemoryDef>(&MA))
if (MU->getBlock() != MD->getBlock() ||
!MSSA->locallyDominates(MD, MU))
return true;
return false;
}

View File

@ -39,8 +39,6 @@
#include "llvm/Analysis/Loads.h"
#include "llvm/Analysis/LoopInfo.h"
#include "llvm/Analysis/LoopPass.h"
#include "llvm/Analysis/MemorySSA.h"
#include "llvm/Analysis/MemorySSAUpdater.h"
#include "llvm/Analysis/ScalarEvolution.h"
#include "llvm/Analysis/ScalarEvolutionAliasAnalysis.h"
#include "llvm/IR/Dominators.h"
@ -69,14 +67,6 @@ static cl::opt<unsigned> MaxNumberOfUseBBsForSinking(
"max-uses-for-sinking", cl::Hidden, cl::init(30),
cl::desc("Do not sink instructions that have too many uses."));
static cl::opt<bool> EnableMSSAInLoopSink(
"enable-mssa-in-loop-sink", cl::Hidden, cl::init(false),
cl::desc("Enable MemorySSA for LoopSink in new pass manager"));
static cl::opt<bool> EnableMSSAInLegacyLoopSink(
"enable-mssa-in-legacy-loop-sink", cl::Hidden, cl::init(false),
cl::desc("Enable MemorySSA for LoopSink in legacy pass manager"));
/// Return adjusted total frequency of \p BBs.
///
/// * If there is only one BB, sinking instruction will not introduce code
@ -182,10 +172,11 @@ findBBsToSinkInto(const Loop &L, const SmallPtrSetImpl<BasicBlock *> &UseBBs,
// sinking is successful.
// \p LoopBlockNumber is used to sort the insertion blocks to ensure
// determinism.
static bool sinkInstruction(
Loop &L, Instruction &I, const SmallVectorImpl<BasicBlock *> &ColdLoopBBs,
const SmallDenseMap<BasicBlock *, int, 16> &LoopBlockNumber, LoopInfo &LI,
DominatorTree &DT, BlockFrequencyInfo &BFI, MemorySSAUpdater *MSSAU) {
static bool sinkInstruction(Loop &L, Instruction &I,
const SmallVectorImpl<BasicBlock *> &ColdLoopBBs,
const SmallDenseMap<BasicBlock *, int, 16> &LoopBlockNumber,
LoopInfo &LI, DominatorTree &DT,
BlockFrequencyInfo &BFI) {
// Compute the set of blocks in loop L which contain a use of I.
SmallPtrSet<BasicBlock *, 2> BBs;
for (auto &U : I.uses()) {
@ -239,21 +230,6 @@ static bool sinkInstruction(
Instruction *IC = I.clone();
IC->setName(I.getName());
IC->insertBefore(&*N->getFirstInsertionPt());
if (MSSAU && MSSAU->getMemorySSA()->getMemoryAccess(&I)) {
// Create a new MemoryAccess and let MemorySSA set its defining access.
MemoryAccess *NewMemAcc =
MSSAU->createMemoryAccessInBB(IC, nullptr, N, MemorySSA::Beginning);
if (NewMemAcc) {
if (auto *MemDef = dyn_cast<MemoryDef>(NewMemAcc))
MSSAU->insertDef(MemDef, /*RenameUses=*/true);
else {
auto *MemUse = cast<MemoryUse>(NewMemAcc);
MSSAU->insertUse(MemUse, /*RenameUses=*/true);
}
}
}
// Replaces uses of I with IC in N
I.replaceUsesWithIf(IC, [N](Use &U) {
return cast<Instruction>(U.getUser())->getParent() == N;
@ -268,11 +244,6 @@ static bool sinkInstruction(
NumLoopSunk++;
I.moveBefore(&*MoveBB->getFirstInsertionPt());
if (MSSAU)
if (MemoryUseOrDef *OldMemAcc = cast_or_null<MemoryUseOrDef>(
MSSAU->getMemorySSA()->getMemoryAccess(&I)))
MSSAU->moveToPlace(OldMemAcc, MoveBB, MemorySSA::Beginning);
return true;
}
@ -281,11 +252,10 @@ static bool sinkInstruction(
static bool sinkLoopInvariantInstructions(Loop &L, AAResults &AA, LoopInfo &LI,
DominatorTree &DT,
BlockFrequencyInfo &BFI,
ScalarEvolution *SE,
AliasSetTracker *CurAST,
MemorySSA *MSSA) {
ScalarEvolution *SE) {
BasicBlock *Preheader = L.getLoopPreheader();
assert(Preheader && "Expected loop to have preheader");
if (!Preheader)
return false;
// Enable LoopSink only when runtime profile is available.
// With static profile, the sinking decision may be sub-optimal.
@ -301,15 +271,13 @@ static bool sinkLoopInvariantInstructions(Loop &L, AAResults &AA, LoopInfo &LI,
}))
return false;
std::unique_ptr<MemorySSAUpdater> MSSAU;
std::unique_ptr<SinkAndHoistLICMFlags> LICMFlags;
if (MSSA) {
MSSAU = std::make_unique<MemorySSAUpdater>(MSSA);
LICMFlags =
std::make_unique<SinkAndHoistLICMFlags>(/*IsSink=*/true, &L, MSSA);
}
bool Changed = false;
AliasSetTracker CurAST(AA);
// Compute alias set.
for (BasicBlock *BB : L.blocks())
CurAST.add(*BB);
CurAST.add(*Preheader);
// Sort loop's basic blocks by frequency
SmallVector<BasicBlock *, 10> ColdLoopBBs;
@ -332,11 +300,9 @@ static bool sinkLoopInvariantInstructions(Loop &L, AAResults &AA, LoopInfo &LI,
// No need to check for instruction's operands are loop invariant.
assert(L.hasLoopInvariantOperands(I) &&
"Insts in a loop's preheader should have loop invariant operands!");
if (!canSinkOrHoistInst(*I, &AA, &DT, &L, CurAST, MSSAU.get(), false,
LICMFlags.get()))
if (!canSinkOrHoistInst(*I, &AA, &DT, &L, &CurAST, nullptr, false))
continue;
if (sinkInstruction(L, *I, ColdLoopBBs, LoopBlockNumber, LI, DT, BFI,
MSSAU.get()))
if (sinkInstruction(L, *I, ColdLoopBBs, LoopBlockNumber, LI, DT, BFI))
Changed = true;
}
@ -345,13 +311,6 @@ static bool sinkLoopInvariantInstructions(Loop &L, AAResults &AA, LoopInfo &LI,
return Changed;
}
static void computeAliasSet(Loop &L, BasicBlock &Preheader,
AliasSetTracker &CurAST) {
for (BasicBlock *BB : L.blocks())
CurAST.add(*BB);
CurAST.add(Preheader);
}
PreservedAnalyses LoopSinkPass::run(Function &F, FunctionAnalysisManager &FAM) {
LoopInfo &LI = FAM.getResult<LoopAnalysis>(F);
// Nothing to do if there are no loops.
@ -362,10 +321,6 @@ PreservedAnalyses LoopSinkPass::run(Function &F, FunctionAnalysisManager &FAM) {
DominatorTree &DT = FAM.getResult<DominatorTreeAnalysis>(F);
BlockFrequencyInfo &BFI = FAM.getResult<BlockFrequencyAnalysis>(F);
MemorySSA *MSSA = EnableMSSAInLoopSink
? &FAM.getResult<MemorySSAAnalysis>(F).getMSSA()
: nullptr;
// We want to do a postorder walk over the loops. Since loops are a tree this
// is equivalent to a reversed preorder walk and preorder is easy to compute
// without recursion. Since we reverse the preorder, we will visit siblings
@ -377,22 +332,11 @@ PreservedAnalyses LoopSinkPass::run(Function &F, FunctionAnalysisManager &FAM) {
do {
Loop &L = *PreorderLoops.pop_back_val();
BasicBlock *Preheader = L.getLoopPreheader();
if (!Preheader)
continue;
std::unique_ptr<AliasSetTracker> CurAST;
if (!EnableMSSAInLoopSink) {
CurAST = std::make_unique<AliasSetTracker>(AA);
computeAliasSet(L, *Preheader, *CurAST.get());
}
// Note that we don't pass SCEV here because it is only used to invalidate
// loops in SCEV and we don't preserve (or request) SCEV at all making that
// unnecessary.
Changed |= sinkLoopInvariantInstructions(L, AA, LI, DT, BFI,
/*ScalarEvolution*/ nullptr,
CurAST.get(), MSSA);
/*ScalarEvolution*/ nullptr);
} while (!PreorderLoops.empty());
if (!Changed)
@ -400,14 +344,6 @@ PreservedAnalyses LoopSinkPass::run(Function &F, FunctionAnalysisManager &FAM) {
PreservedAnalyses PA;
PA.preserveSet<CFGAnalyses>();
if (MSSA) {
PA.preserve<MemorySSAAnalysis>();
if (VerifyMemorySSA)
MSSA->verifyMemorySSA();
}
return PA;
}
@ -422,41 +358,19 @@ struct LegacyLoopSinkPass : public LoopPass {
if (skipLoop(L))
return false;
BasicBlock *Preheader = L->getLoopPreheader();
if (!Preheader)
return false;
AAResults &AA = getAnalysis<AAResultsWrapperPass>().getAAResults();
auto *SE = getAnalysisIfAvailable<ScalarEvolutionWrapperPass>();
std::unique_ptr<AliasSetTracker> CurAST;
MemorySSA *MSSA = nullptr;
if (EnableMSSAInLegacyLoopSink)
MSSA = &getAnalysis<MemorySSAWrapperPass>().getMSSA();
else {
CurAST = std::make_unique<AliasSetTracker>(AA);
computeAliasSet(*L, *Preheader, *CurAST.get());
}
bool Changed = sinkLoopInvariantInstructions(
*L, AA, getAnalysis<LoopInfoWrapperPass>().getLoopInfo(),
return sinkLoopInvariantInstructions(
*L, getAnalysis<AAResultsWrapperPass>().getAAResults(),
getAnalysis<LoopInfoWrapperPass>().getLoopInfo(),
getAnalysis<DominatorTreeWrapperPass>().getDomTree(),
getAnalysis<BlockFrequencyInfoWrapperPass>().getBFI(),
SE ? &SE->getSE() : nullptr, CurAST.get(), MSSA);
if (MSSA && VerifyMemorySSA)
MSSA->verifyMemorySSA();
return Changed;
SE ? &SE->getSE() : nullptr);
}
void getAnalysisUsage(AnalysisUsage &AU) const override {
AU.setPreservesCFG();
AU.addRequired<BlockFrequencyInfoWrapperPass>();
getLoopAnalysisUsage(AU);
if (EnableMSSAInLegacyLoopSink) {
AU.addRequired<MemorySSAWrapperPass>();
AU.addPreserved<MemorySSAWrapperPass>();
}
}
};
}
@ -466,7 +380,6 @@ INITIALIZE_PASS_BEGIN(LegacyLoopSinkPass, "loop-sink", "Loop Sink", false,
false)
INITIALIZE_PASS_DEPENDENCY(LoopPass)
INITIALIZE_PASS_DEPENDENCY(BlockFrequencyInfoWrapperPass)
INITIALIZE_PASS_DEPENDENCY(MemorySSAWrapperPass)
INITIALIZE_PASS_END(LegacyLoopSinkPass, "loop-sink", "Loop Sink", false, false)
Pass *llvm::createLoopSinkPass() { return new LegacyLoopSinkPass(); }

View File

@ -1,7 +1,4 @@
; RUN: opt -S -loop-sink < %s | FileCheck %s
; RUN: opt -S -verify-memoryssa -enable-mssa-in-legacy-loop-sink -loop-sink < %s | FileCheck %s
; RUN: opt -S -aa-pipeline=basic-aa -passes=loop-sink < %s | FileCheck %s
; RUN: opt -S -verify-memoryssa -enable-mssa-in-loop-sink -aa-pipeline=basic-aa -passes=loop-sink < %s | FileCheck %s
target datalayout = "e-m:w-i64:64-f80:128-n8:16:32:64-S128"
target triple = "x86_64-pc-windows-msvc19.13.26128"

View File

@ -1,7 +1,4 @@
; RUN: opt -S -loop-sink < %s | FileCheck %s
; RUN: opt -S -verify-memoryssa -enable-mssa-in-legacy-loop-sink -loop-sink < %s | FileCheck %s
; RUN: opt -S -aa-pipeline=basic-aa -passes=loop-sink < %s | FileCheck %s
; RUN: opt -S -verify-memoryssa -enable-mssa-in-loop-sink -aa-pipeline=basic-aa -passes=loop-sink < %s | FileCheck %s
; CHECK: pr39570
; Make sure not to assert.

View File

@ -1,7 +1,4 @@
; RUN: opt -S -loop-sink < %s | FileCheck %s
; RUN: opt -S -verify-memoryssa -enable-mssa-in-legacy-loop-sink -loop-sink < %s | FileCheck %s
; RUN: opt -S -aa-pipeline=basic-aa -passes=loop-sink < %s | FileCheck %s
; RUN: opt -S -verify-memoryssa -enable-mssa-in-loop-sink -aa-pipeline=basic-aa -passes=loop-sink < %s | FileCheck %s
; The load instruction should not be sunk into following loop.
; CHECK: @foo

View File

@ -1,7 +1,5 @@
; RUN: opt -S -loop-sink < %s | FileCheck %s
; RUN: opt -S -verify-memoryssa -enable-mssa-in-legacy-loop-sink -loop-sink < %s | FileCheck %s
; RUN: opt -S -aa-pipeline=basic-aa -passes=loop-sink < %s | FileCheck %s
; RUN: opt -S -verify-memoryssa -enable-mssa-in-loop-sink -aa-pipeline=basic-aa -passes=loop-sink < %s | FileCheck %s
@g = global i32 0, align 4