Skip to content

Commit

Permalink
[GVN] When merging blocks update LoopInfo if it's available
Browse files Browse the repository at this point in the history
If LoopInfo is available during GVN, BasicAA will use it.  However
MergeBlockIntoPredecessor does not update LI as it merges blocks.

This didn't use to cause problems because LI was freed before
GVN/BasicAA.  Now with OptimizationRemarkEmitter, the lifetime of LI is
extended so LI needs to be kept up-to-date during GVN.

Differential Revision: https://reviews.llvm.org/D27288

llvm-svn: 288307
  • Loading branch information
anemet committed Dec 1, 2016
1 parent 50fb827 commit feafcd9
Show file tree
Hide file tree
Showing 3 changed files with 62 additions and 7 deletions.
3 changes: 2 additions & 1 deletion llvm/include/llvm/Transforms/Scalar/GVN.h
Expand Up @@ -22,6 +22,7 @@
#include "llvm/ADT/SmallPtrSet.h"
#include "llvm/Analysis/AliasAnalysis.h"
#include "llvm/Analysis/AssumptionCache.h"
#include "llvm/Analysis/LoopInfo.h"
#include "llvm/Analysis/MemoryDependenceAnalysis.h"
#include "llvm/IR/Dominators.h"
#include "llvm/IR/IntrinsicInst.h"
Expand Down Expand Up @@ -134,7 +135,7 @@ class GVN : public PassInfoMixin<GVN> {

bool runImpl(Function &F, AssumptionCache &RunAC, DominatorTree &RunDT,
const TargetLibraryInfo &RunTLI, AAResults &RunAA,
MemoryDependenceResults *RunMD);
MemoryDependenceResults *RunMD, LoopInfo *LI);

/// Push a new Value to the LeaderTable onto the list for its value number.
void addToLeaderTable(uint32_t N, Value *V, const BasicBlock *BB) {
Expand Down
16 changes: 10 additions & 6 deletions llvm/lib/Transforms/Scalar/GVN.cpp
Expand Up @@ -586,7 +586,8 @@ PreservedAnalyses GVN::run(Function &F, FunctionAnalysisManager &AM) {
auto &TLI = AM.getResult<TargetLibraryAnalysis>(F);
auto &AA = AM.getResult<AAManager>(F);
auto &MemDep = AM.getResult<MemoryDependenceAnalysis>(F);
bool Changed = runImpl(F, AC, DT, TLI, AA, &MemDep);
auto *LI = AM.getCachedResult<LoopAnalysis>(F);
bool Changed = runImpl(F, AC, DT, TLI, AA, &MemDep, LI);
if (!Changed)
return PreservedAnalyses::all();
PreservedAnalyses PA;
Expand Down Expand Up @@ -2179,7 +2180,7 @@ bool GVN::processInstruction(Instruction *I) {
/// runOnFunction - This is the main transformation entry point for a function.
bool GVN::runImpl(Function &F, AssumptionCache &RunAC, DominatorTree &RunDT,
const TargetLibraryInfo &RunTLI, AAResults &RunAA,
MemoryDependenceResults *RunMD) {
MemoryDependenceResults *RunMD, LoopInfo *LI) {
AC = &RunAC;
DT = &RunDT;
VN.setDomTree(DT);
Expand All @@ -2196,9 +2197,9 @@ bool GVN::runImpl(Function &F, AssumptionCache &RunAC, DominatorTree &RunDT,
for (Function::iterator FI = F.begin(), FE = F.end(); FI != FE; ) {
BasicBlock *BB = &*FI++;

bool removedBlock =
MergeBlockIntoPredecessor(BB, DT, /* LoopInfo */ nullptr, MD);
if (removedBlock) ++NumGVNBlocks;
bool removedBlock = MergeBlockIntoPredecessor(BB, DT, LI, MD);
if (removedBlock)
++NumGVNBlocks;

Changed |= removedBlock;
}
Expand Down Expand Up @@ -2693,13 +2694,16 @@ class llvm::gvn::GVNLegacyPass : public FunctionPass {
if (skipFunction(F))
return false;

auto *LIWP = getAnalysisIfAvailable<LoopInfoWrapperPass>();

return Impl.runImpl(
F, getAnalysis<AssumptionCacheTracker>().getAssumptionCache(F),
getAnalysis<DominatorTreeWrapperPass>().getDomTree(),
getAnalysis<TargetLibraryInfoWrapperPass>().getTLI(),
getAnalysis<AAResultsWrapperPass>().getAAResults(),
NoLoads ? nullptr
: &getAnalysis<MemoryDependenceWrapperPass>().getMemDep());
: &getAnalysis<MemoryDependenceWrapperPass>().getMemDep(),
LIWP ? &LIWP->getLoopInfo() : nullptr);
}

void getAnalysisUsage(AnalysisUsage &AU) const override {
Expand Down
50 changes: 50 additions & 0 deletions llvm/test/Transforms/GVN/stale-loop-info.ll
@@ -0,0 +1,50 @@
; RUN: opt -loops -gvn -S < %s | FileCheck %s

; This used to fail with ASAN enabled and if for some reason LoopInfo remained
; available during GVN. In this case BasicAA will use LI but
; MergeBlockIntoPredecessor in GVN failed to update LI.

target datalayout = "e-m:o-i64:64-f80:128-n8:16:32:64-S128"

%struct.wibble.1028 = type { i32, i32, %struct.barney.881 }
%struct.barney.881 = type { %struct.zot.882 }
%struct.zot.882 = type { [64 x i8] }

; Function Attrs: argmemonly
declare void @snork.1(i8*) local_unnamed_addr #0

define hidden zeroext i1 @eggs(%struct.wibble.1028* %arg, i1 %arg2) unnamed_addr align 2 {
bb:
br i1 %arg2, label %bb14, label %bb3

bb3: ; preds = %bb
%tmp = getelementptr inbounds %struct.wibble.1028, %struct.wibble.1028* %arg, i64 0, i32 2, i32 0, i32 0, i64 0
%tmp5 = bitcast i8* %tmp to %struct.wibble.1028**
br label %bb6

bb6: ; preds = %bb12, %bb3
br label %bb7

bb7: ; preds = %bb6
br i1 undef, label %bb11, label %bb8

bb8: ; preds = %bb7
%tmp9 = load %struct.wibble.1028*, %struct.wibble.1028** %tmp5, align 8
; CHECK: %tmp9 = load %struct.wibble.1028*, %struct.wibble.1028** %tmp5, align 8
%tmp10 = bitcast %struct.wibble.1028* %tmp9 to i8*
br label %bb12

bb11: ; preds = %bb7
br label %bb12

bb12: ; preds = %bb11, %bb8
%tmp13 = phi i8* [ %tmp, %bb11 ], [ %tmp10, %bb8 ]
call void @snork.1(i8* %tmp13) #1
br label %bb6

bb14: ; preds = %bb
ret i1 false
}

attributes #0 = { argmemonly }
attributes #1 = { nounwind }

0 comments on commit feafcd9

Please sign in to comment.