[PATCH] D105996: [AArch64] Enable Upper bound unrolling universally
JinGu Kang via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Fri Aug 20 03:17:45 PDT 2021
jaykang10 added a comment.
In D105996#2956941 <https://reviews.llvm.org/D105996#2956941>, @fhahn wrote:
> 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
Thanks for checking the geek bench score.
After updating the test, let me push this change.
================
Comment at: llvm/test/Transforms/LoopUnroll/AArch64/unroll-upperbound.ll:4
+
+define void @test(i1 %cond) {
+; CHECK-LABEL: @test(
----------------
fhahn wrote:
> It would be helpful to add a brief comment here to explain what the test is checking.
Yep, let me add a comment.
================
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
+ ]
----------------
fhahn wrote:
> nit: maybe indent the case?
Yep, let me change it.
================
Comment at: llvm/test/Transforms/LoopUnroll/AArch64/unroll-upperbound.ll:45
+
+for.body.if.end6_crit_edge:
+ br label %if.then4
----------------
fhahn wrote:
> nit: can the name of the block be shortened/improved?
Yep, let me change it.
================
Comment at: llvm/test/Transforms/LoopUnroll/AArch64/unroll-upperbound.ll:46
+for.body.if.end6_crit_edge:
+ br label %if.then4
+
----------------
fhahn wrote:
> might also be good to add a call or something here so the loop has some observable effect
Yep, let me change it.
================
Comment at: llvm/test/Transforms/LoopUnroll/AArch64/unroll-upperbound.ll:48
+
+if.then4:
+ %inc = add nuw nsw i32 %i.017, 1
----------------
fhahn wrote:
> nit: maybe name this block `latch`/`loop.latch` to make it slightly easier to read?
Yep, let me change it.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D105996/new/
https://reviews.llvm.org/D105996
More information about the llvm-commits
mailing list