[PATCH] D152366: [LoopVectorize] Allow inner loop runtime checks to be hoisted above an outer loop

[Paul Walker via Phabricator via llvm-commits]

paulwalker-arm added inline comments.


================
Comment at: llvm/include/llvm/Analysis/LoopAccessAnalysis.h:52-53
+
+  // When creating runtime checks for an inner loop, where possible try to
+  // create checks in such a way that can get hoisted above the outer loop.
+  static bool HoistRuntimeChecks;
----------------
This doesn't read very well.


================
Comment at: llvm/lib/Analysis/LoopAccessAnalysis.cpp:148
+    cl::desc(
+        "Hoist innter loop runtime memory checks to outer loop if possible"),
+    cl::location(VectorizerParams::HoistRuntimeChecks), cl::init(false));
----------------
Typo, should be "inner".


================
Comment at: llvm/lib/Transforms/Utils/LoopUtils.cpp:1646
+  // whereas using a restricted range check could have allowed us to enter at
+  // least once. This why the behaviour is not currently the default and is
+  // controlled by the parameter 'HoistRuntimeChecks'.
----------------
"This is why the"


================
Comment at: llvm/lib/Transforms/Utils/LoopUtils.cpp:1650
+      isa<SCEVAddRecExpr>(High) && isa<SCEVAddRecExpr>(Low) &&
+      cast<SCEVAddRecExpr>(Low)->getLoop() == TheLoop->getParentLoop()) {
+    const Loop *OuterLoop = TheLoop->getParentLoop();
----------------
Given you evaluate `High` using the outer loop's exit count shouldn't you also check the following?
```
cast<SCEVAddRecExpr>(High)->getLoop() == TheLoop->getParentLoop()
```


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

https://reviews.llvm.org/D152366