[PATCH] D98561: [RISCV] Support masked load/store for fixed vectors.

Mon Mar 15 05:25:46 PDT 2021

frasercrmck added a comment.

Generally, I'd go with the suggestions that clang-tidy has come up with.

================
Comment at: llvm/lib/Target/RISCV/RISCVTargetTransformInfo.h:64
+
+  bool isLegalMaskedLoad(Type *DataType, Align Alignment) {
+    if (!ST->hasStdExtV())
----------------
Personal preference, but since this supports both load and store, for clarity we could make this a separate `isLegalMaskedLoadStore` and have `isLegalMaskedLoad` call that too?

Additionally, is "legality" here just that it shouldn't scalarize? Will the legalizer split too-wide vectors?

================
Comment at: llvm/test/CodeGen/RISCV/rvv/fixed-vectors-masked-load-fp.ll:1
+; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py
+; RUN: llc -mtriple=riscv32 -mattr=+m,+experimental-v,+f,+d,+experimental-zfh -verify-machineinstrs -riscv-v-vector-bits-min=128 -verify-machineinstrs < %s | FileCheck %s --check-prefixes=CHECK,RV32
----------------
It would be good to check that illegal vectors are correctly split in twain and we don't crash/scalarize.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D98561