Reapply [InstCombine] Don't check for alloc fn before fetching alloc size
authorNikita Popov <npopov@redhat.com>
Thu, 21 Jul 2022 08:05:48 +0000 (10:05 +0200)
committerNikita Popov <npopov@redhat.com>
Thu, 21 Jul 2022 09:48:24 +0000 (11:48 +0200)
Reapply the patch with getObjectSize() replaced by getAllocSize().
The former will also look through calls that return their argument,
and we'll end up placing dereferenceable attributes on intrinsics
like llvm.launder.invariant.group. While this isn't wrong, it also
doesn't seem to be particularly useful. For now, use getAllocSize()
instead, which sticks closer to the original behavior of this code.

-----

This code is just interested in the allocsize, not any other
allocator properties.

llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp
llvm/test/Transforms/InstCombine/deref-alloc-fns.ll

index 6a2cfd6..8ac9be5 100644 (file)
@@ -2885,21 +2885,21 @@ bool InstCombinerImpl::annotateAnyAllocSite(CallBase &Call,
   // of the respective allocator declaration with generic attributes.
   bool Changed = false;
 
-  if (isAllocationFn(&Call, TLI)) {
-    uint64_t Size;
-    ObjectSizeOpts Opts;
-    if (getObjectSize(&Call, Size, DL, TLI, Opts) && Size > 0) {
-      // 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)) {
-        Changed = !Call.hasRetAttr(Attribute::Dereferenceable);
-        Call.addRetAttr(
-            Attribute::getWithDereferenceableBytes(Call.getContext(), Size));
-      } else {
-        Changed = !Call.hasRetAttr(Attribute::DereferenceableOrNull);
-        Call.addRetAttr(Attribute::getWithDereferenceableOrNullBytes(
-            Call.getContext(), Size));
-      }
+  if (!Call.getType()->isPointerTy())
+    return Changed;
+
+  Optional<APInt> Size = getAllocSize(&Call, TLI);
+  if (Size && *Size != 0) {
+    // 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)) {
+      Changed = !Call.hasRetAttr(Attribute::Dereferenceable);
+      Call.addRetAttr(Attribute::getWithDereferenceableBytes(
+          Call.getContext(), Size->getLimitedValue()));
+    } else {
+      Changed = !Call.hasRetAttr(Attribute::DereferenceableOrNull);
+      Call.addRetAttr(Attribute::getWithDereferenceableOrNullBytes(
+          Call.getContext(), Size->getLimitedValue()));
     }
   }
 
index 0b52935..41a2427 100644 (file)
@@ -13,6 +13,8 @@ declare noalias i8* @aligned_alloc(i64, i64)
 declare noalias align 16 i8* @memalign(i64, i64)
 ; new[](unsigned int, align_val_t)
 declare noalias i8* @_ZnajSt11align_val_t(i64 %size, i64 %align)
+declare i8* @my_malloc(i64) allocsize(0)
+declare i8* @my_calloc(i64, i64) allocsize(0, 1)
 
 @.str = private unnamed_addr constant [6 x i8] c"hello\00", align 1
 
@@ -354,3 +356,20 @@ define noalias i8* @op_new_align() {
   ret i8* %call
 }
 
+define i8* @my_malloc_constant_size() {
+; CHECK-LABEL: @my_malloc_constant_size(
+; CHECK-NEXT:    [[CALL:%.*]] = call dereferenceable_or_null(32) i8* @my_malloc(i64 32)
+; CHECK-NEXT:    ret i8* [[CALL]]
+;
+  %call = call i8* @my_malloc(i64 32)
+  ret i8* %call
+}
+
+define i8* @my_calloc_constant_size() {
+; CHECK-LABEL: @my_calloc_constant_size(
+; CHECK-NEXT:    [[CALL:%.*]] = call dereferenceable_or_null(128) i8* @my_calloc(i64 32, i64 4)
+; CHECK-NEXT:    ret i8* [[CALL]]
+;
+  %call = call i8* @my_calloc(i64 32, i64 4)
+  ret i8* %call
+}