[LSR] Check yet more intrinsic pointer operands

[dfukalov]

Summary

the patch fixes another assertion in isLegalUse()

[arsenm]

Also needs tests for memset and memmove

test/Transforms/LoopStrengthReduce/AMDGPU/preserve-addrspace-assert.ll

85

Missing the mangled part of the name

[arsenm]

test/Transforms/LoopStrengthReduce/AMDGPU/preserve-addrspace-assert.ll

85

Also could use tests to stress different address spaces for source and dest

[dfukalov]

test updated as requested

[arsenm]

LGTM

lib/Transforms/Scalar/LoopStrengthReduce.cpp

if (StoreInst *SI = dyn_cast<StoreInst>(Inst)) {
     if (CmpX->getPointerOperand() == OperandVal)
       isAddress = true;
   }
   return isAddress;
 }
 
 /// Return the type of the memory being accessed.
 static MemAccessTy getAccessType(const TargetTransformInfo &TTI,
-                                 Instruction *Inst) {
+                                 Instruction *Inst, Value *OperandVal) {
   MemAccessTy AccessTy(Inst->getType(), MemAccessTy::UnknownAddressSpace);
   if (const StoreInst *SI = dyn_cast<StoreInst>(Inst)) {
     AccessTy.MemTy = SI->getOperand(0)->getType();
     AccessTy.AddrSpace = SI->getPointerAddressSpace();
   } else if (const LoadInst *LI = dyn_cast<LoadInst>(Inst)) {
     AccessTy.AddrSpace = LI->getPointerAddressSpace();
   } else if (const AtomicRMWInst *RMW = dyn_cast<AtomicRMWInst>(Inst)) {
     AccessTy.AddrSpace = RMW->getPointerAddressSpace();
   } else if (const AtomicCmpXchgInst *CmpX = dyn_cast<AtomicCmpXchgInst>(Inst)) {
     AccessTy.AddrSpace = CmpX->getPointerAddressSpace();
   } else if (IntrinsicInst *II = dyn_cast<IntrinsicInst>(Inst)) {
     switch (II->getIntrinsicID()) {
     case Intrinsic::prefetch:
+    case Intrinsic::memset:
       AccessTy.AddrSpace = II->getArgOperand(0)->getType()->getPointerAddressSpace();
+      AccessTy.MemTy = OperandVal->getType();
+      break;
+    case Intrinsic::memmove:
+    case Intrinsic::memcpy:
+      AccessTy.AddrSpace = OperandVal->getType()->getPointerAddressSpace();
+      AccessTy.MemTy = OperandVal->getType();
       break;
     default: {
       MemIntrinsicInfo IntrInfo;
       if (TTI.getTgtMemIntrinsic(II, IntrInfo) && IntrInfo.PtrVal) {
         AccessTy.AddrSpace
           = IntrInfo.PtrVal->getType()->getPointerAddressSpace();
       }
 
if (LatchBlock != ExitingBlock)
             // Stride of one or negative one can have reuse with non-addresses.
             if (C->isOne() || C->isMinusOne())
               goto decline_post_inc;
             // Avoid weird situations.
             if (C->getValue().getMinSignedBits() >= 64 ||
                 C->getValue().isMinSignedValue())
               goto decline_post_inc;
             // Check for possible scaled-address reuse.
-            MemAccessTy AccessTy = getAccessType(TTI, UI->getUser());
+            MemAccessTy AccessTy =
+                getAccessType(TTI, UI->getUser(), UI->getOperandValToReplace());
             int64_t Scale = C->getSExtValue();
             if (TTI.isLegalAddressingMode(AccessTy.MemTy, /*BaseGV=*/nullptr,
                                           /*BaseOffset=*/0,
                                           /*HasBaseReg=*/false, Scale,
                                           AccessTy.AddrSpace))
               goto decline_post_inc;
             Scale = -Scale;
             if (TTI.isLegalAddressingMode(AccessTy.MemTy, /*BaseGV=*/nullptr,
static bool canFoldIVIncExpr(const SCEV *IncExpr, Instruction *UserInst,
                              Value *Operand, const TargetTransformInfo &TTI) {
   const SCEVConstant *IncConst = dyn_cast<SCEVConstant>(IncExpr);
   if (!IncConst || !isAddressUse(TTI, UserInst, Operand))
     return false;
 
   if (IncConst->getAPInt().getMinSignedBits() > 64)
     return false;
 
-  MemAccessTy AccessTy = getAccessType(TTI, UserInst);
+  MemAccessTy AccessTy = getAccessType(TTI, UserInst, Operand);
   int64_t IncOffset = IncConst->getValue()->getSExtValue();
   if (!isAlwaysFoldable(TTI, LSRUse::Address, AccessTy, /*BaseGV=*/nullptr,
                         IncOffset, /*HaseBaseReg=*/false))
     return false;
 
   return true;
 }
 
if (IVIncSet.count(UseI)) {
       LLVM_DEBUG(dbgs() << "Use is in profitable chain: " << **UseI << '\n');
       continue;
     }
 
     LSRUse::KindType Kind = LSRUse::Basic;
     MemAccessTy AccessTy;
     if (isAddressUse(TTI, UserInst, U.getOperandValToReplace())) {
       Kind = LSRUse::Address;
-      AccessTy = getAccessType(TTI, UserInst);
+      AccessTy = getAccessType(TTI, UserInst, U.getOperandValToReplace());
     }
 
     const SCEV *S = IU.getExpr(U);
     PostIncLoopSet TmpPostIncLoops = U.getPostIncLoops();
 
     // Equality (== and !=) ICmps are special. We can rewrite (i == N) as
     // (N - i == 0), and this allows (N - i) to be the expression that we work
     // with rather than just N or i, so we can consider the register

test/Transforms/LoopStrengthReduce/AMDGPU/preserve-addrspace-assert.ll

 ; RUN: opt -S -mtriple=amdgcn-amd-amdhsa -loop-reduce %s | FileCheck %s
 
 ; Test for assert resulting from inconsistent isLegalAddressingMode
 ; answers when the address space was dropped from the query.
 
 target datalayout = "e-p:32:32-p1:64:64-p2:64:64-p3:32:32-p4:64:64-p5:32:32-p24:64:64-i64:64-v16:16-v24:32-v32:32-v48:64-v96:128-v192:256-v256:256-v512:512-v1024:1024-v2048:2048-n32:64"
 
 %0 = type { i32, double, i32, float }
 
 ; CHECK-LABEL: @lsr_crash_preserve_addrspace_unknown_type(
 ; CHECK: %tmp4 = bitcast %0 addrspace(3)* %tmp to double addrspace(3)*
 ; CHECK: %scevgep5 = getelementptr double, double addrspace(3)* %tmp4, i32 1
-; CHEC: load double, double addrspace(3)* %scevgep5
+; CHECK: load double, double addrspace(3)* %scevgep5
 
 ; CHECK: %scevgep = getelementptr i32, i32 addrspace(3)* %tmp1, i32 4
 ; CHECK:%tmp14 = load i32, i32 addrspace(3)* %scevgep
 define amdgpu_kernel void @lsr_crash_preserve_addrspace_unknown_type() #0 {
 bb:
   br label %bb1
 
 bb1:                                              ; preds = %bb17, %bb

 bb16:                                             ; preds = %bb12
   unreachable
 
 bb17:                                             ; preds = %bb12, %bb8
   %tmp18 = getelementptr inbounds %0, %0 addrspace(3)* %tmp, i64 2
   br label %bb1
 }
 
+; CHECK-LABEL: @lsr_crash_preserve_addrspace_unknown_type2(
+; CHECK: %scevgep3 = getelementptr i8, i8 addrspace(5)* %array, i32 %j
+; CHECK: %scevgep2 = getelementptr i8, i8 addrspace(5)* %array, i32 %j
+; CHECK: %n8 = load i8, i8 addrspace(5)* %scevgep2, align 4
+; CHECK: call void @llvm.memcpy.p5i8.p5i8.i64(i8 addrspace(5)* %scevgep3, i8 addrspace(5)* %scevgep3, i64 12, i1 false)
+define void @lsr_crash_preserve_addrspace_unknown_type2(i8 addrspace(5)* %array) {
+entry:
+  br label %for.body
+
+for.body:                                         ; preds = %entry, %for.inc
+  %j = phi i32 [ %add, %for.inc ], [ 0, %entry ]
+  %idx = getelementptr inbounds i8, i8 addrspace(5)* %array, i32 %j
+  %t = getelementptr inbounds i8, i8 addrspace(5)* %array, i32 %j
+  %n8 = load i8, i8 addrspace(5)* %t, align 4
+  %n7 = getelementptr inbounds i8, i8 addrspace(5)* %t, i32 42
+  %n9 = load i8, i8 addrspace(5)* %n7, align 4
+  %cmp = icmp sgt i32 %j, 42
+  %add = add nuw nsw i32 %j, 1
+  br i1 %cmp, label %if.then17, label %for.inc
+
+if.then17:                                        ; preds = %for.body
+  call void @llvm.memcpy(i8 addrspace(5)* %idx, i8 addrspace(5)* %idx, i64 12, i1 false)
+  br label %for.inc
+
+for.inc:                                          ; preds = %for.body, %if.then17
+  %exitcond = icmp eq i1 %cmp, 1
+  br i1 %exitcond, label %end, label %for.body
+
+end:                                              ; preds = %for.inc
+  ret void
+}
+
+declare void @llvm.memcpy(i8 addrspace(5)* nocapture writeonly, i8 addrspace(5)* nocapture readonly, i64, i1)
+
 attributes #0 = { nounwind }
 attributes #1 = { nounwind readnone }