[PATCH] D95125: [WebAssembly] Enable loop unrolling

[Sam Parker via Phabricator via llvm-commits]

samparker added a comment.

Thanks for taking a look!

While I agree that we should be wary of code size increases, it would seem a shame to me to move the burden into all the runtimes when LLVM is the shared resource and a very good place to do unrolling and the subsequent simplifications that it enables. I liken this to vectorization, but this can give performance to targets even when they haven't spent the time implementing their SIMD code generators in the various runtimes. I've had a quick look at how vectorization stacks up against unrolling on my ryzen machine:
F15139305: polybench-unroll-vs-vectorize.png <https://reviews.llvm.org/F15139305>

F15139304: adobe-unroll-vs-vectorize.png <https://reviews.llvm.org/F15139304>

F15139303: tsvc-unroll-vs-vectorize.png <https://reviews.llvm.org/F15139303>

TSVC is a great example of the worst case scenario in unrolling code size increases, it's a single file consisting of 100+ loops that iterate through fixed sized arrays. For vectorization, I suspect that many of vectorized loops neither require a scalar remainder or runtime pointer checks and so hide most of the code size increases that usually come with vectorization.



================
Comment at: llvm/lib/Target/WebAssembly/WebAssemblyTargetTransformInfo.cpp:108
+  Loop *L, ScalarEvolution &SE, TTI::UnrollingPreferences &UP) const {
+  // Scan the loop: don't unroll loops with calls.
+  for (BasicBlock *BB : L->blocks())
----------------
tlively wrote:
> Is this because loops with calls are presumed to not be hot enough to be worth unrolling?
I think the general reason is because a call will be a scheduling barrier so can limit the ILP gains of the unroll, this implementation is basically copied from BasicTTI. But from my basic understanding of V8, calls can also be used as interrupt points and so that extra logic doesn't have to be inserted into loops with calls either.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D95125