[PATCH] D32536: Extend memcpy expansion in Transform/Utils to handle wider operand types.

[Tony Jiang via Phabricator via llvm-commits]

jtony added inline comments.


================
Comment at: include/llvm/Analysis/TargetTransformInfo.h:704
+
+  /// \returns The operand types to use when copying the residual block of
+  /// memory in a memcpy expansion where the copy size is a compile-time
----------------
I think the \returns is used for function return values. If I understand correctly, here we are using parameter OpsOut to return the operand types, maybe we should use \param OpsOut?


================
Comment at: lib/Transforms/Utils/LowerMemIntrinsics.cpp:186
+  Value *NewIndex =
+      LoopBuilder.CreateAdd(LoopIndex, ConstantInt::get(CopyLenType, 1));
+  LoopIndex->addIncoming(NewIndex, LoopBB);
----------------
Why do we use 1 here and use 0U at line 203?


================
Comment at: lib/Transforms/Utils/LowerMemIntrinsics.cpp:236
+    Value *ResNewIndex =
+        ResBuilder.CreateAdd(ResidualIndex, ConstantInt::get(CopyLenType, 1));
+    ResidualIndex->addIncoming(ResNewIndex, ResLoopBB);
----------------
same issue with line 186.


================
Comment at: lib/Transforms/Utils/LowerMemIntrinsics.cpp:465
+                                /* TargetTransformInfo */ TTI);
+    } else {
+      createMemCpyLoopUnknownSize(/* InsertBefore */ Memcpy,
----------------
This is optional. Is it better to just call createMemCpyLoopKnownSize once and set the CopyLen  parameter using a ternary expression? So we can save a few lines of code.


Repository:
  rL LLVM

https://reviews.llvm.org/D32536