[llvm] a6074b0 - [BasicAA] Drop dependency on Loop Info. PR43276
Max Kazantsev via llvm-commits
llvm-commits at lists.llvm.org
Tue Mar 16 21:44:01 PDT 2021
Author: Max Kazantsev
Date: 2021-03-17T11:43:44+07:00
New Revision: a6074b092cd526c1a5c4dc4237ee867a65339cbf
URL: https://github.com/llvm/llvm-project/commit/a6074b092cd526c1a5c4dc4237ee867a65339cbf
DIFF: https://github.com/llvm/llvm-project/commit/a6074b092cd526c1a5c4dc4237ee867a65339cbf.diff
LOG: [BasicAA] Drop dependency on Loop Info. PR43276
BasicAA stores a reference to LoopInfo inside. This imposes an implicit
requirement of keeping it up to date whenever we modify the IR (in particular,
whenever we modify terminators of blocks that belong to loops). Failing
to do so leads to incorrect state of the LoopInfo.
Because general AA does not require loop info updates and provides to API to
update it properly, the users of AA reasonably assume that there is no need to
update the loop info. It may be a reason of bugs, as example in PR43276 shows.
This patch drops dependence of BasicAA on LoopInfo to avoid this problem.
This may potentially pessimize the result of queries to BasicAA.
Differential Revision: https://reviews.llvm.org/D98627
Reviewed By: nikic
Added:
llvm/test/Transforms/JumpThreading/pr43276.ll
Modified:
llvm/include/llvm/Analysis/BasicAliasAnalysis.h
llvm/lib/Analysis/BasicAliasAnalysis.cpp
llvm/test/Analysis/BasicAA/invalidation.ll
llvm/unittests/Transforms/Vectorize/VPlanSlpTest.cpp
Removed:
################################################################################
diff --git a/llvm/include/llvm/Analysis/BasicAliasAnalysis.h b/llvm/include/llvm/Analysis/BasicAliasAnalysis.h
index 6b5e9d5b71ba..7ed2074badef 100644
--- a/llvm/include/llvm/Analysis/BasicAliasAnalysis.h
+++ b/llvm/include/llvm/Analysis/BasicAliasAnalysis.h
@@ -35,7 +35,6 @@ class DataLayout;
class DominatorTree;
class Function;
class GEPOperator;
-class LoopInfo;
class PHINode;
class SelectInst;
class TargetLibraryInfo;
@@ -56,23 +55,20 @@ class BasicAAResult : public AAResultBase<BasicAAResult> {
const TargetLibraryInfo &TLI;
AssumptionCache &AC;
DominatorTree *DT;
- LoopInfo *LI;
PhiValues *PV;
public:
BasicAAResult(const DataLayout &DL, const Function &F,
const TargetLibraryInfo &TLI, AssumptionCache &AC,
- DominatorTree *DT = nullptr, LoopInfo *LI = nullptr,
- PhiValues *PV = nullptr)
- : AAResultBase(), DL(DL), F(F), TLI(TLI), AC(AC), DT(DT), LI(LI), PV(PV)
- {}
+ DominatorTree *DT = nullptr, PhiValues *PV = nullptr)
+ : AAResultBase(), DL(DL), F(F), TLI(TLI), AC(AC), DT(DT), PV(PV) {}
BasicAAResult(const BasicAAResult &Arg)
: AAResultBase(Arg), DL(Arg.DL), F(Arg.F), TLI(Arg.TLI), AC(Arg.AC),
- DT(Arg.DT), LI(Arg.LI), PV(Arg.PV) {}
+ DT(Arg.DT), PV(Arg.PV) {}
BasicAAResult(BasicAAResult &&Arg)
: AAResultBase(std::move(Arg)), DL(Arg.DL), F(Arg.F), TLI(Arg.TLI),
- AC(Arg.AC), DT(Arg.DT), LI(Arg.LI), PV(Arg.PV) {}
+ AC(Arg.AC), DT(Arg.DT), PV(Arg.PV) {}
/// Handle invalidation events in the new pass manager.
bool invalidate(Function &Fn, const PreservedAnalyses &PA,
diff --git a/llvm/lib/Analysis/BasicAliasAnalysis.cpp b/llvm/lib/Analysis/BasicAliasAnalysis.cpp
index 454f0e477b74..11fa4d2893e6 100644
--- a/llvm/lib/Analysis/BasicAliasAnalysis.cpp
+++ b/llvm/lib/Analysis/BasicAliasAnalysis.cpp
@@ -23,7 +23,6 @@
#include "llvm/Analysis/CFG.h"
#include "llvm/Analysis/CaptureTracking.h"
#include "llvm/Analysis/InstructionSimplify.h"
-#include "llvm/Analysis/LoopInfo.h"
#include "llvm/Analysis/MemoryBuiltins.h"
#include "llvm/Analysis/MemoryLocation.h"
#include "llvm/Analysis/PhiValues.h"
@@ -104,7 +103,6 @@ bool BasicAAResult::invalidate(Function &Fn, const PreservedAnalyses &PA,
// depend on them.
if (Inv.invalidate<AssumptionAnalysis>(Fn, PA) ||
(DT && Inv.invalidate<DominatorTreeAnalysis>(Fn, PA)) ||
- (LI && Inv.invalidate<LoopAnalysis>(Fn, PA)) ||
(PV && Inv.invalidate<PhiValuesAnalysis>(Fn, PA)))
return true;
@@ -1690,7 +1688,7 @@ bool BasicAAResult::isValueEqualInPotentialCycles(const Value *V,
// the Values cannot come from
diff erent iterations of a potential cycle the
// phi nodes could be involved in.
for (auto *P : VisitedPhiBBs)
- if (isPotentiallyReachable(&P->front(), Inst, nullptr, DT, LI))
+ if (isPotentiallyReachable(&P->front(), Inst, nullptr, DT))
return false;
return true;
@@ -1804,9 +1802,8 @@ BasicAAResult BasicAA::run(Function &F, FunctionAnalysisManager &AM) {
auto &TLI = AM.getResult<TargetLibraryAnalysis>(F);
auto &AC = AM.getResult<AssumptionAnalysis>(F);
auto *DT = &AM.getResult<DominatorTreeAnalysis>(F);
- auto *LI = AM.getCachedResult<LoopAnalysis>(F);
auto *PV = AM.getCachedResult<PhiValuesAnalysis>(F);
- return BasicAAResult(F.getParent()->getDataLayout(), F, TLI, AC, DT, LI, PV);
+ return BasicAAResult(F.getParent()->getDataLayout(), F, TLI, AC, DT, PV);
}
BasicAAWrapperPass::BasicAAWrapperPass() : FunctionPass(ID) {
@@ -1834,13 +1831,11 @@ bool BasicAAWrapperPass::runOnFunction(Function &F) {
auto &ACT = getAnalysis<AssumptionCacheTracker>();
auto &TLIWP = getAnalysis<TargetLibraryInfoWrapperPass>();
auto &DTWP = getAnalysis<DominatorTreeWrapperPass>();
- auto *LIWP = getAnalysisIfAvailable<LoopInfoWrapperPass>();
auto *PVWP = getAnalysisIfAvailable<PhiValuesWrapperPass>();
Result.reset(new BasicAAResult(F.getParent()->getDataLayout(), F,
TLIWP.getTLI(F), ACT.getAssumptionCache(F),
&DTWP.getDomTree(),
- LIWP ? &LIWP->getLoopInfo() : nullptr,
PVWP ? &PVWP->getResult() : nullptr));
return false;
diff --git a/llvm/test/Analysis/BasicAA/invalidation.ll b/llvm/test/Analysis/BasicAA/invalidation.ll
index 27e94cb6a2e2..facc0305b4bd 100644
--- a/llvm/test/Analysis/BasicAA/invalidation.ll
+++ b/llvm/test/Analysis/BasicAA/invalidation.ll
@@ -13,18 +13,6 @@
; CHECK-DT-INVALIDATE: Running pass: AAEvaluator
; CHECK-DT-INVALIDATE: Running analysis: BasicAA
;
-; Check LoopInfo specifically.
-; RUN: opt -disable-output -disable-verify -debug-pass-manager %s 2>&1 \
-; RUN: -passes='require<loops>,require<aa>,invalidate<loops>,aa-eval' -aa-pipeline='basic-aa' \
-; RUN: | FileCheck %s --check-prefix=CHECK-LI-INVALIDATE
-; CHECK-LI-INVALIDATE: Running pass: RequireAnalysisPass
-; CHECK-LI-INVALIDATE: Running analysis: BasicAA
-; CHECK-LI-INVALIDATE: Running pass: InvalidateAnalysisPass
-; CHECK-LI-INVALIDATE: Invalidating analysis: LoopAnalysis
-; CHECK-LI-INVALIDATE: Invalidating analysis: BasicAA
-; CHECK-LI-INVALIDATE: Running pass: AAEvaluator
-; CHECK-LI-INVALIDATE: Running analysis: BasicAA
-;
; Check PhiValues specifically.
; RUN: opt -disable-output -disable-verify -debug-pass-manager %s 2>&1 \
; RUN: -passes='require<phi-values>,require<aa>,invalidate<phi-values>,aa-eval' -aa-pipeline='basic-aa' \
diff --git a/llvm/test/Transforms/JumpThreading/pr43276.ll b/llvm/test/Transforms/JumpThreading/pr43276.ll
new file mode 100644
index 000000000000..3d933867e7da
--- /dev/null
+++ b/llvm/test/Transforms/JumpThreading/pr43276.ll
@@ -0,0 +1,87 @@
+; NOTE: Assertions have been autogenerated by utils/update_test_checks.py
+; RUN: opt -S < %s -aa-pipeline=basic-aa -passes='require<loops>,jump-threading' | FileCheck %s
+
+target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128-ni:1"
+target triple = "x86_64-unknown-linux-gnu"
+
+ at global = external global i8*
+
+define i32 @wibble() {
+; CHECK-LABEL: @wibble(
+; CHECK-NEXT: bb19:
+; CHECK-NEXT: [[TMP20:%.*]] = getelementptr i8, i8* undef, i64 16
+; CHECK-NEXT: [[TMP21:%.*]] = load atomic i8*, i8** @global unordered, align 8
+; CHECK-NEXT: [[TMP22:%.*]] = getelementptr inbounds i8, i8* [[TMP21]], i64 936
+; CHECK-NEXT: br label [[BB3:%.*]]
+; CHECK: bb2:
+; CHECK-NEXT: br label [[BB3]]
+; CHECK: bb3:
+; CHECK-NEXT: [[TMP:%.*]] = phi i8* [ [[TMP22]], [[BB19:%.*]] ], [ undef, [[BB2:%.*]] ]
+; CHECK-NEXT: [[TMP4:%.*]] = phi i8* [ [[TMP21]], [[BB19]] ], [ undef, [[BB2]] ]
+; CHECK-NEXT: [[TMP5:%.*]] = bitcast i8* [[TMP]] to i64*
+; CHECK-NEXT: [[TMP6:%.*]] = getelementptr inbounds i8, i8* [[TMP4]], i64 848
+; CHECK-NEXT: [[TMP7:%.*]] = bitcast i8* [[TMP6]] to i8**
+; CHECK-NEXT: br label [[BB11:%.*]]
+; CHECK: bb11:
+; CHECK-NEXT: [[TMP12:%.*]] = load atomic i8*, i8** [[TMP7]] unordered, align 8
+; CHECK-NEXT: [[TMP13:%.*]] = icmp eq i8* [[TMP12]], null
+; CHECK-NEXT: br i1 [[TMP13]], label [[BB17:%.*]], label [[BB16:%.*]]
+; CHECK: bb16:
+; CHECK-NEXT: store atomic i64 undef, i64* [[TMP5]] unordered, align 8
+; CHECK-NEXT: br label [[BB11]]
+; CHECK: bb17:
+; CHECK-NEXT: ret i32 undef
+;
+bb:
+ br label %bb1
+
+bb1: ; preds = %bb
+ br label %bb18
+
+bb2: ; No predecessors!
+ br label %bb3
+
+bb3: ; preds = %bb19, %bb2
+ %tmp = phi i8* [ %tmp22, %bb19 ], [ undef, %bb2 ]
+ %tmp4 = phi i8* [ %tmp21, %bb19 ], [ undef, %bb2 ]
+ %tmp5 = bitcast i8* %tmp to i64*
+ %tmp6 = getelementptr inbounds i8, i8* %tmp4, i64 848
+ %tmp7 = bitcast i8* %tmp6 to i8**
+ br label %bb11
+
+bb11: ; preds = %bb16, %bb3
+ %tmp12 = load atomic i8*, i8** %tmp7 unordered, align 8
+ %tmp13 = icmp eq i8* %tmp12, null
+ br i1 %tmp13, label %bb17, label %bb14
+
+bb14: ; preds = %bb11
+ br label %bb15
+
+bb15: ; preds = %bb14
+ br label %bb16
+
+bb16: ; preds = %bb15
+ store atomic i64 undef, i64* %tmp5 unordered, align 8
+ br label %bb11
+
+bb17: ; preds = %bb11
+ ret i32 undef
+
+bb18: ; preds = %bb1
+ br label %bb19
+
+bb19: ; preds = %bb18
+ %tmp20 = getelementptr i8, i8* undef, i64 16
+ %tmp21 = load atomic i8*, i8** @global unordered, align 8
+ %tmp22 = getelementptr inbounds i8, i8* %tmp21, i64 936
+ br label %bb3
+}
+
+define void @zot(i8* align 8 dereferenceable_or_null(16) %arg, i32 %arg1) {
+; CHECK-LABEL: @zot(
+; CHECK-NEXT: bb:
+; CHECK-NEXT: ret void
+;
+bb:
+ ret void
+}
diff --git a/llvm/unittests/Transforms/Vectorize/VPlanSlpTest.cpp b/llvm/unittests/Transforms/Vectorize/VPlanSlpTest.cpp
index 268f43774d12..af0bef544e38 100644
--- a/llvm/unittests/Transforms/Vectorize/VPlanSlpTest.cpp
+++ b/llvm/unittests/Transforms/Vectorize/VPlanSlpTest.cpp
@@ -40,7 +40,7 @@ class VPlanSlpTest : public VPlanTestBase {
VPlan &Plan) {
AC.reset(new AssumptionCache(F));
SE.reset(new ScalarEvolution(F, TLI, *AC, *DT, *LI));
- BasicAA.reset(new BasicAAResult(DL, F, TLI, *AC, &*DT, &*LI));
+ BasicAA.reset(new BasicAAResult(DL, F, TLI, *AC, &*DT));
AARes.reset(new AAResults(TLI));
AARes->addAAResult(*BasicAA);
PSE.reset(new PredicatedScalarEvolution(*SE, *L));
More information about the llvm-commits
mailing list