[PATCH] D113798: Add loop unrolling and peeling preferences for RISCV

[Craig Topper via Phabricator via llvm-commits]

craig.topper added inline comments.


================
Comment at: llvm/lib/Target/RISCV/RISCVTargetTransformInfo.cpp:204
+    for (auto &I : *BB) {
+      // Don't unroll vectorized loop. Vectorization does not benefit from it as
+      // much as scalar code.
----------------
mcberg2021 wrote:
> frasercrmck wrote:
> > Is this truly checking "vectorized loops" or just loops containing vector instructions? We've already checked that the loop isn't vectorized according to metadata. What about code written using RVV intrinsics, or with OpenCL/SYCL/etc? We might want to unroll those loops, right?
> I think for now I am going to mark this with a TODO for more tuning, I updated the comment for vectorized instructions, it will be uploaded soon...
This part of the patch came from a change I made internally. I believe this entire loop was just naively copied from the ARM target.


================
Comment at: llvm/lib/Target/RISCV/RISCVTargetTransformInfo.cpp:217
+
+      SmallVector<const Value *, 4> Operands(I.operand_values());
+      Cost +=
----------------
mcberg2021 wrote:
> frasercrmck wrote:
> > Does the explicit size of `4` help much or should we just use `SmallVector<const Value*>`?
> This setting mirrors SLP's generic vector operand setting, which we utilize, so it does seem appropriate.
This was also copied from ARM.


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

https://reviews.llvm.org/D113798