[llvm] 4479c0c - Allow nonnull/align attribute to accept poison
Juneyoung Lee via llvm-commits
llvm-commits at lists.llvm.org
Tue Jan 19 18:54:41 PST 2021
Author: Juneyoung Lee
Date: 2021-01-20T11:31:23+09:00
New Revision: 4479c0c2c0be019b9932c6f1380a40e6cb48da25
URL: https://github.com/llvm/llvm-project/commit/4479c0c2c0be019b9932c6f1380a40e6cb48da25
DIFF: https://github.com/llvm/llvm-project/commit/4479c0c2c0be019b9932c6f1380a40e6cb48da25.diff
LOG: Allow nonnull/align attribute to accept poison
Currently LLVM is relying on ValueTracking's `isKnownNonZero` to attach `nonnull`, which can return true when the value is poison.
To make the semantics of `nonnull` consistent with the behavior of `isKnownNonZero`, this makes the semantics of `nonnull` to accept poison, and return poison if the input pointer isn't null.
This makes many transformations like below legal:
```
%p = gep inbounds %x, 1 ; % p is non-null pointer or poison
call void @f(%p) ; instcombine converts this to call void @f(nonnull %p)
```
Instead, this semantics makes propagation of `nonnull` to caller illegal.
The reason is that, passing poison to `nonnull` does not immediately raise UB anymore, so such program is still well defined, if the callee does not use the argument.
Having `noundef` attribute there re-allows this.
```
define void @f(i8* %p) { ; functionattr cannot mark %p nonnull here anymore
call void @g(i8* nonnull %p) ; .. because @g never raises UB if it never uses %p.
ret void
}
```
Another attribute that needs to be updated is `align`. This patch updates the semantics of align to accept poison as well.
Reviewed By: jdoerfert
Differential Revision: https://reviews.llvm.org/D90529
Added:
Modified:
llvm/docs/LangRef.rst
llvm/include/llvm/IR/Argument.h
llvm/lib/Analysis/ValueTracking.cpp
llvm/lib/IR/Function.cpp
llvm/lib/Transforms/IPO/FunctionAttrs.cpp
llvm/test/Analysis/ValueTracking/known-nonnull-at.ll
llvm/test/Transforms/Attributor/align.ll
llvm/test/Transforms/Attributor/nonnull.ll
llvm/test/Transforms/FunctionAttrs/nonnull.ll
llvm/test/Transforms/InstCombine/call_nonnull_arg.ll
llvm/test/Transforms/InstCombine/unused-nonnull.ll
Removed:
################################################################################
diff --git a/llvm/docs/LangRef.rst b/llvm/docs/LangRef.rst
index 1b6052f58f9d..cd3bb0de4f34 100644
--- a/llvm/docs/LangRef.rst
+++ b/llvm/docs/LangRef.rst
@@ -1160,10 +1160,12 @@ Currently, only the following parameter attributes are defined:
.. _attr_align:
``align <n>`` or ``align(<n>)``
- This indicates that the pointer value may be assumed by the optimizer to
- have the specified alignment. If the pointer value does not have the
- specified alignment, behavior is undefined. ``align 1`` has no effect on
- non-byval, non-preallocated arguments.
+ This indicates that the pointer value has the specified alignment.
+ If the pointer value does not have the specified alignment,
+ :ref:`poison value <poisonvalues>` is returned or passed instead. The
+ ``align`` attribute should be combined with the ``noundef`` attribute to
+ ensure a pointer is aligned, or otherwise the behavior is undefined. Note
+ that ``align 1`` has no effect on non-byval, non-preallocated arguments.
Note that this attribute has additional semantics when combined with the
``byval`` or ``preallocated`` attribute, which are documented there.
@@ -1225,7 +1227,9 @@ Currently, only the following parameter attributes are defined:
This indicates that the parameter or return pointer is not null. This
attribute may only be applied to pointer typed parameters. This is not
checked or enforced by LLVM; if the parameter or return pointer is null,
- the behavior is undefined.
+ :ref:`poison value <poisonvalues>` is returned or passed instead.
+ The ``nonnull`` attribute should be combined with the ``noundef`` attribute
+ to ensure a pointer is not null or otherwise the behavior is undefined.
``dereferenceable(<n>)``
This indicates that the parameter or return pointer is dereferenceable. This
diff --git a/llvm/include/llvm/IR/Argument.h b/llvm/include/llvm/IR/Argument.h
index f59a498dc75d..76d780485ea0 100644
--- a/llvm/include/llvm/IR/Argument.h
+++ b/llvm/include/llvm/IR/Argument.h
@@ -52,7 +52,9 @@ class Argument final : public Value {
/// Return true if this argument has the nonnull attribute. Also returns true
/// if at least one byte is known to be dereferenceable and the pointer is in
/// addrspace(0).
- bool hasNonNullAttr() const;
+ /// If AllowUndefOrPoison is true, respect the semantics of nonnull attribute
+ /// and return true even if the argument can be undef or poison.
+ bool hasNonNullAttr(bool AllowUndefOrPoison = true) const;
/// If this argument has the dereferenceable attribute, return the number of
/// bytes known to be dereferenceable. Otherwise, zero is returned.
diff --git a/llvm/lib/Analysis/ValueTracking.cpp b/llvm/lib/Analysis/ValueTracking.cpp
index 4f0c7057089b..ef3558ef136e 100644
--- a/llvm/lib/Analysis/ValueTracking.cpp
+++ b/llvm/lib/Analysis/ValueTracking.cpp
@@ -2088,7 +2088,8 @@ static bool isKnownNonNullFromDominatingCondition(const Value *V,
if (auto *CalledFunc = CB->getCalledFunction())
for (const Argument &Arg : CalledFunc->args())
if (CB->getArgOperand(Arg.getArgNo()) == V &&
- Arg.hasNonNullAttr() && DT->dominates(CB, CtxI))
+ Arg.hasNonNullAttr(/* AllowUndefOrPoison */ false) &&
+ DT->dominates(CB, CtxI))
return true;
// If the value is used as a load/store, then the pointer must be non null.
diff --git a/llvm/lib/IR/Function.cpp b/llvm/lib/IR/Function.cpp
index 2f02b4e175da..17247123f87f 100644
--- a/llvm/lib/IR/Function.cpp
+++ b/llvm/lib/IR/Function.cpp
@@ -87,9 +87,11 @@ void Argument::setParent(Function *parent) {
Parent = parent;
}
-bool Argument::hasNonNullAttr() const {
+bool Argument::hasNonNullAttr(bool AllowUndefOrPoison) const {
if (!getType()->isPointerTy()) return false;
- if (getParent()->hasParamAttribute(getArgNo(), Attribute::NonNull))
+ if (getParent()->hasParamAttribute(getArgNo(), Attribute::NonNull) &&
+ (AllowUndefOrPoison ||
+ getParent()->hasParamAttribute(getArgNo(), Attribute::NoUndef)))
return true;
else if (getDereferenceableBytes() > 0 &&
!NullPointerIsDefined(getParent(),
diff --git a/llvm/lib/Transforms/IPO/FunctionAttrs.cpp b/llvm/lib/Transforms/IPO/FunctionAttrs.cpp
index 2e24cad1393b..210186a0550e 100644
--- a/llvm/lib/Transforms/IPO/FunctionAttrs.cpp
+++ b/llvm/lib/Transforms/IPO/FunctionAttrs.cpp
@@ -642,7 +642,7 @@ static bool addArgumentAttrsFromCallsites(Function &F) {
if (auto *CB = dyn_cast<CallBase>(&I)) {
if (auto *CalledFunc = CB->getCalledFunction()) {
for (auto &CSArg : CalledFunc->args()) {
- if (!CSArg.hasNonNullAttr())
+ if (!CSArg.hasNonNullAttr(/* AllowUndefOrPoison */ false))
continue;
// If the non-null callsite argument operand is an argument to 'F'
diff --git a/llvm/test/Analysis/ValueTracking/known-nonnull-at.ll b/llvm/test/Analysis/ValueTracking/known-nonnull-at.ll
index 258d8e47c872..41613136f89f 100644
--- a/llvm/test/Analysis/ValueTracking/known-nonnull-at.ll
+++ b/llvm/test/Analysis/ValueTracking/known-nonnull-at.ll
@@ -1,7 +1,8 @@
; NOTE: Assertions have been autogenerated by utils/update_test_checks.py
; RUN: opt -S -instsimplify < %s | FileCheck %s
-declare void @bar(i8* %a, i8* nonnull %b)
+declare void @bar(i8* %a, i8* nonnull noundef %b)
+declare void @bar_without_noundef(i8* %a, i8* nonnull %b)
; 'y' must be nonnull.
@@ -17,6 +18,19 @@ define i1 @caller1(i8* %x, i8* %y) {
; Don't know anything about 'y'.
+define i1 @caller1_maybepoison(i8* %x, i8* %y) {
+; CHECK-LABEL: @caller1_maybepoison(
+; CHECK-NEXT: call void @bar_without_noundef(i8* [[X:%.*]], i8* [[Y:%.*]])
+; CHECK-NEXT: [[NULL_CHECK:%.*]] = icmp eq i8* [[Y]], null
+; CHECK-NEXT: ret i1 [[NULL_CHECK]]
+;
+ call void @bar_without_noundef(i8* %x, i8* %y)
+ %null_check = icmp eq i8* %y, null
+ ret i1 %null_check
+}
+
+; Don't know anything about 'y'.
+
define i1 @caller2(i8* %x, i8* %y) {
; CHECK-LABEL: @caller2(
; CHECK-NEXT: call void @bar(i8* [[Y:%.*]], i8* [[X:%.*]])
@@ -187,7 +201,7 @@ define i8* @test_load_store_after_check(i8* %0) {
; CHECK-NEXT: [[NULL_CHECK:%.*]] = icmp eq i8* [[TMP1]], null
; CHECK-NEXT: br i1 [[NULL_CHECK]], label [[RETURN:%.*]], label [[IF_END:%.*]]
; CHECK: if.end:
-; CHECK-NEXT: store i8 7, i8* [[TMP1]]
+; CHECK-NEXT: store i8 7, i8* [[TMP1]], align 1
; CHECK-NEXT: br label [[RETURN]]
; CHECK: return:
; CHECK-NEXT: [[RETVAL_0:%.*]] = phi i8* [ [[TMP1]], [[IF_END]] ], [ null, [[ENTRY:%.*]] ]
diff --git a/llvm/test/Transforms/Attributor/align.ll b/llvm/test/Transforms/Attributor/align.ll
index 0c36d4a5d07e..5318a829a762 100644
--- a/llvm/test/Transforms/Attributor/align.ll
+++ b/llvm/test/Transforms/Attributor/align.ll
@@ -1041,6 +1041,19 @@ return: ; preds = %entry, %if.then
ret i32* %retval.0
}
+; FIXME: align 4 should not be propagated to the caller's p unless there is noundef
+define void @align4_caller(i8* %p) {
+; CHECK-LABEL: define {{[^@]+}}@align4_caller
+; CHECK-SAME: (i8* align 4 [[P:%.*]]) {
+; CHECK-NEXT: call void @align4_callee(i8* align 4 [[P]])
+; CHECK-NEXT: ret void
+;
+ call void @align4_callee(i8* %p)
+ ret void
+}
+
+declare void @align4_callee(i8* align(4) %p)
+
attributes #0 = { nounwind uwtable noinline }
attributes #1 = { uwtable noinline }
diff --git a/llvm/test/Transforms/Attributor/nonnull.ll b/llvm/test/Transforms/Attributor/nonnull.ll
index 23653c80299a..cc3ae684b09f 100644
--- a/llvm/test/Transforms/Attributor/nonnull.ll
+++ b/llvm/test/Transforms/Attributor/nonnull.ll
@@ -1654,5 +1654,18 @@ define i8* @nonnull_function_ptr_2() {
ret i8* %bc
}
+; FIXME: nonnull should not be propagated to the caller's p unless there is noundef
+define void @nonnull_caller(i8* %p) {
+; CHECK-LABEL: define {{[^@]+}}@nonnull_caller
+; CHECK-SAME: (i8* nonnull [[P:%.*]]) {
+; CHECK-NEXT: call void @nonnull_callee(i8* nonnull [[P]])
+; CHECK-NEXT: ret void
+;
+ call void @nonnull_callee(i8* %p)
+ ret void
+}
+
+declare void @nonnull_callee(i8* nonnull %p)
+
attributes #0 = { null_pointer_is_valid }
attributes #1 = { nounwind willreturn}
diff --git a/llvm/test/Transforms/FunctionAttrs/nonnull.ll b/llvm/test/Transforms/FunctionAttrs/nonnull.ll
index 9e3958022995..56457b33d1c6 100644
--- a/llvm/test/Transforms/FunctionAttrs/nonnull.ll
+++ b/llvm/test/Transforms/FunctionAttrs/nonnull.ll
@@ -327,12 +327,21 @@ declare void @use1(i8* %x)
declare void @use2(i8* %x, i8* %y);
declare void @use3(i8* %x, i8* %y, i8* %z);
-declare void @use1nonnull(i8* nonnull %x);
-declare void @use2nonnull(i8* nonnull %x, i8* nonnull %y);
-declare void @use3nonnull(i8* nonnull %x, i8* nonnull %y, i8* nonnull %z);
+declare void @use1nonnull(i8* nonnull noundef %x);
+declare void @use1nonnull_without_noundef(i8* nonnull %x);
+declare void @use2nonnull(i8* nonnull noundef %x, i8* nonnull noundef %y);
+declare void @use3nonnull(i8* nonnull noundef %x, i8* nonnull noundef %y, i8* nonnull noundef %z);
declare i8 @use1safecall(i8* %x) readonly nounwind ; readonly+nounwind guarantees that execution continues to successor
+; Without noundef, nonnull cannot be propagated to the parent
+
+define void @parent_poison(i8* %a) {
+; FNATTR-LABEL: @parent_poison(i8* %a)
+ call void @use1nonnull_without_noundef(i8* %a)
+ ret void
+}
+
; Can't extend non-null to parent for any argument because the 2nd call is not guaranteed to execute.
define void @parent1(i8* %a, i8* %b, i8* %c) {
diff --git a/llvm/test/Transforms/InstCombine/call_nonnull_arg.ll b/llvm/test/Transforms/InstCombine/call_nonnull_arg.ll
index 8127f4734fcd..080bb1e7ac04 100644
--- a/llvm/test/Transforms/InstCombine/call_nonnull_arg.ll
+++ b/llvm/test/Transforms/InstCombine/call_nonnull_arg.ll
@@ -7,13 +7,13 @@ declare void @dummy(i32*, i32)
define void @test(i32* %a, i32 %b) {
; CHECK-LABEL: @test(
; CHECK-NEXT: entry:
-; CHECK-NEXT: [[COND1:%.*]] = icmp eq i32* %a, null
-; CHECK-NEXT: br i1 [[COND1]], label %dead, label %not_null
+; CHECK-NEXT: [[COND1:%.*]] = icmp eq i32* [[A:%.*]], null
+; CHECK-NEXT: br i1 [[COND1]], label [[DEAD:%.*]], label [[NOT_NULL:%.*]]
; CHECK: not_null:
-; CHECK-NEXT: [[COND2:%.*]] = icmp eq i32 %b, 0
-; CHECK-NEXT: br i1 [[COND2]], label %dead, label %not_zero
+; CHECK-NEXT: [[COND2:%.*]] = icmp eq i32 [[B:%.*]], 0
+; CHECK-NEXT: br i1 [[COND2]], label [[DEAD]], label [[NOT_ZERO:%.*]]
; CHECK: not_zero:
-; CHECK-NEXT: call void @dummy(i32* nonnull %a, i32 %b)
+; CHECK-NEXT: call void @dummy(i32* nonnull [[A]], i32 [[B]])
; CHECK-NEXT: ret void
; CHECK: dead:
; CHECK-NEXT: unreachable
@@ -31,16 +31,17 @@ dead:
unreachable
}
-; The nonnull attribute in the 'bar' declaration is
-; propagated to the parameters of the 'baz' callsite.
+; The nonnull attribute in the 'bar' declaration is
+; propagated to the parameters of the 'baz' callsite.
-declare void @bar(i8*, i8* nonnull)
+declare void @bar(i8*, i8* nonnull noundef)
+declare void @bar_without_noundef(i8*, i8* nonnull)
declare void @baz(i8*, i8*)
define void @deduce_nonnull_from_another_call(i8* %a, i8* %b) {
; CHECK-LABEL: @deduce_nonnull_from_another_call(
-; CHECK-NEXT: call void @bar(i8* %a, i8* %b)
-; CHECK-NEXT: call void @baz(i8* nonnull %b, i8* nonnull %b)
+; CHECK-NEXT: call void @bar(i8* [[A:%.*]], i8* [[B:%.*]])
+; CHECK-NEXT: call void @baz(i8* nonnull [[B]], i8* nonnull [[B]])
; CHECK-NEXT: ret void
;
call void @bar(i8* %a, i8* %b)
@@ -48,3 +49,15 @@ define void @deduce_nonnull_from_another_call(i8* %a, i8* %b) {
ret void
}
+
+define void @deduce_nonnull_from_another_call2(i8* %a, i8* %b) {
+; CHECK-LABEL: @deduce_nonnull_from_another_call2(
+; CHECK-NEXT: call void @bar_without_noundef(i8* [[A:%.*]], i8* [[B:%.*]])
+; CHECK-NEXT: call void @baz(i8* [[B]], i8* [[B]])
+; CHECK-NEXT: ret void
+;
+ call void @bar_without_noundef(i8* %a, i8* %b)
+ call void @baz(i8* %b, i8* %b)
+ ret void
+}
+
diff --git a/llvm/test/Transforms/InstCombine/unused-nonnull.ll b/llvm/test/Transforms/InstCombine/unused-nonnull.ll
index 382d2634b86c..74173ad4592d 100644
--- a/llvm/test/Transforms/InstCombine/unused-nonnull.ll
+++ b/llvm/test/Transforms/InstCombine/unused-nonnull.ll
@@ -35,9 +35,9 @@ done:
ret i32 %retval
}
-define i32 @compute(i8* nonnull %ptr, i32 %x) #1 {
+define i32 @compute(i8* noundef nonnull %ptr, i32 %x) #1 {
; CHECK-LABEL: define {{[^@]+}}@compute
-; CHECK-SAME: (i8* nocapture nonnull readnone [[PTR:%.*]], i32 returned [[X:%.*]]) local_unnamed_addr #1
+; CHECK-SAME: (i8* nocapture noundef nonnull readnone [[PTR:%.*]], i32 returned [[X:%.*]]) local_unnamed_addr #1
; CHECK-NEXT: ret i32 [[X]]
;
ret i32 %x
More information about the llvm-commits
mailing list