[InstCombine] avoid crashing on attribute propagation

[spatel]

Summary

In https://llvm.org/PR48810 , we are crashing while trying to propagate attributes from mempcpy (returns void*) to memcpy (returns nothing - void).
We can avoid the crash by removing known incompatible attributes for the void return type.
I'm not sure if this goes far enough (should we just drop all attributes since this isn't the same function?). We also need to audit other transforms in LibCallSimplifier to make sure there are no other cases that have the same problem.

[jdoerfert]

LGTM, one comment below though.

llvm/lib/Transforms/Utils/SimplifyLibCalls.cpp

1158

We could attach (some, if not all) return value attributes to the first operand in order to preserve them. I guess a FIXME would do for now if you don't feel like including this.

[xbolva00]

LG

[spatel]

llvm/lib/Transforms/Utils/SimplifyLibCalls.cpp

1158

I haven't looked at what is currently possible with attribute transforms other than this, so I'll add a comment. Not sure if/when I can extend the patch.

[jdoerfert]

llvm/lib/Transforms/Utils/SimplifyLibCalls.cpp

1158

Comment is fine. This bug needs fixing ;)

llvm/lib/Transforms/Utils/SimplifyLibCalls.cpp

 }
 
 Value *LibCallSimplifier::optimizeMemPCpy(CallInst *CI, IRBuilderBase &B) {
   Value *Dst = CI->getArgOperand(0);
   Value *N = CI->getArgOperand(2);
   // mempcpy(x, y, n) -> llvm.memcpy(align 1 x, align 1 y, n), x + n
   CallInst *NewCI =
       B.CreateMemCpy(Dst, Align(1), CI->getArgOperand(1), Align(1), N);
+  // Propagate attributes, but memcpy has no return value, so make sure that
+  // any return attributes are compliant.
+  // TODO: Attach return value attributes to the 1st operand to preserve them?
   NewCI->setAttributes(CI->getAttributes());
+  NewCI->removeAttributes(AttributeList::ReturnIndex,
+                          AttributeFuncs::typeIncompatible(NewCI->getType()));
   return B.CreateInBoundsGEP(B.getInt8Ty(), Dst, N);
 }
 
 Value *LibCallSimplifier::optimizeMemMove(CallInst *CI, IRBuilderBase &B) {
   Value *Size = CI->getArgOperand(2);
   annotateNonNullAndDereferenceable(CI, {0, 1}, Size, DL);
   if (isa<IntrinsicInst>(CI))
     return nullptr;

llvm/test/Transforms/InstCombine/mempcpy.ll

 ; CHECK-NEXT:    call void @llvm.memcpy.p0i8.p0i8.i64(i8* nonnull align 1 dereferenceable(1024) [[D:%.*]], i8* nonnull align 1 dereferenceable(1024) [[S:%.*]], i64 1024, i1 false)
 ; CHECK-NEXT:    [[TMP1:%.*]] = getelementptr inbounds i8, i8* [[D]], i64 1024
 ; CHECK-NEXT:    ret i8* [[TMP1]]
 ;
   %r = tail call i8* @mempcpy(i8* %d, i8* %s, i64 1024)
   ret i8* %r
 }
 
+; The original call may have attributes that can not propagate to memcpy.
+
+define i32 @PR48810() {
+; CHECK-LABEL: @PR48810(
+; CHECK-NEXT:    call void @llvm.memcpy.p0i8.p0i8.i64(i8* align 1 undef, i8* align 536870912 null, i64 undef, i1 false)
+; CHECK-NEXT:    ret i32 undef
+;
+  %r = call dereferenceable(1) i8* @mempcpy(i8* undef, i8* null, i64 undef)
+  ret i32 undef
+}
+
 declare i8* @mempcpy(i8*, i8* nocapture readonly, i64)