[PATCH] D97947: [AArch64] Enable runtime unrolling for in-order scheduling models

[Nicholas Guy via Phabricator via llvm-commits]

NickGuy added inline comments.


================
Comment at: llvm/lib/Target/AArch64/AArch64TargetTransformInfo.cpp:1110
+
+  // Scan the loop: don't unroll loops with calls as this could prevent
+  // inlining.
----------------
fhahn wrote:
> NickGuy wrote:
> > fhahn wrote:
> > > This comment seems out of date?
> > Doesn't seem like it is to me. If a function call is found, it bails without enabling runtime unrolling. This check is also performed by many other implementations of this hook.
> >  If a function call is found,
> That's correct, but you are also bailing out on other cases, like instructions with vector types, right?
Right, I misunderstood. I've updated it to mention that we bail out in vector loops.


================
Comment at: llvm/lib/Target/AArch64/AArch64TargetTransformInfo.cpp:1128
+
+  // Force runtime unrolling for in-order models
+  // If mcpu is omitted, then an in-order model will be used, meaning this
----------------
dmgreen wrote:
> "Force" doesn't sound like the right wording to me. It is enabling runtime/partial unrolling, which the loop unroller may use or not.
Reworded both the commit message and this comment


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97947