From bb0344644a656734d707ab9c0baf6eb0533ac905 Mon Sep 17 00:00:00 2001
From: Philip Reames <listmail@philipreames.com>
Date: Thu, 1 Oct 2020 16:44:12 -0700
Subject: [PATCH] [memcpyopt] Conservatively handle non-integral pointers

If we allow the non-integral pointers to become memset and memcpy, we loose the ability to reason about pointer propagation.  This patch is modeled on changes we've carried downstream for a long time, figured it was worth being equally conservative for other users.  There is room to refine the semantics and handling here if anyone is motivated.
---
 .../lib/Transforms/Scalar/MemCpyOptimizer.cpp | 18 ++++++++--
 .../test/Transforms/MemCpyOpt/non-integral.ll | 36 +++++++++++++++++++
 2 files changed, 52 insertions(+), 2 deletions(-)
 create mode 100644 llvm/test/Transforms/MemCpyOpt/non-integral.ll

diff --git a/llvm/lib/Transforms/Scalar/MemCpyOptimizer.cpp b/llvm/lib/Transforms/Scalar/MemCpyOptimizer.cpp
index 0d66092a7035..01f3c322b1f4 100644
--- a/llvm/lib/Transforms/Scalar/MemCpyOptimizer.cpp
+++ b/llvm/lib/Transforms/Scalar/MemCpyOptimizer.cpp
@@ -352,8 +352,15 @@ Instruction *MemCpyOptPass::tryMergingIntoMemset(Instruction *StartInst,
       // If this is a store, see if we can merge it in.
       if (!NextStore->isSimple()) break;
 
+      Value *StoredVal = NextStore->getValueOperand();
+
+      // Don't convert stores of non-integral pointer types to memsets (which
+      // stores integers).
+      if (DL.isNonIntegralPointerType(StoredVal->getType()->getScalarType()))
+        break;
+
       // Check to see if this stored value is of the same byte-splattable value.
-      Value *StoredByte = isBytewiseValue(NextStore->getOperand(0), DL);
+      Value *StoredByte = isBytewiseValue(StoredVal, DL);
       if (isa<UndefValue>(ByteVal) && StoredByte)
         ByteVal = StoredByte;
       if (ByteVal != StoredByte)
@@ -556,8 +563,15 @@ bool MemCpyOptPass::processStore(StoreInst *SI, BasicBlock::iterator &BBI) {
 
   const DataLayout &DL = SI->getModule()->getDataLayout();
 
+  Value *StoredVal = SI->getValueOperand();
+
+  // Not all the transforms below are correct for non-integral pointers, bail
+  // until we've audited the individual pieces.
+  if (DL.isNonIntegralPointerType(StoredVal->getType()->getScalarType()))
+    return false;
+
   // Load to store forwarding can be interpreted as memcpy.
-  if (LoadInst *LI = dyn_cast<LoadInst>(SI->getOperand(0))) {
+  if (LoadInst *LI = dyn_cast<LoadInst>(StoredVal)) {
     if (LI->isSimple() && LI->hasOneUse() &&
         LI->getParent() == SI->getParent()) {
 
diff --git a/llvm/test/Transforms/MemCpyOpt/non-integral.ll b/llvm/test/Transforms/MemCpyOpt/non-integral.ll
new file mode 100644
index 000000000000..eecbea32adb5
--- /dev/null
+++ b/llvm/test/Transforms/MemCpyOpt/non-integral.ll
@@ -0,0 +1,36 @@
+; NOTE: Assertions have been autogenerated by utils/update_test_checks.py
+; RUN: opt -memcpyopt -S < %s | FileCheck %s
+
+target datalayout = "e-m:o-i64:64-f80:128-n8:16:32:64-S128-ni:1"
+
+define void @illegal_memset(i64 addrspace(1)** %p) {
+; CHECK-LABEL: @illegal_memset(
+; CHECK-NEXT:  entry:
+; CHECK-NEXT:    [[P1:%.*]] = bitcast i64 addrspace(1)** [[P:%.*]] to i8*
+; CHECK-NEXT:    call void @llvm.memset.p0i8.i64(i8* [[P1]], i8 0, i64 8, i1 false)
+; CHECK-NEXT:    [[GEP:%.*]] = getelementptr i64 addrspace(1)*, i64 addrspace(1)** [[P]], i64 1
+; CHECK-NEXT:    store i64 addrspace(1)* null, i64 addrspace(1)** [[GEP]], align 8
+; CHECK-NEXT:    ret void
+;
+entry:
+  %p1 = bitcast i64 addrspace(1)** %p to i8*
+  call void @llvm.memset.p0i8.i64(i8* %p1, i8 0, i64 8, i32 0, i1 false)
+  %gep = getelementptr i64 addrspace(1)*, i64 addrspace(1)** %p, i64 1
+  store i64 addrspace(1)* null, i64 addrspace(1)** %gep
+  ret void
+}
+
+define void @illegal_memcpy(<2 x i8 addrspace(1)*>* noalias align 16 %a,
+; CHECK-LABEL: @illegal_memcpy(
+; CHECK-NEXT:    [[VAL:%.*]] = load <2 x i8 addrspace(1)*>, <2 x i8 addrspace(1)*>* [[A:%.*]], align 16
+; CHECK-NEXT:    store <2 x i8 addrspace(1)*> [[VAL]], <2 x i8 addrspace(1)*>* [[B:%.*]], align 16
+; CHECK-NEXT:    ret void
+;
+  <2 x i8 addrspace(1)*>* noalias align 16 %b) {
+  %val = load <2 x i8 addrspace(1)*>, <2 x i8 addrspace(1)*>* %a, align 16
+  store <2 x i8 addrspace(1)*> %val, <2 x i8 addrspace(1)*>* %b, align 16
+  ret void
+}
+
+declare void @llvm.memset.p1i8.i64(i8 addrspace(1)*, i8, i64, i32, i1)
+declare void @llvm.memset.p0i8.i64(i8*, i8, i64, i32, i1)