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

[Eli Friedman via Phabricator via llvm-commits]

efriedma added inline comments.


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


================
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();
----------------
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.


================
Comment at: lib/Target/ARM/ARMTargetTransformInfo.h:126
+  void getUnrollingPreferences(Loop *L, ScalarEvolution &SE,
+                               TTI::UnrollingPreferences &UP);
+
----------------
"override"?


https://reviews.llvm.org/D34619