[llvm] 99f869c - [Attributes] Remove nonnull from UB-implying attributes

[Nikita Popov via llvm-commits]

Author: Nikita Popov
Date: 2021-07-25T18:07:31+02:00
New Revision: 99f869c8f00a36dac3c774178b69d05876a29a31

URL: https://github.com/llvm/llvm-project/commit/99f869c8f00a36dac3c774178b69d05876a29a31
DIFF: https://github.com/llvm/llvm-project/commit/99f869c8f00a36dac3c774178b69d05876a29a31.diff

LOG: [Attributes] Remove nonnull from UB-implying attributes

>From LangRef:

> if the parameter or return pointer is null, poison value 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.

Dropping noundef is sufficient to prevent UB. Including nonnull
in this method just muddies the semantics.

Added: 
    

Modified: 
    llvm/lib/IR/Attributes.cpp
    llvm/test/Transforms/DeadArgElim/NoundefAttrs.ll
    llvm/test/Transforms/InstCombine/unused-nonnull.ll

Removed: 
    


################################################################################
diff  --git a/llvm/lib/IR/Attributes.cpp b/llvm/lib/IR/Attributes.cpp
index fc05f52ee7415..92721c743179e 100644
--- a/llvm/lib/IR/Attributes.cpp
+++ b/llvm/lib/IR/Attributes.cpp
@@ -1340,7 +1340,6 @@ AttributeList::removeParamUndefImplyingAttributes(LLVMContext &C,
                                                   unsigned ArgNo) const {
   AttrBuilder B;
   B.addAttribute(Attribute::NoUndef);
-  B.addAttribute(Attribute::NonNull);
   B.addDereferenceableAttr(1);
   B.addDereferenceableOrNullAttr(1);
   return removeParamAttributes(C, ArgNo, B);

diff  --git a/llvm/test/Transforms/DeadArgElim/NoundefAttrs.ll b/llvm/test/Transforms/DeadArgElim/NoundefAttrs.ll
index 576f972fc87a8..155974cbf1adf 100644
--- a/llvm/test/Transforms/DeadArgElim/NoundefAttrs.ll
+++ b/llvm/test/Transforms/DeadArgElim/NoundefAttrs.ll
@@ -1,10 +1,10 @@
 ; RUN: opt -deadargelim -S < %s | FileCheck %s
 
-; If caller is changed to pass in undef, noundef and related attributes
-; should be deleted.
+; If caller is changed to pass in undef, noundef, dereferenceable and other
+; attributes that imply immediate undefined behavior should be delete.
+; Other attributes like nonnull, which only imply poison, can be safely kept.
 
-
-; CHECK:   define i64 @bar(i64* %0, i64 %1)
+; CHECK:   define i64 @bar(i64* nonnull %0, i64 %1)
 define i64 @bar(i64* nonnull dereferenceable(8) %0, i64 %1) {
 entry:
   %2 = add i64 %1, 8
@@ -12,7 +12,7 @@ entry:
 }
 
 define i64 @foo(i64* %p, i64 %v) {
-; CHECK:   %retval = call i64 @bar(i64* undef, i64 %v)
+; CHECK:   %retval = call i64 @bar(i64* nonnull undef, i64 %v)
   %retval = call i64 @bar(i64* nonnull dereferenceable(8) %p, i64 %v)
   ret i64 %retval
 }

diff  --git a/llvm/test/Transforms/InstCombine/unused-nonnull.ll b/llvm/test/Transforms/InstCombine/unused-nonnull.ll
index 0f6ef72999803..f29485218abe5 100644
--- a/llvm/test/Transforms/InstCombine/unused-nonnull.ll
+++ b/llvm/test/Transforms/InstCombine/unused-nonnull.ll
@@ -9,7 +9,7 @@ target triple = "x86_64-unknown-linux-gnu"
 
 define i32 @main(i32 %argc, i8** %argv) #0 {
 ; CHECK-LABEL: define {{[^@]+}}@main
-; CHECK-SAME: (i32 [[ARGC:%.*]], i8** nocapture readnone [[ARGV:%.*]]) local_unnamed_addr #0
+; CHECK-SAME: (i32 [[ARGC:%.*]], i8** nocapture readnone [[ARGV:%.*]]) local_unnamed_addr #[[ATTR0:[0-9]+]] {
 ; CHECK-NEXT:  entry:
 ; CHECK-NEXT:    [[TMP0:%.*]] = icmp slt i32 [[ARGC]], 2
 ; CHECK-NEXT:    [[SPEC_SELECT:%.*]] = select i1 [[TMP0]], i32 0, i32 [[ARGC]]
@@ -37,7 +37,7 @@ done:
 
 define i32 @compute(i8* noundef nonnull %ptr, i32 %x) #1 {
 ; CHECK-LABEL: define {{[^@]+}}@compute
-; CHECK-SAME: (i8* nocapture readnone [[PTR:%.*]], i32 returned [[X:%.*]]) local_unnamed_addr #1
+; CHECK-SAME: (i8* nocapture nonnull readnone [[PTR:%.*]], i32 returned [[X:%.*]]) local_unnamed_addr #[[ATTR1:[0-9]+]] {
 ; CHECK-NEXT:    ret i32 [[X]]
 ;
   ret i32 %x