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 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
