From a2173836acdccbb709328368a657f9d17a58e7c9 Mon Sep 17 00:00:00 2001 From: Krzysztof Pszeniczny Date: Wed, 16 May 2018 13:16:54 +0000 Subject: [PATCH] [BasicAA] Fix handling of invariant group launders Summary: A recent patch ([[ https://reviews.llvm.org/rL331587 | rL331587 ]]) to Capture Tracking taught it that the `launder_invariant_group` intrinsic captures its argument only by returning it. Unfortunately, BasicAA still considered every call instruction as a possible escape source and hence concluded that the result of a `launder_invariant_group` call cannot alias any local non-escaping value. This led to [[ https://bugs.llvm.org/show_bug.cgi?id=37458 | bug 37458 ]]. This patch updates the relevant check for escape sources in BasicAA. Reviewers: Prazek, kuhar, rsmith, hfinkel, sanjoy, xbolva00 Reviewed By: hfinkel, xbolva00 Subscribers: JDevlieghere, hiraditya, llvm-commits Differential Revision: https://reviews.llvm.org/D46900 git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@332466 91177308-0d34-0410-b5e6-96231b3b80d8 --- lib/Analysis/BasicAliasAnalysis.cpp | 13 ++++++- lib/Analysis/CaptureTracking.cpp | 2 + test/Analysis/BasicAA/invariant_group.ll | 48 ++++++++++++++++++++++++ 3 files changed, 62 insertions(+), 1 deletion(-) create mode 100644 test/Analysis/BasicAA/invariant_group.ll diff --git a/lib/Analysis/BasicAliasAnalysis.cpp b/lib/Analysis/BasicAliasAnalysis.cpp index aaa79bc0199..55a53f2122a 100644 --- a/lib/Analysis/BasicAliasAnalysis.cpp +++ b/lib/Analysis/BasicAliasAnalysis.cpp @@ -132,7 +132,18 @@ static bool isNonEscapingLocalObject(const Value *V) { /// Returns true if the pointer is one which would have been considered an /// escape by isNonEscapingLocalObject. static bool isEscapeSource(const Value *V) { - if (isa(V) || isa(V) || isa(V)) + if (auto CS = ImmutableCallSite(V)) { + // launder_invariant_group captures its argument only by returning it, + // so it might not be considered an escape by isNonEscapingLocalObject. + // Note that adding similar special cases for intrinsics in CaptureTracking + // requires handling them here too. + if (CS.getIntrinsicID() == Intrinsic::launder_invariant_group) + return false; + + return true; + } + + if (isa(V)) return true; // The load case works because isNonEscapingLocalObject considers all diff --git a/lib/Analysis/CaptureTracking.cpp b/lib/Analysis/CaptureTracking.cpp index 782e277222f..54466cd3467 100644 --- a/lib/Analysis/CaptureTracking.cpp +++ b/lib/Analysis/CaptureTracking.cpp @@ -249,6 +249,8 @@ void llvm::PointerMayBeCaptured(const Value *V, CaptureTracker *Tracker) { // launder.invariant.group only captures pointer by returning it, // so the pointer wasn't captured if returned pointer is not captured. + // Note that adding similar special cases for intrinsics requires handling + // them in 'isEscapeSource' in BasicAA. if (CS.getIntrinsicID() == Intrinsic::launder_invariant_group) { AddUses(I); break; diff --git a/test/Analysis/BasicAA/invariant_group.ll b/test/Analysis/BasicAA/invariant_group.ll new file mode 100644 index 00000000000..5396afe090b --- /dev/null +++ b/test/Analysis/BasicAA/invariant_group.ll @@ -0,0 +1,48 @@ +; RUN: opt < %s -basicaa -aa-eval -print-all-alias-modref-info -disable-output 2>&1 | FileCheck %s +; RUN: opt < %s -basicaa -gvn -S | FileCheck -check-prefix=CHECK-GVN %s + +; The input *.ll had been adapted from bug 37458: +; +; struct A { virtual void f(); int n; }; +; +; int h() { +; A a; +; a.n = 42; +; return __builtin_launder(&a)->n; +; } + +%struct.A = type <{ i8*, i8 }> + +; CHECK: testLaunderInvariantGroupIsNotEscapeSource +; CHECK-GVN: testLaunderInvariantGroupIsNotEscapeSource +define i8 @testLaunderInvariantGroupIsNotEscapeSource() { +; CHECK-DAG: MustAlias: %struct.A* %a, i8* %a.bitcast +; CHECK-DAG: PartialAlias: %struct.A* %a, i8* %n +; CHECK-DAG: NoAlias: i8* %a.bitcast, i8* %n +; CHECK-DAG: MustAlias: %struct.A* %a, i8* %a.laundered +; CHECK-DAG: MustAlias: i8* %a.bitcast, i8* %a.laundered +; CHECK-DAG: NoAlias: i8* %a.laundered, i8* %n +; CHECK-DAG: PartialAlias: %struct.A* %a, i8* %n.laundered +; CHECK-DAG: NoAlias: i8* %a.bitcast, i8* %n.laundered +; CHECK-DAG: MustAlias: i8* %n, i8* %n.laundered +; CHECK-DAG: NoAlias: i8* %a.laundered, i8* %n.laundered +; CHECK-DAG: NoModRef: Ptr: %struct.A* %a <-> %a.laundered = call i8* @llvm.launder.invariant.group.p0i8(i8* nonnull %a.bitcast) +; CHECK-DAG: NoModRef: Ptr: i8* %a.bitcast <-> %a.laundered = call i8* @llvm.launder.invariant.group.p0i8(i8* nonnull %a.bitcast) +; CHECK-DAG: NoModRef: Ptr: i8* %n <-> %a.laundered = call i8* @llvm.launder.invariant.group.p0i8(i8* nonnull %a.bitcast) +; CHECK-DAG: NoModRef: Ptr: i8* %a.laundered <-> %a.laundered = call i8* @llvm.launder.invariant.group.p0i8(i8* nonnull %a.bitcast) +; CHECK-DAG: NoModRef: Ptr: i8* %n.laundered <-> %a.laundered = call i8* @llvm.launder.invariant.group.p0i8(i8* nonnull %a.bitcast) + +entry: + %a = alloca %struct.A, align 8 + %a.bitcast = bitcast %struct.A* %a to i8* + %n = getelementptr inbounds %struct.A, %struct.A* %a, i64 0, i32 1 + store i8 42, i8* %n + %a.laundered = call i8* @llvm.launder.invariant.group.p0i8(i8* nonnull %a.bitcast) + %n.laundered = getelementptr inbounds i8, i8* %a.laundered, i64 8 + %v = load i8, i8* %n.laundered +; make sure that the load from %n.laundered to %v aliases the store of 42 to %n +; CHECK-GVN: ret i8 42 + ret i8 %v +} + +declare i8* @llvm.launder.invariant.group.p0i8(i8*)