[PATCH] D106939: [RISCV] If the maskedoff is vundefined(), use ta, ma for vsetvli.

Fraser Cormack via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Thu Jul 29 01:47:15 PDT 2021


frasercrmck added a comment.

LGTM but there are test failures. Is that just a whole load of `mu->ma` changes that have been omitted for a smaller diff?



================
Comment at: llvm/lib/Target/RISCV/RISCVInsertVSETVLI.cpp:379
   bool ForceTailAgnostic = RISCVII::doesForceTailAgnostic(TSFlags);
+  bool MaskAgnostic = true;
   bool TailAgnostic = true;
----------------
nit, but purely from a code layout and comment perspective this `MaskAgnostic` is in the middle of two "Tail" variables. Also the "tail" variables are well-commented ahead of time but the "mask" logic is only commented on inside the if statement below. It just looks a bit imbalanced.

I'm wondering if it'd be better there was even something simple like "Default to mask agnostic unless the operation is masked and the destination is tied to a source."


================
Comment at: llvm/lib/Target/RISCV/RISCVInsertVSETVLI.cpp:409
   InstrInfo.setVTYPE(VLMul, SEW, /*TailAgnostic*/ TailAgnostic,
-                     /*MaskAgnostic*/ false, MaskRegOp);
+                     /*MaskAgnostic*/ MaskAgnostic, MaskRegOp);
 
----------------
nit: maybe we don't need `/*TailAgnostic*/` or `/*MaskAgnostic*/` anymore?


================
Comment at: llvm/test/CodeGen/RISCV/rvv/maskedoff-undef.ll:3
+; RUN: llc -mtriple=riscv64 -mattr=+experimental-v -verify-machineinstrs \
+; RUN:   --riscv-no-aliases < %s | FileCheck %s
+
----------------
I don't think we're typically using `--riscv-no-aliases` in our CodeGen tests?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D106939



More information about the cfe-commits mailing list