[PATCH] D105996: [AArch64] Enable Upper bound unrolling universally

[Florian Hahn via Phabricator via llvm-commits]

fhahn accepted this revision.
fhahn added a comment.

In D105996#2954149 <https://reviews.llvm.org/D105996#2954149>, @jaykang10 wrote:

> @fhahn Can you share the GeekBench score please? If it is ok, I would like to push this change.

Looks like there are no notable changes to geekbench scores either.

LGTM, which a few nits about the test case



================
Comment at: llvm/test/Transforms/LoopUnroll/AArch64/unroll-upperbound.ll:4
+
+define void @test(i1 %cond) {
+; CHECK-LABEL: @test(
----------------
It would be helpful to add a brief comment here to explain what the test is checking.


================
Comment at: llvm/test/Transforms/LoopUnroll/AArch64/unroll-upperbound.ll:42
+  switch i32 %i.017, label %for.body.if.end6_crit_edge [
+  i32 2, label %if.then4
+  ]
----------------
nit: maybe indent the case?


================
Comment at: llvm/test/Transforms/LoopUnroll/AArch64/unroll-upperbound.ll:45
+
+for.body.if.end6_crit_edge:
+  br label %if.then4
----------------
nit: can the name of the block be shortened/improved?


================
Comment at: llvm/test/Transforms/LoopUnroll/AArch64/unroll-upperbound.ll:46
+for.body.if.end6_crit_edge:
+  br label %if.then4
+
----------------
might also be good to add a call or something here so the loop has some observable effect


================
Comment at: llvm/test/Transforms/LoopUnroll/AArch64/unroll-upperbound.ll:48
+
+if.then4:
+  %inc = add nuw nsw i32 %i.017, 1
----------------
nit: maybe name this block `latch`/`loop.latch` to make it slightly easier to read?


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

https://reviews.llvm.org/D105996