[PATCH] D107353: [LoopIdiom] let the pass deal with runtime memset size

[Whitney Tsang via Phabricator via llvm-commits]

Whitney added inline comments.


================
Comment at: llvm/lib/Transforms/Scalar/LoopIdiomRecognize.cpp:925
 
-  APInt Stride = ConstStride->getAPInt();
-  if (SizeInBytes != Stride && SizeInBytes != -Stride)
-    return false;
+  bool IsNegStride;
+  const bool IsConstantSize = isa<ConstantInt>(MSI->getLength());
----------------
initialize the variable? 


================
Comment at: llvm/lib/Transforms/Scalar/LoopIdiomRecognize.cpp:929
+  if (IsConstantSize) {
+    // Memset size is constant
+    // Reject memsets that are so large that they overflow an unsigned.
----------------
Add a period at the end of a sentence. 


================
Comment at: llvm/lib/Transforms/Scalar/LoopIdiomRecognize.cpp:948
+  } else {
+    // Memset size is non-constant
+    // Check if the pointer stride matches the memset size.
----------------
Add a period at the end of a sentence.


================
Comment at: llvm/lib/Transforms/Scalar/LoopIdiomRecognize.cpp:955
+    // will fold expressions that is covered by the loop guard at loop entry.
+    // The pass will compare again after the folding and proceed if equal.
+    LLVM_DEBUG(dbgs() << "  memset size is non-constant\n");
----------------
```
    // If the original StrideSCEV and MemsetSizeSCEV does not match, the pass
    // will fold expressions that is covered by the loop guard at loop entry.
    // The pass will compare again after the folding and proceed if equal.
```
That should be added in the future patch that implement that.


================
Comment at: llvm/test/Transforms/LoopIdiom/memset-runtime-debug.ll:1
+; NOTE: Assertions have been autogenerated by utils/update_test_checks.py UTC_ARGS: -p --function-signature --check-attributes --check-globals --include-generated-funcs --force-update
+; REQUIRES: asserts
----------------
is that true? Or you manually generated this test case?
If not, please remove this line.


================
Comment at: llvm/test/Transforms/LoopIdiom/memset-runtime-debug.ll:47
+;	  for (i=0; i<n; ++i) {
+;     int *arr = ar + i + i * m; // ar[i];
+;     memset(arr, 0, m * sizeof(int));
----------------
That's not ar[i]


================
Comment at: llvm/test/Transforms/LoopIdiom/memset-runtime.ll:2
+; NOTE: Assertions have been autogenerated by utils/update_test_checks.py
+; REQUIRES: asserts
+; RUN: opt -passes="function(loop(loop-idiom,loop-deletion),simplifycfg)" -S < %s | FileCheck %s
----------------
Why do we need assert for this test case?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D107353