From: Philip Reames <listmail@philipreames.com>
Date: Tue, 12 Mar 2019 20:15:05 +0000 (+0000)
Subject: [SROA] Fix a crash when trying to convert a memset to an non-integral pointer type
X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=1ca9798456a7214cc2ae574470ce814e1c697632;p=llvm

[SROA] Fix a crash when trying to convert a memset to an non-integral pointer type

The included test case currently crashes on tip of tree. Rather than adding a bailout, I chose to restructure the code so that the existing helper function could be used. Given that, the majority of the diff is NFC-ish, but the key difference is that canConvertValue returns false when only one side is a non-integral pointer.

Thanks to Cherry Zhang for the test case.

Differential Revision: https://reviews.llvm.org/D59000




git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@355962 91177308-0d34-0410-b5e6-96231b3b80d8
---

diff --git a/lib/Transforms/Scalar/SROA.cpp b/lib/Transforms/Scalar/SROA.cpp
index 7a63bf58b47..9786614db0a 100644
--- a/lib/Transforms/Scalar/SROA.cpp
+++ b/lib/Transforms/Scalar/SROA.cpp
@@ -2727,15 +2727,26 @@ private:
 
     Type *AllocaTy = NewAI.getAllocatedType();
     Type *ScalarTy = AllocaTy->getScalarType();
+    
+    const bool CanContinue = [&]() {
+      if (VecTy || IntTy)
+        return true;
+      if (BeginOffset > NewAllocaBeginOffset ||
+          EndOffset < NewAllocaEndOffset)
+        return false;
+      auto *C = cast<ConstantInt>(II.getLength());
+      if (C->getBitWidth() > 64)
+        return false;
+      const auto Len = C->getZExtValue();
+      auto *Int8Ty = IntegerType::getInt8Ty(NewAI.getContext());
+      auto *SrcTy = VectorType::get(Int8Ty, Len);
+      return canConvertValue(DL, SrcTy, AllocaTy) &&
+        DL.isLegalInteger(DL.getTypeSizeInBits(ScalarTy));
+    }();
 
     // If this doesn't map cleanly onto the alloca type, and that type isn't
     // a single value type, just emit a memset.
-    if (!VecTy && !IntTy &&
-        (BeginOffset > NewAllocaBeginOffset || EndOffset < NewAllocaEndOffset ||
-         SliceSize != DL.getTypeStoreSize(AllocaTy) ||
-         !AllocaTy->isSingleValueType() ||
-         !DL.isLegalInteger(DL.getTypeSizeInBits(ScalarTy)) ||
-         DL.getTypeSizeInBits(ScalarTy) % 8 != 0)) {
+    if (!CanContinue) {
       Type *SizeTy = II.getLength()->getType();
       Constant *Size = ConstantInt::get(SizeTy, NewEndOffset - NewBeginOffset);
       CallInst *New = IRB.CreateMemSet(
diff --git a/test/Transforms/SROA/non-integral-pointers.ll b/test/Transforms/SROA/non-integral-pointers.ll
index 63286309f6f..166f5dc7b42 100644
--- a/test/Transforms/SROA/non-integral-pointers.ll
+++ b/test/Transforms/SROA/non-integral-pointers.ll
@@ -44,3 +44,45 @@ neverTaken:
 alwaysTaken:
   ret i64 42
 }
+
+define i64 addrspace(4)* @memset(i1 %alwaysFalse) {
+; CHECK-LABEL: @memset(
+; CHECK-NOT: inttoptr
+; CHECK-NOT: ptrtoint
+entry:
+  %x = alloca i64 addrspace(4)*
+  %cast.0 = bitcast i64 addrspace(4)** %x to i8*
+  call void @llvm.memset.p0i8.i64(i8* align 8 %cast.0, i8 5, i64 16, i1 false)
+  br i1 %alwaysFalse, label %neverTaken, label %alwaysTaken
+
+neverTaken:
+  %x.field.ld.0 = load i64 addrspace(4)*, i64 addrspace(4)** %x
+  ret i64 addrspace(4)* %x.field.ld.0
+  
+alwaysTaken:
+  ret i64 addrspace(4)* null
+}
+
+;; TODO: This one demonstrates a missed oppurtunity.  The only known bit
+;; pattern for a non-integral bit pattern is that null is zero.  As such
+;; we could do SROA and replace the memset w/a null store.  This will
+;; usually be gotten by instcombine.
+define i64 addrspace(4)* @memset_null(i1 %alwaysFalse) {
+; CHECK-LABEL: @memset_null(
+; CHECK-NOT: inttoptr
+; CHECK-NOT: ptrtoint
+entry:
+  %x = alloca i64 addrspace(4)*
+  %cast.0 = bitcast i64 addrspace(4)** %x to i8*
+  call void @llvm.memset.p0i8.i64(i8* align 8 %cast.0, i8 0, i64 16, i1 false)
+  br i1 %alwaysFalse, label %neverTaken, label %alwaysTaken
+
+neverTaken:
+  %x.field.ld.0 = load i64 addrspace(4)*, i64 addrspace(4)** %x
+  ret i64 addrspace(4)* %x.field.ld.0
+  
+alwaysTaken:
+  ret i64 addrspace(4)* null
+}
+
+declare void @llvm.memset.p0i8.i64(i8*, i8, i64, i1)