[PATCH] D118441: Preserve aliasing info during memory intrinsics lowering

[Evgeniy via Phabricator via llvm-commits]

ebrevnov added a comment.

In D118441#3283987 <https://reviews.llvm.org/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?



================
Comment at: llvm/lib/Transforms/Utils/LowerMemIntrinsics.cpp:80
                                                 PartSrcAlign, SrcIsVolatile);
+    if (!CanOverlap)
+      // Set alias scope for loads.
----------------
arsenm wrote:
> Braces
According to code style single statement if doesn't need braces.


================
Comment at: llvm/unittests/Transforms/Utils/MemTransferLowering.cpp:120
     // F.dump();
     auto *TargetBB = getBasicBlockByName(F, "vector.body");
+    EXPECT_NE(TargetBB, nullptr);
----------------
xbolva00 wrote:
> Fragile? Why not simple test to check presence of additional metadata?
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?


================
Comment at: llvm/unittests/Transforms/Utils/MemTransferLowering.cpp:126
 
   // llvm::DebugFlag = true;
   // llvm::setCurrentDebugType("loop-vectorize");
----------------
xbolva00 wrote:
> Please remove code after // 
Sure.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D118441/new/

https://reviews.llvm.org/D118441