[llvm-branch-commits] [llvm] 4479c0c - Allow nonnull/align attribute to accept poison

Juneyoung Lee via llvm-branch-commits llvm-branch-commits at lists.llvm.org
Tue Jan 19 18:59:17 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-branch-commits mailing list