[llvm] eb4f41d - [Attributor] Really use the executed-context
Johannes Doerfert via llvm-commits
llvm-commits at lists.llvm.org
Thu Oct 31 13:10:08 PDT 2019
Author: Johannes Doerfert
Date: 2019-10-31T15:09:45-05:00
New Revision: eb4f41dfe58fc88794e1e227935a6f972f1a50e4
URL: https://github.com/llvm/llvm-project/commit/eb4f41dfe58fc88794e1e227935a6f972f1a50e4
DIFF: https://github.com/llvm/llvm-project/commit/eb4f41dfe58fc88794e1e227935a6f972f1a50e4.diff
LOG: [Attributor] Really use the executed-context
Before we did not follow casts and geps when we looked at the users of a
pointer in the pointers must-be-executed-context. This caused us to fail
to determine if it was accessed for sure. With this change we follow
such users now.
The above extension exposed problems in getKnownNonNullAndDerefBytesForUse
which did not always check what the base pointer was. We also did not
handle negative offsets as conservative as we have to without explicit
loop handling. Finally, we should not derive a huge number if we access
a pointer that was traversed backwards first.
The problems exposed by this functional change are already tested in the
existing test cases as is the functional change.
Differential Revision: https://reviews.llvm.org/D69647
Added:
Modified:
llvm/lib/Transforms/IPO/Attributor.cpp
llvm/test/Transforms/FunctionAttrs/arg_nocapture.ll
llvm/test/Transforms/FunctionAttrs/dereferenceable.ll
llvm/test/Transforms/FunctionAttrs/nocapture.ll
llvm/test/Transforms/FunctionAttrs/nosync.ll
llvm/test/Transforms/InferFunctionAttrs/dereferenceable.ll
Removed:
################################################################################
diff --git a/llvm/lib/Transforms/IPO/Attributor.cpp b/llvm/lib/Transforms/IPO/Attributor.cpp
index ca0ff3ab03e1..6589b16348f4 100644
--- a/llvm/lib/Transforms/IPO/Attributor.cpp
+++ b/llvm/lib/Transforms/IPO/Attributor.cpp
@@ -726,19 +726,16 @@ struct AAFromMustBeExecutedContext : public Base {
MustBeExecutedContextExplorer &Explorer =
A.getInfoCache().getMustBeExecutedContextExplorer();
- SetVector<const Use *> NextUses;
-
auto EIt = Explorer.begin(CtxI), EEnd = Explorer.end(CtxI);
- for (const Use *U : Uses) {
+ for (unsigned u = 0 ; u < Uses.size(); ++u) {
+ const Use *U = Uses[u];
if (const Instruction *UserI = dyn_cast<Instruction>(U->getUser())) {
bool Found = Explorer.findInContextOf(UserI, EIt, EEnd);
if (Found && Base::followUse(A, U, UserI))
for (const Use &Us : UserI->uses())
- NextUses.insert(&Us);
+ Uses.insert(&Us);
}
}
- for (const Use *U : NextUses)
- Uses.insert(U);
return BeforeState == S ? ChangeStatus::UNCHANGED : ChangeStatus::CHANGED;
}
@@ -1550,25 +1547,41 @@ static int64_t getKnownNonNullAndDerefBytesForUse(
return DerefAA.getKnownDereferenceableBytes();
}
+ // We need to follow common pointer manipulation uses to the accesses they
+ // feed into. We can try to be smart to avoid looking through things we do not
+ // like for now, e.g., non-inbounds GEPs.
+ if (isa<CastInst>(I)) {
+ TrackUse = true;
+ return 0;
+ }
+ if (auto *GEP = dyn_cast<GetElementPtrInst>(I))
+ if (GEP->hasAllZeroIndices() ||
+ (GEP->isInBounds() && GEP->hasAllConstantIndices())) {
+ TrackUse = true;
+ return 0;
+ }
+
int64_t Offset;
if (const Value *Base = getBasePointerOfAccessPointerOperand(I, Offset, DL)) {
if (Base == &AssociatedValue && getPointerOperand(I) == UseV) {
int64_t DerefBytes =
- Offset + (int64_t)DL.getTypeStoreSize(PtrTy->getPointerElementType());
+ (int64_t)DL.getTypeStoreSize(PtrTy->getPointerElementType()) + Offset;
IsNonNull |= !NullPointerIsDefined;
- return DerefBytes;
+ return std::max(int64_t(0), DerefBytes);
}
}
if (const Value *Base =
GetPointerBaseWithConstantOffset(UseV, Offset, DL,
/*AllowNonInbounds*/ false)) {
- auto &DerefAA =
- A.getAAFor<AADereferenceable>(QueryingAA, IRPosition::value(*Base));
- IsNonNull |= (!NullPointerIsDefined && DerefAA.isKnownNonNull());
- IsNonNull |= (!NullPointerIsDefined && (Offset != 0));
- int64_t DerefBytes = DerefAA.getKnownDereferenceableBytes();
- return std::max(int64_t(0), DerefBytes - Offset);
+ if (Base == &AssociatedValue) {
+ auto &DerefAA =
+ A.getAAFor<AADereferenceable>(QueryingAA, IRPosition::value(*Base));
+ IsNonNull |= (!NullPointerIsDefined && DerefAA.isKnownNonNull());
+ IsNonNull |= (!NullPointerIsDefined && (Offset != 0));
+ int64_t DerefBytes = DerefAA.getKnownDereferenceableBytes();
+ return std::max(int64_t(0), DerefBytes - std::max(int64_t(0), Offset));
+ }
}
return 0;
@@ -1586,6 +1599,8 @@ struct AANonNullImpl : AANonNull {
if (!NullIsDefined &&
hasAttr({Attribute::NonNull, Attribute::Dereferenceable}))
indicateOptimisticFixpoint();
+ else if (isa<ConstantPointerNull>(getAssociatedValue()))
+ indicatePessimisticFixpoint();
else
AANonNull::initialize(A);
}
diff --git a/llvm/test/Transforms/FunctionAttrs/arg_nocapture.ll b/llvm/test/Transforms/FunctionAttrs/arg_nocapture.ll
index 021c4df533e1..18a4f7671472 100644
--- a/llvm/test/Transforms/FunctionAttrs/arg_nocapture.ll
+++ b/llvm/test/Transforms/FunctionAttrs/arg_nocapture.ll
@@ -116,7 +116,7 @@ entry:
;
; CHECK: define dereferenceable_or_null(8) i64* @scc_B(double* readnone returned dereferenceable_or_null(8) "no-capture-maybe-returned" %a)
;
-; CHECK: define dereferenceable_or_null(2) i8* @scc_C(i16* readnone returned dereferenceable_or_null(2) "no-capture-maybe-returned" %a)
+; CHECK: define dereferenceable_or_null(4) i8* @scc_C(i16* readnone returned dereferenceable_or_null(4) "no-capture-maybe-returned" %a)
;
; float *scc_A(int *a) {
; return (float*)(a ? (int*)scc_A((int*)scc_B((double*)scc_C((short*)a))) : a);
@@ -260,7 +260,7 @@ entry:
; }
;
; There should *not* be a no-capture attribute on %a
-; CHECK: define nonnull i64* @not_captured_but_returned_1(i64* writeonly "no-capture-maybe-returned" %a)
+; CHECK: define nonnull dereferenceable(8) i64* @not_captured_but_returned_1(i64* nonnull writeonly dereferenceable(16) "no-capture-maybe-returned" %a)
define i64* @not_captured_but_returned_1(i64* %a) #0 {
entry:
%add.ptr = getelementptr inbounds i64, i64* %a, i64 1
@@ -320,7 +320,7 @@ entry:
; }
;
; There should *not* be a no-capture attribute on %a
-; CHECK: define nonnull i64* @negative_test_not_captured_but_returned_call_1a(i64* writeonly "no-capture-maybe-returned" %a)
+; CHECK: define nonnull dereferenceable(8) i64* @negative_test_not_captured_but_returned_call_1a(i64* writeonly "no-capture-maybe-returned" %a)
define i64* @negative_test_not_captured_but_returned_call_1a(i64* %a) #0 {
entry:
%call = call i64* @not_captured_but_returned_1(i64* %a)
diff --git a/llvm/test/Transforms/FunctionAttrs/dereferenceable.ll b/llvm/test/Transforms/FunctionAttrs/dereferenceable.ll
index a8608478430a..973f7f517e61 100644
--- a/llvm/test/Transforms/FunctionAttrs/dereferenceable.ll
+++ b/llvm/test/Transforms/FunctionAttrs/dereferenceable.ll
@@ -30,8 +30,8 @@ define i32* @test3_1(i32* dereferenceable(8) %0) local_unnamed_addr {
}
define i32* @test3_2(i32* dereferenceable_or_null(32) %0) local_unnamed_addr {
-; FIXME: Argument should be mark dereferenceable because of GEP `inbounds`.
-; ATTRIBUTOR: define nonnull dereferenceable(16) i32* @test3_2(i32* readnone dereferenceable_or_null(32) "no-capture-maybe-returned" %0)
+; FIXME: We should not have both deref(x) and deref_or_null(y) with x >= y.
+; ATTRIBUTOR: define nonnull dereferenceable(16) i32* @test3_2(i32* nonnull readnone dereferenceable(32) dereferenceable_or_null(32) "no-capture-maybe-returned" %0)
%ret = getelementptr inbounds i32, i32* %0, i64 4
ret i32* %ret
}
@@ -196,8 +196,8 @@ define i32* @f7_3() {
}
define i32* @test_for_minus_index(i32* %p) {
-; FIXME: This should be define nonnull dereferenceable(8) i32* @test_for_minus_index(i32* nonnull %p)
-; ATTRIBUTOR: define nonnull dereferenceable(8) i32* @test_for_minus_index(i32* writeonly "no-capture-maybe-returned" %p)
+; FIXME: This should have a return dereferenceable(8) but we need to make sure it will work in loops as well.
+; ATTRIBUTOR: define nonnull i32* @test_for_minus_index(i32* nonnull writeonly "no-capture-maybe-returned" %p)
%q = getelementptr inbounds i32, i32* %p, i32 -2
store i32 1, i32* %q
ret i32* %q
diff --git a/llvm/test/Transforms/FunctionAttrs/nocapture.ll b/llvm/test/Transforms/FunctionAttrs/nocapture.ll
index f8927a60cb14..4fb455896485 100644
--- a/llvm/test/Transforms/FunctionAttrs/nocapture.ll
+++ b/llvm/test/Transforms/FunctionAttrs/nocapture.ll
@@ -306,7 +306,8 @@ define i1 @captureICmpRev(i32* %x) {
ret i1 %1
}
-; EITHER: define i1 @nocaptureInboundsGEPICmp(i32* nocapture readnone %x)
+; FNATTR: define i1 @nocaptureInboundsGEPICmp(i32* nocapture readnone %x)
+; ATTRIBUTOR: define i1 @nocaptureInboundsGEPICmp(i32* nocapture nonnull readnone %x)
define i1 @nocaptureInboundsGEPICmp(i32* %x) {
%1 = getelementptr inbounds i32, i32* %x, i32 5
%2 = bitcast i32* %1 to i8*
@@ -314,7 +315,8 @@ define i1 @nocaptureInboundsGEPICmp(i32* %x) {
ret i1 %3
}
-; EITHER: define i1 @nocaptureInboundsGEPICmpRev(i32* nocapture readnone %x)
+; FNATTR: define i1 @nocaptureInboundsGEPICmpRev(i32* nocapture readnone %x)
+; ATTRIBUTOR: define i1 @nocaptureInboundsGEPICmpRev(i32* nocapture nonnull readnone %x)
define i1 @nocaptureInboundsGEPICmpRev(i32* %x) {
%1 = getelementptr inbounds i32, i32* %x, i32 5
%2 = bitcast i32* %1 to i8*
diff --git a/llvm/test/Transforms/FunctionAttrs/nosync.ll b/llvm/test/Transforms/FunctionAttrs/nosync.ll
index 381c5abb2614..253b34d83080 100644
--- a/llvm/test/Transforms/FunctionAttrs/nosync.ll
+++ b/llvm/test/Transforms/FunctionAttrs/nosync.ll
@@ -28,7 +28,7 @@ target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
; FNATTR: Function Attrs: norecurse nounwind optsize readnone ssp uwtable
; FNATTR-NEXT: define nonnull i32* @foo(%struct.ST* readnone %s)
; ATTRIBUTOR: Function Attrs: nofree nosync nounwind optsize readnone ssp uwtable
-; ATTRIBUTOR-NEXT: define nonnull i32* @foo(%struct.ST* readnone "no-capture-maybe-returned" %s)
+; ATTRIBUTOR-NEXT: define nonnull i32* @foo(%struct.ST* nonnull readnone "no-capture-maybe-returned" %s)
define i32* @foo(%struct.ST* %s) nounwind uwtable readnone optsize ssp {
entry:
%arrayidx = getelementptr inbounds %struct.ST, %struct.ST* %s, i64 1, i32 2, i32 1, i64 5, i64 13
@@ -224,7 +224,7 @@ define void @scc2(i32* %0) noinline nounwind uwtable {
; FNATTR: Function Attrs: nofree norecurse nounwind
; FNATTR-NEXT: define void @foo1(i32* nocapture %0, %"struct.std::atomic"* nocapture %1)
; ATTRIBUTOR-NOT: nosync
-; ATTRIBUTOR: define void @foo1(i32* nocapture nonnull writeonly dereferenceable(4) %0, %"struct.std::atomic"* nocapture writeonly %1)
+; ATTRIBUTOR: define void @foo1(i32* nocapture nonnull writeonly dereferenceable(4) %0, %"struct.std::atomic"* nocapture nonnull writeonly dereferenceable(1) %1)
define void @foo1(i32* %0, %"struct.std::atomic"* %1) {
store i32 100, i32* %0, align 4
@@ -237,7 +237,7 @@ define void @foo1(i32* %0, %"struct.std::atomic"* %1) {
; FNATTR: Function Attrs: nofree norecurse nounwind
; FNATTR-NEXT: define void @bar(i32* nocapture readnone %0, %"struct.std::atomic"* nocapture readonly %1)
; ATTRIBUTOR-NOT: nosync
-; ATTRIBUTOR: define void @bar(i32* nocapture readnone %0, %"struct.std::atomic"* nocapture readonly %1)
+; ATTRIBUTOR: define void @bar(i32* nocapture readnone %0, %"struct.std::atomic"* nocapture nonnull readonly dereferenceable(1) %1)
define void @bar(i32* %0, %"struct.std::atomic"* %1) {
%3 = getelementptr inbounds %"struct.std::atomic", %"struct.std::atomic"* %1, i64 0, i32 0, i32 0
br label %4
@@ -257,7 +257,7 @@ define void @bar(i32* %0, %"struct.std::atomic"* %1) {
; FNATTR: Function Attrs: nofree norecurse nounwind
; FNATTR-NEXT: define void @foo1_singlethread(i32* nocapture %0, %"struct.std::atomic"* nocapture %1)
; ATTRIBUTOR: Function Attrs: nofree nosync nounwind willreturn
-; ATTRIBUTOR: define void @foo1_singlethread(i32* nocapture nonnull writeonly dereferenceable(4) %0, %"struct.std::atomic"* nocapture writeonly %1)
+; ATTRIBUTOR: define void @foo1_singlethread(i32* nocapture nonnull writeonly dereferenceable(4) %0, %"struct.std::atomic"* nocapture nonnull writeonly dereferenceable(1) %1)
define void @foo1_singlethread(i32* %0, %"struct.std::atomic"* %1) {
store i32 100, i32* %0, align 4
@@ -270,7 +270,7 @@ define void @foo1_singlethread(i32* %0, %"struct.std::atomic"* %1) {
; FNATTR: Function Attrs: nofree norecurse nounwind
; FNATTR-NEXT: define void @bar_singlethread(i32* nocapture readnone %0, %"struct.std::atomic"* nocapture readonly %1)
; ATTRIBUTOR: Function Attrs: nofree nosync nounwind
-; ATTRIBUTOR: define void @bar_singlethread(i32* nocapture readnone %0, %"struct.std::atomic"* nocapture readonly %1)
+; ATTRIBUTOR: define void @bar_singlethread(i32* nocapture readnone %0, %"struct.std::atomic"* nocapture nonnull readonly dereferenceable(1) %1)
define void @bar_singlethread(i32* %0, %"struct.std::atomic"* %1) {
%3 = getelementptr inbounds %"struct.std::atomic", %"struct.std::atomic"* %1, i64 0, i32 0, i32 0
br label %4
diff --git a/llvm/test/Transforms/InferFunctionAttrs/dereferenceable.ll b/llvm/test/Transforms/InferFunctionAttrs/dereferenceable.ll
index 17c76f11d6c8..25412051a7f6 100644
--- a/llvm/test/Transforms/InferFunctionAttrs/dereferenceable.ll
+++ b/llvm/test/Transforms/InferFunctionAttrs/dereferenceable.ll
@@ -8,9 +8,7 @@
define <4 x double> @PR21780(double* %ptr) {
; CHECK-LABEL: @PR21780(double* %ptr)
-; FIXME: this should be @PR21780(double* nonnull dereferenceable(32) %ptr)
-; trakcing use of GEP in Attributor would fix this problem.
-; ATTRIBUTOR-LABEL: @PR21780(double* nocapture nonnull readonly dereferenceable(8) %ptr)
+; ATTRIBUTOR-LABEL: @PR21780(double* nocapture nonnull readonly dereferenceable(32) %ptr)
; GEP of index 0 is simplified away.
%arrayidx1 = getelementptr inbounds double, double* %ptr, i64 1
@@ -33,9 +31,7 @@ define <4 x double> @PR21780(double* %ptr) {
define double @PR21780_only_access3_with_inbounds(double* %ptr) {
; CHECK-LABEL: @PR21780_only_access3_with_inbounds(double* %ptr)
-; FIXME: this should be @PR21780_only_access3_with_inbounds(double* nonnull dereferenceable(32) %ptr)
-; trakcing use of GEP in Attributor would fix this problem.
-; ATTRIBUTOR-LABEL: @PR21780_only_access3_with_inbounds(double* nocapture readonly %ptr)
+; ATTRIBUTOR-LABEL: @PR21780_only_access3_with_inbounds(double* nocapture nonnull readonly dereferenceable(32) %ptr)
%arrayidx3 = getelementptr inbounds double, double* %ptr, i64 3
%t3 = load double, double* %arrayidx3, align 8
More information about the llvm-commits
mailing list