[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