[llvm] 2cafbcb - [instcombine] Key deref vs deref_or_null annotation of allocation sites off nonnull attribute

[Philip Reames via llvm-commits]

Author: Philip Reames
Date: 2022-01-08T10:33:54-08:00
New Revision: 2cafbcb560d9e6e2300941d088e754b01d56595b

URL: https://github.com/llvm/llvm-project/commit/2cafbcb560d9e6e2300941d088e754b01d56595b
DIFF: https://github.com/llvm/llvm-project/commit/2cafbcb560d9e6e2300941d088e754b01d56595b.diff

LOG: [instcombine] Key deref vs deref_or_null annotation of allocation sites off nonnull attribute

Goal is to remove use of isOpNewLike.  I looked at a couple approaches to this, and this turned out to be the cheapest one.  Just letting deref_or_null be generated causes a bunch of test diffs, and I couldn't convince myself there wasn't a real regression somewhere.  A generic instcombine to convert deref_or_null + nonnull to deref is annoying complicated since you have to mix facts from callsite and declaration while manipulating only existing call site attributes.  It just wasn't worth the code complexity.

Note that the change in new-delete-itanium.ll is a real regression.  If you have a callsite which overrides the builtin status of a nobuiltin declaration, *and* you don't put the apppriate attributes on that callsite, you may lose the deref fact.  I decided this didn't matter; if anyone disagrees, you can add this case to the generic non-null inference.

Added: 
    

Modified: 
    llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp
    llvm/test/Transforms/InstCombine/new-delete-itanium.ll
    llvm/test/Transforms/InstCombine/objsize-64.ll

Removed: 
    


################################################################################
diff  --git a/llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp b/llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp
index afd1ef2753cc3..33450b7eb1372 100644
--- a/llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp
+++ b/llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp
@@ -2571,12 +2571,17 @@ static IntrinsicInst *findInitTrampoline(Value *Callee) {
 }
 
 void InstCombinerImpl::annotateAnyAllocSite(CallBase &Call, const TargetLibraryInfo *TLI) {
+  // Note: We only handle cases which can't be driven from generic attributes
+  // here.  So, for example, nonnull and noalias (which are common properties
+  // of some allocation functions) are expected to be handled via annotation
+  // of the respective allocator declaration with generic attributes.
 
   uint64_t Size;
   ObjectSizeOpts Opts;
   if (getObjectSize(&Call, Size, DL, TLI, Opts) && Size > 0) {
-    // TODO: should be annotating these nonnull
-    if (isOpNewLikeFn(&Call, TLI))
+    // TODO: We really should just emit deref_or_null here and then
+    // let the generic inference code combine that with nonnull.
+    if (Call.hasRetAttr(Attribute::NonNull))
       Call.addRetAttr(Attribute::getWithDereferenceableBytes(
           Call.getContext(), Size));
     else

diff  --git a/llvm/test/Transforms/InstCombine/new-delete-itanium.ll b/llvm/test/Transforms/InstCombine/new-delete-itanium.ll
index d2044fa05c020..4f0470c8a159a 100644
--- a/llvm/test/Transforms/InstCombine/new-delete-itanium.ll
+++ b/llvm/test/Transforms/InstCombine/new-delete-itanium.ll
@@ -201,8 +201,8 @@ define void @test10()  {
 
 define void @test11() {
 ; CHECK-LABEL: @test11(
-; CHECK-NEXT:    [[CALL:%.*]] = call dereferenceable(8) i8* @_Znwm(i64 8) #[[ATTR5]]
-; CHECK-NEXT:    call void @_ZdlPv(i8* nonnull [[CALL]])
+; CHECK-NEXT:    [[CALL:%.*]] = call dereferenceable_or_null(8) i8* @_Znwm(i64 8) #[[ATTR5]]
+; CHECK-NEXT:    call void @_ZdlPv(i8* [[CALL]])
 ; CHECK-NEXT:    ret void
 ;
   %call = call i8* @_Znwm(i64 8) builtin

diff  --git a/llvm/test/Transforms/InstCombine/objsize-64.ll b/llvm/test/Transforms/InstCombine/objsize-64.ll
index 06ad232fce09c..4a8522f90b5c5 100644
--- a/llvm/test/Transforms/InstCombine/objsize-64.ll
+++ b/llvm/test/Transforms/InstCombine/objsize-64.ll
@@ -3,7 +3,7 @@
 target datalayout = "e-p:64:64:64-i1:8:8-i8:8:8-i16:16:16-i32:32:32-i64:64:64-f32:32:32-f64:64:64-v64:64:64-v128:128:128-a0:0:64-s0:64:64-f80:128:128-n8:16:32:64-S128"
 
 declare noalias i8* @malloc(i32) nounwind
-declare noalias i8* @_Znwm(i64)  ; new(unsigned long)
+declare noalias nonnull i8* @_Znwm(i64)  ; new(unsigned long)
 declare i32 @__gxx_personality_v0(...)
 declare void @__cxa_call_unexpected(i8*)
 declare i64 @llvm.objectsize.i64(i8*, i1) nounwind readonly
@@ -33,7 +33,7 @@ define i64 @f2(i8** %esc) nounwind uwtable ssp personality i8* bitcast (i32 (...
 ; CHECK-NEXT:    [[TMP0:%.*]] = landingpad { i8*, i32 }
 ; CHECK-NEXT:    filter [0 x i8*] zeroinitializer
 ; CHECK-NEXT:    [[TMP1:%.*]] = extractvalue { i8*, i32 } [[TMP0]], 0
-; CHECK-NEXT:    tail call void @__cxa_call_unexpected(i8* [[TMP1]]) #3
+; CHECK-NEXT:    tail call void @__cxa_call_unexpected(i8* [[TMP1]]) #[[ATTR3:[0-9]+]]
 ; CHECK-NEXT:    unreachable
 ;
 entry: