[llvm] 8e0ff44 - [InstCombine] Make varargs cast transform compatible with opaque ptrs

[Nikita Popov via llvm-commits]

Author: Nikita Popov
Date: 2021-06-24T21:57:05+02:00
New Revision: 8e0ff44bf896733f04c5cc9e68aee2b777860ddd

URL: https://github.com/llvm/llvm-project/commit/8e0ff44bf896733f04c5cc9e68aee2b777860ddd
DIFF: https://github.com/llvm/llvm-project/commit/8e0ff44bf896733f04c5cc9e68aee2b777860ddd.diff

LOG: [InstCombine] Make varargs cast transform compatible with opaque ptrs

The whole transform can be dropped once we have fully transitioned
to opaque pointers (as it's purpose is to remove no-op pointer
casts). For now, make sure that it handles opaque pointers correctly.

Added: 
    

Modified: 
    llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp
    llvm/test/Transforms/InstCombine/opaque-ptr.ll

Removed: 
    


################################################################################
diff  --git a/llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp b/llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp
index ece98ffe39f46..256c79dbdf1dd 100644
--- a/llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp
+++ b/llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp
@@ -2031,20 +2031,27 @@ static bool isSafeToEliminateVarargsCast(const CallBase &Call,
       isa<GCResultInst>(Call))
     return false;
 
+  // Opaque pointers are compatible with any byval types.
+  PointerType *SrcTy = cast<PointerType>(CI->getOperand(0)->getType());
+  if (SrcTy->isOpaque())
+    return true;
+
   // The size of ByVal or InAlloca arguments is derived from the type, so we
   // can't change to a type with a 
diff erent size.  If the size were
   // passed explicitly we could avoid this check.
   if (!Call.isPassPointeeByValueArgument(ix))
     return true;
 
-  Type* SrcTy =
-            cast<PointerType>(CI->getOperand(0)->getType())->getElementType();
-  Type *DstTy = Call.isByValArgument(ix)
-                    ? Call.getParamByValType(ix)
-                    : cast<PointerType>(CI->getType())->getElementType();
-  if (!SrcTy->isSized() || !DstTy->isSized())
+  // The transform currently only handles type replacement for byval, not other
+  // type-carrying attributes.
+  if (!Call.isByValArgument(ix))
+    return false;
+
+  Type *SrcElemTy = SrcTy->getElementType();
+  Type *DstElemTy = Call.getParamByValType(ix);
+  if (!SrcElemTy->isSized() || !DstElemTy->isSized())
     return false;
-  if (DL.getTypeAllocSize(SrcTy) != DL.getTypeAllocSize(DstTy))
+  if (DL.getTypeAllocSize(SrcElemTy) != DL.getTypeAllocSize(DstElemTy))
     return false;
   return true;
 }
@@ -2310,6 +2317,7 @@ Instruction *InstCombinerImpl::visitCallBase(CallBase &Call) {
   if (IntrinsicInst *II = findInitTrampoline(Callee))
     return transformCallThroughTrampoline(Call, *II);
 
+  // TODO: Drop this transform once opaque pointer transition is done.
   FunctionType *FTy = Call.getFunctionType();
   if (FTy->isVarArg()) {
     int ix = FTy->getNumParams();
@@ -2321,13 +2329,14 @@ Instruction *InstCombinerImpl::visitCallBase(CallBase &Call) {
       if (CI && isSafeToEliminateVarargsCast(Call, DL, CI, ix)) {
         replaceUse(*I, CI->getOperand(0));
 
-        // Update the byval type to match the argument type.
-        if (Call.isByValArgument(ix)) {
+        // Update the byval type to match the pointer type.
+        // Not necessary for opaque pointers.
+        PointerType *NewTy = cast<PointerType>(CI->getOperand(0)->getType());
+        if (!NewTy->isOpaque() && Call.isByValArgument(ix)) {
           Call.removeParamAttr(ix, Attribute::ByVal);
           Call.addParamAttr(
               ix, Attribute::getWithByValType(
-                      Call.getContext(),
-                      CI->getOperand(0)->getType()->getPointerElementType()));
+                      Call.getContext(), NewTy->getElementType()));
         }
         Changed = true;
       }

diff  --git a/llvm/test/Transforms/InstCombine/opaque-ptr.ll b/llvm/test/Transforms/InstCombine/opaque-ptr.ll
index 6a83587a3fca8..a996c619525a2 100644
--- a/llvm/test/Transforms/InstCombine/opaque-ptr.ll
+++ b/llvm/test/Transforms/InstCombine/opaque-ptr.ll
@@ -124,3 +124,45 @@ define void @call(ptr %a) {
   call void %a()
   ret void
 }
+
+declare void @varargs(...)
+define void @varargs_cast_typed_to_opaque_same_type(i32* %a) {
+; CHECK-LABEL: @varargs_cast_typed_to_opaque_same_type(
+; CHECK-NEXT:    call void (...) @varargs(i32* byval(i32) [[A:%.*]])
+; CHECK-NEXT:    ret void
+;
+  %b = bitcast i32* %a to ptr
+  call void (...) @varargs(ptr byval(i32) %b)
+  ret void
+}
+
+define void @varargs_cast_typed_to_opaque_
diff erent_type(i32* %a) {
+; CHECK-LABEL: @varargs_cast_typed_to_opaque_
diff erent_type(
+; CHECK-NEXT:    call void (...) @varargs(i32* byval(i32) [[A:%.*]])
+; CHECK-NEXT:    ret void
+;
+  %b = bitcast i32* %a to ptr
+  call void (...) @varargs(ptr byval(float) %b)
+  ret void
+}
+
+define void @varargs_cast_typed_to_opaque_
diff erent_size(i32* %a) {
+; CHECK-LABEL: @varargs_cast_typed_to_opaque_
diff erent_size(
+; CHECK-NEXT:    [[B:%.*]] = bitcast i32* [[A:%.*]] to ptr
+; CHECK-NEXT:    call void (...) @varargs(ptr byval(i64) [[B]])
+; CHECK-NEXT:    ret void
+;
+  %b = bitcast i32* %a to ptr
+  call void (...) @varargs(ptr byval(i64) %b)
+  ret void
+}
+
+define void @varargs_cast_opaque_to_typed(ptr %a) {
+; CHECK-LABEL: @varargs_cast_opaque_to_typed(
+; CHECK-NEXT:    call void (...) @varargs(ptr byval(i8) [[A:%.*]])
+; CHECK-NEXT:    ret void
+;
+  %b = bitcast ptr %a to i8*
+  call void (...) @varargs(i8* byval(i8) %b)
+  ret void
+}