[PATCH] D99174: [ARM] Enable UpperBound unrolling for all loops

[Sjoerd Meijer via Phabricator via llvm-commits]

SjoerdMeijer added inline comments.


================
Comment at: llvm/lib/Target/ARM/ARMTargetTransformInfo.cpp:2121
+  // below.
+  UP.UpperBound = true;
+
----------------
Looking at this again, I am now confused why we always want to enable this while....


================
Comment at: llvm/lib/Target/ARM/ARMTargetTransformInfo.cpp:2130
   UP.PartialOptSizeThreshold = 0;
   if (L->getHeader()->getParent()->hasOptSize())
     return;
----------------
... here we check of Oz and Os. So we could bail here, but still set UpperBound to true. Is that correct or what we want?


================
Comment at: llvm/test/Transforms/LoopUnroll/ARM/upperbound.ll:78
+; CHECK-NEXT:    [[SWITCH:%.*]] = icmp ult i32 [[L86_OFF]], 24
+; CHECK-NEXT:    br i1 [[SWITCH]], label [[FOR_END_I_IF_END8_I_CRIT_EDGE_I:%.*]], label [[FOR_INC_I_3_I_5:%.*]]
 ; CHECK:       for.end.i.if.end8.i_crit_edge.i:
----------------
dmgreen wrote:
> SjoerdMeijer wrote:
> > I find this test change a bit difficult to read. What exactly is changing here?
> > This first check here s just a check to see if the loop executes at all, but from the other changes below I find it difficult to see if this gets now unrolled or if there are other changes.
> It is getting fully unrolled, which combined with the array values becoming constant and known, managed to simplify most of the unrolled code away.  I was just using it as an example of something that will now be unrolled beneficially.
Ah, okay, got it, cheers.


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

https://reviews.llvm.org/D99174