[instcombine] Delete duplicate object size logic

[reames]

Summary

InstCombine appears to duplicate the allocation size logic used inside getObjectSize when figuring out which attributes are safe to place on the callsite. We can use the existing utility function instead.

The test change is correct. With aligned_alloc, a zero alignment is required to return nullptr. As such, deref_or_null is a correct attribute to use.

Posting for review as the code implementing getObjectSize is a tad subtle, and I want to get a second set of eyes to make sure I'm not missing a more material difference.

[xbolva00]

llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp

2578

all allocation fns have nonnull attribute, I believe.

[reames]

llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp

2578

Er, definitely not. Malloc can return nullptr on error. Did you maybe mean that all operator new versions did? I couldn't find that code, can you point me to it?

[xbolva00]

llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp

2578

Yeah right, just for new calls.

Yes, todo is correct here for others

[reames]

llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp

2578

Your wording in last comment is unclear to me, can you rephrase?

[xbolva00]

llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp

2578

That for throwing version of 'new' we already have nonnull.

I am wondering if just “deref(N)” attribute alone is enough for other parts of LLVM to assume also “nonnull” (for addr space 0)

[reames]

llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp

2578

Can you point me to the code which infers nonnull for the throwing operator new variants? I did not find it on a quick skim. In particular, I do not see this in BuildLibCalls.cpp which I would expect.

(Regardless, I'd like to leave this cleanup to a separate patch.)

[xbolva00]

No tests fail, so this looks good to go.

[xbolva00]

llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp

2578

https://clang.llvm.org/doxygen/SemaDecl_8cpp_source.html#l15072

[reames]

llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp

2578

Ah, I was looking in LLVM proper. Thanks!

Will cleanup the tests and remove this in a separate commit.

[nikic]

LGTM

llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp

static IntrinsicInst *findInitTrampoline(Value *Callee) {
   if (IntrinsicInst *IT = findInitTrampolineFromAlloca(TrampMem))
     return IT;
   if (IntrinsicInst *IT = findInitTrampolineFromBB(AdjustTramp, TrampMem))
     return IT;
   return nullptr;
 }
 
 void InstCombinerImpl::annotateAnyAllocSite(CallBase &Call, const TargetLibraryInfo *TLI) {
-  unsigned NumArgs = Call.arg_size();
-  ConstantInt *Op0C = dyn_cast<ConstantInt>(Call.getOperand(0));
-  ConstantInt *Op1C =
-      (NumArgs == 1) ? nullptr : dyn_cast<ConstantInt>(Call.getOperand(1));
-  // Bail out if the allocation size is zero (or an invalid alignment of zero
-  // with aligned_alloc).
-  if ((Op0C && Op0C->isNullValue()) || (Op1C && Op1C->isNullValue()))
-    return;
 
-  if (isMallocLikeFn(&Call, TLI) && Op0C) {
+  uint64_t Size;
+  ObjectSizeOpts Opts;
+  if (getObjectSize(&Call, Size, DL, TLI, Opts) && Size > 0) {
+    // TODO: should be annotating these nonnull
     if (isOpNewLikeFn(&Call, TLI))
       Call.addRetAttr(Attribute::getWithDereferenceableBytes(
-          Call.getContext(), Op0C->getZExtValue()));
+          Call.getContext(), Size));
     else
       Call.addRetAttr(Attribute::getWithDereferenceableOrNullBytes(
-          Call.getContext(), Op0C->getZExtValue()));
-  } else if (isAlignedAllocLikeFn(&Call, TLI)) {
-    if (Op1C)
-      Call.addRetAttr(Attribute::getWithDereferenceableOrNullBytes(
-          Call.getContext(), Op1C->getZExtValue()));
-    // Add alignment attribute if alignment is a power of two constant.
-    if (Op0C && Op0C->getValue().ult(llvm::Value::MaximumAlignment) &&
-        isKnownNonZero(Call.getOperand(1), DL, 0, &AC, &Call, &DT)) {
-      uint64_t AlignmentVal = Op0C->getZExtValue();
-      if (llvm::isPowerOf2_64(AlignmentVal)) {
-        Call.removeRetAttr(Attribute::Alignment);
-        Call.addRetAttr(Attribute::getWithAlignment(Call.getContext(),
-                                                    Align(AlignmentVal)));
-      }
-    }
-  } else if (isReallocLikeFn(&Call, TLI) && Op1C) {
-    Call.addRetAttr(Attribute::getWithDereferenceableOrNullBytes(
-        Call.getContext(), Op1C->getZExtValue()));
-  } else if (isCallocLikeFn(&Call, TLI) && Op0C && Op1C) {
-    bool Overflow;
-    const APInt &N = Op0C->getValue();
-    APInt Size = N.umul_ov(Op1C->getValue(), Overflow);
-    if (!Overflow)
-      Call.addRetAttr(Attribute::getWithDereferenceableOrNullBytes(
-          Call.getContext(), Size.getZExtValue()));
-  } else if (isStrdupLikeFn(&Call, TLI)) {
-    uint64_t Len = GetStringLength(Call.getOperand(0));
-    if (Len) {
-      // strdup
-      if (NumArgs == 1)
-        Call.addRetAttr(Attribute::getWithDereferenceableOrNullBytes(
-            Call.getContext(), Len));
-      // strndup
-      else if (NumArgs == 2 && Op1C)
-        Call.addRetAttr(Attribute::getWithDereferenceableOrNullBytes(
-            Call.getContext(), std::min(Len, Op1C->getZExtValue() + 1)));
-    }
-  }
+          Call.getContext(), Size));
+  }
+
+  if (!isAlignedAllocLikeFn(&Call, TLI))
+    return;
+
+  ConstantInt *Op0C = dyn_cast<ConstantInt>(Call.getOperand(0));
 }
 
 /// Improvements for call, callbr and invoke instructions.
 Instruction *InstCombinerImpl::visitCallBase(CallBase &Call) {
   if (isAllocationFn(&Call, &TLI))
     annotateAnyAllocSite(Call, &TLI);
 
   bool Changed = false;

llvm/test/Transforms/InstCombine/deref-alloc-fns.ll

;
   ret i8* %call
 }
 
 declare noalias i8* @foo(i8*, i8*, i8*)
 
 define noalias i8* @aligned_alloc_dynamic_args(i64 %align, i64 %size) {
 ; CHECK-LABEL: @aligned_alloc_dynamic_args(
 ; CHECK-NEXT:    [[CALL:%.*]] = tail call noalias dereferenceable_or_null(1024) i8* @aligned_alloc(i64 [[ALIGN:%.*]], i64 1024)
-; CHECK-NEXT:    [[CALL_1:%.*]] = tail call noalias i8* @aligned_alloc(i64 0, i64 1024)
+; CHECK-NEXT:    [[CALL_1:%.*]] = tail call noalias dereferenceable_or_null(1024) i8* @aligned_alloc(i64 0, i64 1024)
 ; CHECK-NEXT:    [[CALL_2:%.*]] = tail call noalias i8* @aligned_alloc(i64 32, i64 [[SIZE:%.*]])
 ; CHECK-NEXT:    [[TMP1:%.*]] = call i8* @foo(i8* [[CALL]], i8* [[CALL_1]], i8* [[CALL_2]])
 ; CHECK-NEXT:    ret i8* [[CALL]]
 ;
   %call = tail call noalias i8* @aligned_alloc(i64 %align, i64 1024)
   %call_1 = tail call noalias i8* @aligned_alloc(i64 0, i64 1024)
   %call_2 = tail call noalias i8* @aligned_alloc(i64 32, i64 %size)