[PATCH] D95659: [RISCV] Initial support of LoopVectorizer for RISC-V Vector.

[Craig Topper via Phabricator via llvm-commits]

craig.topper added inline comments.


================
Comment at: llvm/lib/Target/RISCV/MCTargetDesc/RISCVBaseInfo.h:330
 
+static constexpr unsigned RVVBitsPerBlock = 64;
+
----------------
I'm not sure this should be in this file. This file belongs to the MC layer, but this isn't an MC layer property or a property of the V extension. It's a property of our CodeGen implementation.

I'm not sure where a better place to put it is.


================
Comment at: llvm/lib/Target/RISCV/RISCVISelLowering.cpp:4576
+      (ElemVT == MVT::f64 && Align == 8))
+    return true;
+
----------------
You can probably just the element VTs and then check that Align == ElemVT.getStoreSize() rather than spelling out all of the alignments.

What is considered misaligned for scalable vectors? Should we be checking the alignement is >= the element size?


================
Comment at: llvm/lib/Target/RISCV/RISCVTargetTransformInfo.h:55
+      if (ST->hasStdExtV())
+        return RISCVVType::RVVBitsPerBlock;
+      return 0;
----------------
Is this used with scalable vectors?

AArch64 seems to base their return here for SVE on a command line, but I expected them to require specific scalable vector types in IR for the backend to work.


================
Comment at: llvm/test/Transforms/LoopVectorize/RISCV/scalable-vf-hint.ll:1
+; RUN: opt -mtriple=riscv64 -mattr=+m,+experimental-v -loop-vectorize \
+; RUN:   -riscv-vector-bits-max=512 -S < %s 2>&1 \
----------------
Is this directory new? If so it needs a lit.local.cfg to mark that all tests in it require the RISCV target to be compiled


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D95659