Preserve aliasing info during memory intrinsics lowering

[ebrevnov]

Summary

By specification, source and destination of llvm.memcpy.* must either be equal or non-overlapping. This semantics is hard or impossible to figure out once lowered. This patch explicitly marks loads from source and stores to destination as not aliasing if source and destination is known to be not equal.

[xbolva00]

llvm/unittests/Transforms/Utils/MemTransferLowering.cpp

134–135

Please remove code after //

[xbolva00]

llvm/unittests/Transforms/Utils/MemTransferLowering.cpp

115–116

Fragile? Why not simple test to check presence of additional metadata?

[arsenm]

Needs some lit tests that show the metadata after expansion

llvm/lib/Transforms/Utils/LowerMemIntrinsics.cpp

79

Braces

88

Braces

132

Braces

145

Braces

217

Braces

223

Braces

279

Braces

287

Braces

[ebrevnov]

In D118441#3283987, @arsenm wrote:

Needs some lit tests that show the metadata after expansion

The problem is there is now pass which runs this utility. I'm not aware how to exercise it from lit test. For that reason I had to create a unit test. Is there a way to build a lit test in such case?

llvm/lib/Transforms/Utils/LowerMemIntrinsics.cpp

79

According to code style single statement if doesn't need braces.

llvm/unittests/Transforms/Utils/MemTransferLowering.cpp

115–116

I think you are right it should be checking for metadata. On the other hand, my final goal is to make sure the loop is vectorizable. Do you think it's ok to test both cases?

134–135

Sure.

[arsenm]

In D118441#3286321, @ebrevnov wrote:

In D118441#3283987, @arsenm wrote:

Needs some lit tests that show the metadata after expansion

The problem is there is now pass which runs this utility. I'm not aware how to exercise it from lit test. For that reason I had to create a unit test. Is there a way to build a lit test in such case?

AMDGPU uses this, see llvm/test/CodeGen/AMDGPU/lower-mem-intrinsics.ll

llvm/lib/Transforms/Utils/LowerMemIntrinsics.cpp

79

It's spread over multiple lines with the comment, it should have braces

[ebrevnov]

In D118441#3289224, @arsenm wrote:

In D118441#3286321, @ebrevnov wrote:

In D118441#3283987, @arsenm wrote:

Needs some lit tests that show the metadata after expansion

The problem is there is now pass which runs this utility. I'm not aware how to exercise it from lit test. For that reason I had to create a unit test. Is there a way to build a lit test in such case?

AMDGPU uses this, see llvm/test/CodeGen/AMDGPU/lower-mem-intrinsics.ll

In order for this to work I would need to modify AMDGPULowerIntrinsics pass to provide additional argument(result of ScalarEvolutionAnalysis) to expandMemCpyAsLoop. Currently, this pass doesn't depend on SCEV analysis.

[ebrevnov]

Fixed according to comments

[arsenm]

In D118441#3292872, @ebrevnov wrote:

In D118441#3289224, @arsenm wrote:

In D118441#3286321, @ebrevnov wrote:

In D118441#3283987, @arsenm wrote:

Needs some lit tests that show the metadata after expansion

The problem is there is now pass which runs this utility. I'm not aware how to exercise it from lit test. For that reason I had to create a unit test. Is there a way to build a lit test in such case?

AMDGPU uses this, see llvm/test/CodeGen/AMDGPU/lower-mem-intrinsics.ll

In order for this to work I would need to modify AMDGPULowerIntrinsics pass to provide additional argument(result of ScalarEvolutionAnalysis) to expandMemCpyAsLoop. Currently, this pass doesn't depend on SCEV analysis.

That's fine

[ebrevnov]

In D118441#3296826, @arsenm wrote:

In D118441#3292872, @ebrevnov wrote:

In D118441#3289224, @arsenm wrote:

In D118441#3286321, @ebrevnov wrote:

In D118441#3283987, @arsenm wrote:

Needs some lit tests that show the metadata after expansion

The problem is there is now pass which runs this utility. I'm not aware how to exercise it from lit test. For that reason I had to create a unit test. Is there a way to build a lit test in such case?

AMDGPU uses this, see llvm/test/CodeGen/AMDGPU/lower-mem-intrinsics.ll

In order for this to work I would need to modify AMDGPULowerIntrinsics pass to provide additional argument(result of ScalarEvolutionAnalysis) to expandMemCpyAsLoop. Currently, this pass doesn't depend on SCEV analysis.

That's fine

Do you mean it's fine to modify AMDGPULowerIntrinsics pass? If that's what you mean I don't see there is enough rational to do a non NFC change just to be able to create a test.

[xbolva00]

So otherwise upload a new patch which somehow uses this new functionality. What is motivation behind this change? If cannot be tested using current pipeline..

[xbolva00]

And if there is a one - wait until that one is accepted and you can push that one + this one together.

[ebrevnov]

I'm extending functionality of memory lowering utilities which meant to be used in different context. At the moment there is no any upstream pass which will take advantage of added functionality immediately. Rather, we have downstream functionality which will make use of it. In future, upstream users can decide to utilize the functionality on case by case bases... I don't have enough expertise in those areas to drive changes

[xbolva00]

Ok, I think we dont think we really need to require lit test and the provided unit test is good enough.

[arsenm]

In D118441#3318738, @ebrevnov wrote:

Do you mean it's fine to modify AMDGPULowerIntrinsics pass? If that's what you mean I don't see there is enough rational to do a non NFC change just to be able to create a test.

Yes. It's a good improvement either way, and lit tests are much more stable and maintainable than ones directly touching the APIs

[arsenm]

LGTM but a lit test would be much more maintainable

llvm/unittests/Transforms/Utils/MemTransferLowering.cpp

170

These operands should be swapped to get correct error messages

[ebrevnov]

llvm/unittests/Transforms/Utils/MemTransferLowering.cpp

170

Didn't know. Will fix. Thanks

[ebrevnov]

Update

[arsenm]

llvm/lib/Transforms/Utils/LowerMemIntrinsics.cpp

481

Isn't this implied by the usual constraints of memcpy? The source and destination aren't allowed to overlap. Why does this need the SCEV check?

[nikic]

llvm/lib/Transforms/Utils/LowerMemIntrinsics.cpp

481

llvm.memcpy (unlike memcpy) is allowed to have src == dst (but no other overlap).

llvm/include/llvm/Transforms/Utils/LowerMemIntrinsics.h

 
 namespace llvm {
 
 class ConstantInt;
 class Instruction;
 class MemCpyInst;
 class MemMoveInst;
 class MemSetInst;
+class ScalarEvolution;
 class TargetTransformInfo;
 class Value;
 struct Align;
 
 /// Emit a loop implementing the semantics of llvm.memcpy where the size is not
 /// a compile-time constant. Loop will be insterted at \p InsertBefore.
 void createMemCpyLoopUnknownSize(Instruction *InsertBefore, Value *SrcAddr,
-                                 Value *DstAddr, Value *CopyLen,
-                                 Align SrcAlign, Align DestAlign,
-                                 bool SrcIsVolatile, bool DstIsVolatile,
+                                 Value *DstAddr, Value *CopyLen, Align SrcAlign,
+                                 Align DestAlign, bool SrcIsVolatile,
+                                 bool DstIsVolatile, bool CanOverlap,
                                  const TargetTransformInfo &TTI);
 
 /// Emit a loop implementing the semantics of an llvm.memcpy whose size is a
 /// compile time constant. Loop is inserted at \p InsertBefore.
 void createMemCpyLoopKnownSize(Instruction *InsertBefore, Value *SrcAddr,
                                Value *DstAddr, ConstantInt *CopyLen,
                                Align SrcAlign, Align DestAlign,
                                bool SrcIsVolatile, bool DstIsVolatile,
-                               const TargetTransformInfo &TTI);
+                               bool CanOverlap, const TargetTransformInfo &TTI);
 
 /// Expand \p MemCpy as a loop. \p MemCpy is not deleted.
-void expandMemCpyAsLoop(MemCpyInst *MemCpy, const TargetTransformInfo &TTI);
+void expandMemCpyAsLoop(MemCpyInst *MemCpy, const TargetTransformInfo &TTI,
+                        ScalarEvolution *SE = nullptr);
 
 /// Expand \p MemMove as a loop. \p MemMove is not deleted.
 void expandMemMoveAsLoop(MemMoveInst *MemMove);
 
 /// Expand \p MemSet as a loop. \p MemSet is not deleted.
 void expandMemSetAsLoop(MemSetInst *MemSet);
 
 } // End llvm namespace
 
 #endif

llvm/lib/Target/NVPTX/NVPTXLowerAggrCopies.cpp

ConstantInt *CopyLen =
         ConstantInt::get(Type::getInt32Ty(Context), NumLoads);
 
     createMemCpyLoopKnownSize(/* ConvertedInst */ SI,
                               /* SrcAddr */ SrcAddr, /* DstAddr */ DstAddr,
                               /* CopyLen */ CopyLen,
                               /* SrcAlign */ LI->getAlign(),
                               /* DestAlign */ SI->getAlign(),
                               /* SrcIsVolatile */ LI->isVolatile(),
-                              /* DstIsVolatile */ SI->isVolatile(), TTI);
+                              /* DstIsVolatile */ SI->isVolatile(),
+                              /* CanOverlap */ true, TTI);
 
     SI->eraseFromParent();
     LI->eraseFromParent();
   }
 
   // Transform mem* intrinsic calls.
   for (MemIntrinsic *MemCall : MemCalls) {
     if (MemCpyInst *Memcpy = dyn_cast<MemCpyInst>(MemCall)) {

llvm/lib/Transforms/Utils/LowerMemIntrinsics.cpp

 //===- LowerMemIntrinsics.cpp ----------------------------------*- C++ -*--===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 
 #include "llvm/Transforms/Utils/LowerMemIntrinsics.h"
+#include "llvm/Analysis/ScalarEvolution.h"
 #include "llvm/Analysis/TargetTransformInfo.h"
 #include "llvm/IR/IRBuilder.h"
 #include "llvm/IR/IntrinsicInst.h"
+#include "llvm/IR/MDBuilder.h"
 #include "llvm/Transforms/Utils/BasicBlockUtils.h"
 
 using namespace llvm;
 
 void llvm::createMemCpyLoopKnownSize(Instruction *InsertBefore, Value *SrcAddr,
                                      Value *DstAddr, ConstantInt *CopyLen,
                                      Align SrcAlign, Align DstAlign,
                                      bool SrcIsVolatile, bool DstIsVolatile,
+                                     bool CanOverlap,
                                      const TargetTransformInfo &TTI) {
   // No need to expand zero length copies.
   if (CopyLen->isZero())
     return;
 
   BasicBlock *PreLoopBB = InsertBefore->getParent();
   BasicBlock *PostLoopBB = nullptr;
   Function *ParentFunc = PreLoopBB->getParent();
   LLVMContext &Ctx = PreLoopBB->getContext();
   const DataLayout &DL = ParentFunc->getParent()->getDataLayout();
+  MDBuilder MDB(Ctx);
+  MDNode *NewDomain = MDB.createAnonymousAliasScopeDomain("MemCopyDomain");
+  StringRef Name = "MemCopyAliasScope";
+  MDNode *NewScope = MDB.createAnonymousAliasScope(NewDomain, Name);
 
   unsigned SrcAS = cast<PointerType>(SrcAddr->getType())->getAddressSpace();
   unsigned DstAS = cast<PointerType>(DstAddr->getType())->getAddressSpace();
 
   Type *TypeOfCopyLen = CopyLen->getType();
   Type *LoopOpType = TTI.getMemcpyLoopLoweringType(
       Ctx, CopyLen, SrcAS, DstAS, SrcAlign.value(), DstAlign.value());
 
if (LoopEndCount != 0) {
     Align PartSrcAlign(commonAlignment(SrcAlign, LoopOpSize));
 
     IRBuilder<> LoopBuilder(LoopBB);
     PHINode *LoopIndex = LoopBuilder.CreatePHI(TypeOfCopyLen, 2, "loop-index");
     LoopIndex->addIncoming(ConstantInt::get(TypeOfCopyLen, 0U), PreLoopBB);
     // Loop Body
     Value *SrcGEP =
         LoopBuilder.CreateInBoundsGEP(LoopOpType, SrcAddr, LoopIndex);
-    Value *Load = LoopBuilder.CreateAlignedLoad(LoopOpType, SrcGEP,
+    LoadInst *Load = LoopBuilder.CreateAlignedLoad(LoopOpType, SrcGEP,
                                                    PartSrcAlign, SrcIsVolatile);
+    if (!CanOverlap) {
+      // Set alias scope for loads.
+      Load->setMetadata(LLVMContext::MD_alias_scope,
+                        MDNode::get(Ctx, NewScope));
+    }
     Value *DstGEP =
         LoopBuilder.CreateInBoundsGEP(LoopOpType, DstAddr, LoopIndex);
-    LoopBuilder.CreateAlignedStore(Load, DstGEP, PartDstAlign, DstIsVolatile);
-
+    StoreInst *Store = LoopBuilder.CreateAlignedStore(
+        Load, DstGEP, PartDstAlign, DstIsVolatile);
+    if (!CanOverlap) {
+      // Indicate that stores don't overlap loads.
+      Store->setMetadata(LLVMContext::MD_noalias, MDNode::get(Ctx, NewScope));
+    }
     Value *NewIndex =
         LoopBuilder.CreateAdd(LoopIndex, ConstantInt::get(TypeOfCopyLen, 1U));
     LoopIndex->addIncoming(NewIndex, LoopBB);
 
     // Create the loop branch condition.
     Constant *LoopEndCI = ConstantInt::get(TypeOfCopyLen, LoopEndCount);
     LoopBuilder.CreateCondBr(LoopBuilder.CreateICmpULT(NewIndex, LoopEndCI),
                              LoopBB, PostLoopBB);
for (auto OpTy : RemainingOps) {
              "Division should have no Remainder!");
       // Cast source to operand type and load
       PointerType *SrcPtrType = PointerType::get(OpTy, SrcAS);
       Value *CastedSrc = SrcAddr->getType() == SrcPtrType
                              ? SrcAddr
                              : RBuilder.CreateBitCast(SrcAddr, SrcPtrType);
       Value *SrcGEP = RBuilder.CreateInBoundsGEP(
           OpTy, CastedSrc, ConstantInt::get(TypeOfCopyLen, GepIndex));
-      Value *Load =
+      LoadInst *Load =
           RBuilder.CreateAlignedLoad(OpTy, SrcGEP, PartSrcAlign, SrcIsVolatile);
-
+      if (!CanOverlap) {
+        // Set alias scope for loads.
+        Load->setMetadata(LLVMContext::MD_alias_scope,
+                          MDNode::get(Ctx, NewScope));
+      }
       // Cast destination to operand type and store.
       PointerType *DstPtrType = PointerType::get(OpTy, DstAS);
       Value *CastedDst = DstAddr->getType() == DstPtrType
                              ? DstAddr
                              : RBuilder.CreateBitCast(DstAddr, DstPtrType);
       Value *DstGEP = RBuilder.CreateInBoundsGEP(
           OpTy, CastedDst, ConstantInt::get(TypeOfCopyLen, GepIndex));
-      RBuilder.CreateAlignedStore(Load, DstGEP, PartDstAlign, DstIsVolatile);
-
+      StoreInst *Store = RBuilder.CreateAlignedStore(Load, DstGEP, PartDstAlign,
+                                                     DstIsVolatile);
+      if (!CanOverlap) {
+        // Indicate that stores don't overlap loads.
+        Store->setMetadata(LLVMContext::MD_noalias, MDNode::get(Ctx, NewScope));
+      }
       BytesCopied += OperandSize;
     }
   }
   assert(BytesCopied == CopyLen->getZExtValue() &&
          "Bytes copied should match size in the call!");
 }
 
 void llvm::createMemCpyLoopUnknownSize(Instruction *InsertBefore,
                                        Value *SrcAddr, Value *DstAddr,
                                        Value *CopyLen, Align SrcAlign,
                                        Align DstAlign, bool SrcIsVolatile,
-                                       bool DstIsVolatile,
+                                       bool DstIsVolatile, bool CanOverlap,
                                        const TargetTransformInfo &TTI) {
   BasicBlock *PreLoopBB = InsertBefore->getParent();
   BasicBlock *PostLoopBB =
       PreLoopBB->splitBasicBlock(InsertBefore, "post-loop-memcpy-expansion");
 
   Function *ParentFunc = PreLoopBB->getParent();
   const DataLayout &DL = ParentFunc->getParent()->getDataLayout();
   LLVMContext &Ctx = PreLoopBB->getContext();
+  MDBuilder MDB(Ctx);
+  MDNode *NewDomain = MDB.createAnonymousAliasScopeDomain("MemCopyDomain");
+  StringRef Name = "MemCopyAliasScope";
+  MDNode *NewScope = MDB.createAnonymousAliasScope(NewDomain, Name);
+
   unsigned SrcAS = cast<PointerType>(SrcAddr->getType())->getAddressSpace();
   unsigned DstAS = cast<PointerType>(DstAddr->getType())->getAddressSpace();
 
   Type *LoopOpType = TTI.getMemcpyLoopLoweringType(
       Ctx, CopyLen, SrcAS, DstAS, SrcAlign.value(), DstAlign.value());
   unsigned LoopOpSize = DL.getTypeStoreSize(LoopOpType);
 
   IRBuilder<> PLBuilder(PreLoopBB->getTerminator());
void llvm::createMemCpyLoopUnknownSize(Instruction *InsertBefore,
 
   Align PartSrcAlign(commonAlignment(SrcAlign, LoopOpSize));
   Align PartDstAlign(commonAlignment(DstAlign, LoopOpSize));
 
   PHINode *LoopIndex = LoopBuilder.CreatePHI(CopyLenType, 2, "loop-index");
   LoopIndex->addIncoming(ConstantInt::get(CopyLenType, 0U), PreLoopBB);
 
   Value *SrcGEP = LoopBuilder.CreateInBoundsGEP(LoopOpType, SrcAddr, LoopIndex);
-  Value *Load = LoopBuilder.CreateAlignedLoad(LoopOpType, SrcGEP, PartSrcAlign,
-                                              SrcIsVolatile);
+  LoadInst *Load = LoopBuilder.CreateAlignedLoad(LoopOpType, SrcGEP,
+                                                 PartSrcAlign, SrcIsVolatile);
+  if (!CanOverlap) {
+    // Set alias scope for loads.
+    Load->setMetadata(LLVMContext::MD_alias_scope, MDNode::get(Ctx, NewScope));
+  }
   Value *DstGEP = LoopBuilder.CreateInBoundsGEP(LoopOpType, DstAddr, LoopIndex);
+  StoreInst *Store =
       LoopBuilder.CreateAlignedStore(Load, DstGEP, PartDstAlign, DstIsVolatile);
-
+  if (!CanOverlap) {
+    // Indicate that stores don't overlap loads.
+    Store->setMetadata(LLVMContext::MD_noalias, MDNode::get(Ctx, NewScope));
+  }
   Value *NewIndex =
       LoopBuilder.CreateAdd(LoopIndex, ConstantInt::get(CopyLenType, 1U));
   LoopIndex->addIncoming(NewIndex, LoopBB);
 
   if (!LoopOpIsInt8) {
    // Add in the
    Value *RuntimeResidual = PLBuilder.CreateURem(CopyLen, CILoopOpSize);
    Value *RuntimeBytesCopied = PLBuilder.CreateSub(CopyLen, RuntimeResidual);
Value *RuntimeBytesCopied = PLBuilder.CreateSub(CopyLen, RuntimeResidual);
 
     Value *SrcAsInt8 =
         ResBuilder.CreateBitCast(SrcAddr, PointerType::get(Int8Type, SrcAS));
     Value *DstAsInt8 =
         ResBuilder.CreateBitCast(DstAddr, PointerType::get(Int8Type, DstAS));
     Value *FullOffset = ResBuilder.CreateAdd(RuntimeBytesCopied, ResidualIndex);
     Value *SrcGEP =
         ResBuilder.CreateInBoundsGEP(Int8Type, SrcAsInt8, FullOffset);
-    Value *Load = ResBuilder.CreateAlignedLoad(Int8Type, SrcGEP, PartSrcAlign,
-                                               SrcIsVolatile);
+    LoadInst *Load = ResBuilder.CreateAlignedLoad(Int8Type, SrcGEP,
+                                                  PartSrcAlign, SrcIsVolatile);
+    if (!CanOverlap) {
+      // Set alias scope for loads.
+      Load->setMetadata(LLVMContext::MD_alias_scope,
+                        MDNode::get(Ctx, NewScope));
+    }
     Value *DstGEP =
         ResBuilder.CreateInBoundsGEP(Int8Type, DstAsInt8, FullOffset);
-    ResBuilder.CreateAlignedStore(Load, DstGEP, PartDstAlign, DstIsVolatile);
-
+    StoreInst *Store = ResBuilder.CreateAlignedStore(Load, DstGEP, PartDstAlign,
+                                                     DstIsVolatile);
+    if (!CanOverlap) {
+      // Indicate that stores don't overlap loads.
+      Store->setMetadata(LLVMContext::MD_noalias, MDNode::get(Ctx, NewScope));
+    }
     Value *ResNewIndex =
         ResBuilder.CreateAdd(ResidualIndex, ConstantInt::get(CopyLenType, 1U));
     ResidualIndex->addIncoming(ResNewIndex, ResLoopBB);
 
     // Create the loop branch condition.
     ResBuilder.CreateCondBr(
         ResBuilder.CreateICmpULT(ResNewIndex, RuntimeResidual), ResLoopBB,
         PostLoopBB);
Value *NewIndex =
       LoopBuilder.CreateAdd(LoopIndex, ConstantInt::get(TypeOfCopyLen, 1));
   LoopIndex->addIncoming(NewIndex, LoopBB);
 
   LoopBuilder.CreateCondBr(LoopBuilder.CreateICmpULT(NewIndex, CopyLen), LoopBB,
                            NewBB);
 }
 
 void llvm::expandMemCpyAsLoop(MemCpyInst *Memcpy,
-                              const TargetTransformInfo &TTI) {
+                              const TargetTransformInfo &TTI,
+                              ScalarEvolution *SE) {
+  bool CanOverlap = true;
+  if (SE) {
+    auto *SrcSCEV = SE->getSCEV(Memcpy->getRawSource());
+    auto *DestSCEV = SE->getSCEV(Memcpy->getRawDest());
+    if (SE->isKnownPredicateAt(CmpInst::ICMP_NE, SrcSCEV, DestSCEV, Memcpy))
+      CanOverlap = false;
+  }
+
   if (ConstantInt *CI = dyn_cast<ConstantInt>(Memcpy->getLength())) {
     createMemCpyLoopKnownSize(
         /* InsertBefore */ Memcpy,
         /* SrcAddr */ Memcpy->getRawSource(),
         /* DstAddr */ Memcpy->getRawDest(),
         /* CopyLen */ CI,
         /* SrcAlign */ Memcpy->getSourceAlign().valueOrOne(),
         /* DestAlign */ Memcpy->getDestAlign().valueOrOne(),
         /* SrcIsVolatile */ Memcpy->isVolatile(),
         /* DstIsVolatile */ Memcpy->isVolatile(),
+        /* CanOverlap */ CanOverlap,
         /* TargetTransformInfo */ TTI);
   } else {
     createMemCpyLoopUnknownSize(
         /* InsertBefore */ Memcpy,
         /* SrcAddr */ Memcpy->getRawSource(),
         /* DstAddr */ Memcpy->getRawDest(),
         /* CopyLen */ Memcpy->getLength(),
         /* SrcAlign */ Memcpy->getSourceAlign().valueOrOne(),
         /* DestAlign */ Memcpy->getDestAlign().valueOrOne(),
         /* SrcIsVolatile */ Memcpy->isVolatile(),
         /* DstIsVolatile */ Memcpy->isVolatile(),
+        /* CanOverlap */ CanOverlap,
         /* TargetTransformInfo */ TTI);
   }
 }
 
 void llvm::expandMemMoveAsLoop(MemMoveInst *Memmove) {
   createMemMoveLoop(/* InsertBefore */ Memmove,
                     /* SrcAddr */ Memmove->getRawSource(),
                     /* DstAddr */ Memmove->getRawDest(),

llvm/unittests/Transforms/Utils/MemTransferLowering.cpp

ParseAssembly("declare void @llvm.memcpy.p0i8.p0i8.i64(i8*, i8 *, i64, i1)\n"
                 "memcpy:\n"
                 "  call void @llvm.memcpy.p0i8.p0i8.i64(i8* %dst, i8* %src, "
                 "i64 1024, i1 false)\n"
                 "  br label %exit\n"
                 "exit:\n"
                 "  ret void\n"
                 "}\n");
 
   FunctionPassManager FPM;
   FPM.addPass(ForwardingPass(
       [=](Function &F, FunctionAnalysisManager &FAM) -> PreservedAnalyses {
         TargetTransformInfo TTI(M->getDataLayout());
         auto *MemCpyBB = getBasicBlockByName(F, "memcpy");
         Instruction *Inst = &MemCpyBB->front();
         MemCpyInst *MemCpyI = cast<MemCpyInst>(Inst);
-        expandMemCpyAsLoop(MemCpyI, TTI);
+        auto &SE = FAM.getResult<ScalarEvolutionAnalysis>(F);
+        expandMemCpyAsLoop(MemCpyI, TTI, &SE);
         auto *CopyLoopBB = getBasicBlockByName(F, "load-store-loop");
         Instruction *LoadInst =
             getInstructionByOpcode(*CopyLoopBB, Instruction::Load, 1);
-        EXPECT_NONFATAL_FAILURE(
-            EXPECT_NE(LoadInst->getMetadata(LLVMContext::MD_alias_scope),
-                      nullptr),
-            "");
+        EXPECT_NE(nullptr, LoadInst->getMetadata(LLVMContext::MD_alias_scope));
         Instruction *StoreInst =
             getInstructionByOpcode(*CopyLoopBB, Instruction::Store, 1);
-        EXPECT_NONFATAL_FAILURE(
-            EXPECT_NE(StoreInst->getMetadata(LLVMContext::MD_noalias), nullptr),
-            "");
+        EXPECT_NE(nullptr, StoreInst->getMetadata(LLVMContext::MD_noalias));
         return PreservedAnalyses::none();
       }));
   MPM.addPass(createModuleToFunctionPassAdaptor(std::move(FPM)));
 
   MPM.run(*M, MAM);
 }
 
 // This test indirectly checks that loads and stores (generated as a result of
 // llvm.memcpy lowering) doesn't alias by making sure the loop can be
 // successfully vectorized without additional runtime checks.
 TEST_F(MemTransferLowerTest, VecMemCpyKnownLength) {
   ParseAssembly("declare void @llvm.memcpy.p0i8.p0i8.i64(i8*, i8 *, i64, i1)\n"
                 "define void @foo(i8* %dst, i8* %src, i64 %n) optsize {\n"
TEST_F(MemTransferLowerTest, VecMemCpyKnownLength) {
 
   FunctionPassManager FPM;
   FPM.addPass(ForwardingPass(
       [=](Function &F, FunctionAnalysisManager &FAM) -> PreservedAnalyses {
         TargetTransformInfo TTI(M->getDataLayout());
         auto *MemCpyBB = getBasicBlockByName(F, "memcpy");
         Instruction *Inst = &MemCpyBB->front();
         MemCpyInst *MemCpyI = cast<MemCpyInst>(Inst);
-        expandMemCpyAsLoop(MemCpyI, TTI);
+        auto &SE = FAM.getResult<ScalarEvolutionAnalysis>(F);
+        expandMemCpyAsLoop(MemCpyI, TTI, &SE);
         return PreservedAnalyses::none();
       }));
   FPM.addPass(LoopVectorizePass(LoopVectorizeOptions()));
   FPM.addPass(ForwardingPass(
       [=](Function &F, FunctionAnalysisManager &FAM) -> PreservedAnalyses {
         auto *TargetBB = getBasicBlockByName(F, "vector.body");
-        EXPECT_NONFATAL_FAILURE(EXPECT_NE(TargetBB, nullptr), "");
+        EXPECT_NE(nullptr, TargetBB);
         return PreservedAnalyses::all();
       }));
   MPM.addPass(createModuleToFunctionPassAdaptor(std::move(FPM)));
 
   MPM.run(*M, MAM);
 }
 } // namespace