[PATCH] D34619: [ARM] Enable partial and runtime unrolling

[Sam Parker via Phabricator via llvm-commits]

samparker added inline comments.


================
Comment at: lib/Target/ARM/ARMTargetTransformInfo.cpp:552
+  if (!ST->isThumb2() || !ST->isMClass() || (L->getNumBlocks() != 1))
+    return;
+
----------------
efriedma wrote:
> If you want this to be a no-op for other architectures, shouldn't this be "return BasicTTIImplBase::getUnrollingPreferences(L, SE, UP)"?
ah yes, thanks.


================
Comment at: lib/Target/ARM/ARMTargetTransformInfo.cpp:559
+  // Scan the loop: don't unroll loops with calls as this could prevent
+  // inlining.
+  BasicBlock *BB = L->getLoopLatch();
----------------
efriedma wrote:
> Do we really call into getUnrollingPreferences for the early unroll pass?  We probably don't want to be using target-specific unroll heuristics for "createSimpleLoopUnrollPass".  But I guess that's not something you need to fix in this patch.
As far as I can see, the unroll pass functionality can only be controlled externally via the command line options, or by target hooks.


================
Comment at: lib/Target/ARM/ARMTargetTransformInfo.h:126
+  void getUnrollingPreferences(Loop *L, ScalarEvolution &SE,
+                               TTI::UnrollingPreferences &UP);
+
----------------
efriedma wrote:
> "override"?
oddly no, it's not virtual.


https://reviews.llvm.org/D34619