[PATCH] D36134: [ARM] Improve loop unrolling for Cortex-M

[Sam Parker via Phabricator via llvm-commits]

samparker added inline comments.


================
Comment at: lib/Target/ARM/ARMTargetTransformInfo.cpp:619
+    if (isa<SCEVCouldNotCompute>(Count) || !Count->getType()->isIntegerTy())
+      UP.DefaultUnrollRuntimeCount = 4;
+  }
----------------
efriedma wrote:
> This seems like you're running into some sort of limitation of unrolling infrastructure.  Maybe we need to add a feature to unroll remainder loops?
> 
> Also, which function in the test covers this codepath?
The integer type check isn't actually tested and wasn't something that I was interested in, so I will remove it.

I'm not sure I understand what you mean. For clarity and posterity, the type check on the SCEV is not querying the number of iterations but whether the expression is based on int, float, etc... values. Currently, there is a TODO in the unroller to handle counts with pointer types. The runtime unroller creates a unrolled body and just uses an if-else statement to execute the correct loop, but the original loop is also called after the unrolled loop for the remaining iterations (N % unroll_count). The runtime unroller, by default, will only unroll the loops for which SCEV can produce a trip count because it can guarantee than the basic block can be duplicated and merged. Otherwise, the body can be duplicated but the basic blocks cannot be merged. The iterate_inc function is what tests this and hopefully highlights the problem that the loop count is dependent on the length of the linked list and SCEV cannot be expected to be able express this. 


https://reviews.llvm.org/D36134