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

[craig.topper]

llvm/lib/Target/RISCV/RISCVInsertVSETVLI.cpp

379

ture -> true

382

Wouldn't the lack of tied operand for those indicate they weren't masked.

What if we did

bool MaskAgnostic = true;
if (MI.isRegTiedToUseOperand(0, &UseOpIdx)) {
  MaskAgnostic = RISCVII::hasDummyMaskOp(TSFlags);
}

384

Doesn't have hasDummyMaskOp return a bool? We shouldn't need a conditional operator.

[HsiangKai]

Address comments.

[frasercrmck]

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

llvm/lib/Target/RISCV/RISCVInsertVSETVLI.cpp

379

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."

412

nit: maybe we don't need /*TailAgnostic*/ or /*MaskAgnostic*/ anymore?

llvm/test/CodeGen/RISCV/rvv/maskedoff-undef.ll

4

I don't think we're typically using --riscv-no-aliases in our CodeGen tests?

[craig.topper]

llvm/test/CodeGen/RISCV/rvv/maskedoff-undef.ll

4

We were at one point using that on rvv tests, but I think it's been removed now.

[HsiangKai]

• Add more comments.
• Remove unnecessary --riscv-no-aliases in the test case.

[HsiangKai]

In D106939#2912807, @frasercrmck wrote:

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

Updated test cases are put in https://reviews.llvm.org/D107022.

[HsiangKai]

In D106939#2915134, @HsiangKai wrote:

In D106939#2912807, @frasercrmck wrote:

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

Updated test cases are put in https://reviews.llvm.org/D107022.

Should I combine these two into one patch?

[frasercrmck]

In D106939#2915784, @HsiangKai wrote:

In D106939#2915134, @HsiangKai wrote:

In D106939#2912807, @frasercrmck wrote:

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

Updated test cases are put in https://reviews.llvm.org/D107022.

Should I combine these two into one patch?

Hmm, unsure, the test changes are pretty much unreviewable given how large the diff would be. There's nothing unexpected in there, presumably?

clang/test/CodeGen/RISCV/rvv-intrinsics/maskedoff-undefined.c

+// NOTE: Assertions have been autogenerated by utils/update_cc_test_checks.py
+// REQUIRES: riscv-registered-target
+// RUN: %clang_cc1 -triple riscv64 -target-feature +experimental-v \
+// RUN:   -disable-O0-optnone -emit-llvm %s -o - | opt -S -mem2reg \
+// RUN:   | FileCheck --check-prefix=CHECK-RV64 %s
+
+#include <riscv_vector.h>
+
+// CHECK-RV64-LABEL: @test_vadd_vv_i8m1_m(
+// CHECK-RV64-NEXT:  entry:
+// CHECK-RV64-NEXT:    [[TMP0:%.*]] = call <vscale x 8 x i8> @llvm.riscv.vadd.mask.nxv8i8.nxv8i8.i64(<vscale x 8 x i8> undef, <vscale x 8 x i8> [[OP1:%.*]], <vscale x 8 x i8> [[OP2:%.*]], <vscale x 8 x i1> [[MASK:%.*]], i64 [[VL:%.*]])
+// CHECK-RV64-NEXT:    ret <vscale x 8 x i8> [[TMP0]]
+//
+vint8m1_t test_vadd_vv_i8m1_m (vbool8_t mask, vint8m1_t op1, vint8m1_t op2, size_t vl) {
+  return vadd_vv_i8m1_m(mask, vundefined_i8m1(), op1, op2, vl);
+}

llvm/lib/Target/RISCV/RISCVInsertVSETVLI.cpp

static VSETVLIInfo computeInfoForInstr(const MachineInstr &MI, uint64_t TSFlags,
   unsigned SEW = Log2SEW ? 1 << Log2SEW : 8;
   assert(RISCVVType::isValidSEW(SEW) && "Unexpected SEW");
 
   // Default to tail agnostic unless the destination is tied to a source.
   // Unless the source is undef. In that case the user would have some control
   // over the tail values. Some pseudo instructions force a tail agnostic policy
   // despite having a tied def.
   bool ForceTailAgnostic = RISCVII::doesForceTailAgnostic(TSFlags);
+  // hasDummyMaskOp(TSFlags) == ture means it is a non-masked instruction.
+  // FIXME: hasDummyMaskOp() is the closest attribute to distinguish masked
+  // and non-masked instructions. However, there are some exceptions for this
+  // attribute. PseudoVMV_V_[VXI] and PseudoVFMV_V_F have no need to append
+  // the dummy mask operand in MC lowering and they are non-masked instructions.
+  bool MaskAgnostic = RISCVII::hasDummyMaskOp(TSFlags) ? true : false;
   bool TailAgnostic = true;
   unsigned UseOpIdx;
-  if (!ForceTailAgnostic && MI.isRegTiedToUseOperand(0, &UseOpIdx)) {
+  if (MI.isRegTiedToUseOperand(0, &UseOpIdx)) {
+    if (!ForceTailAgnostic)
       TailAgnostic = false;
-    // If the tied operand is an IMPLICIT_DEF we can keep TailAgnostic.
+    // If the tied operand is an IMPLICIT_DEF we can keep MaskAgnostic and
+    // TailAgnostic.
     const MachineOperand &UseMO = MI.getOperand(UseOpIdx);
     MachineInstr *UseMI = MRI->getVRegDef(UseMO.getReg());
     if (UseMI) {
       UseMI = elideCopies(UseMI, MRI);
-      if (UseMI && UseMI->isImplicitDef())
+      if (UseMI && UseMI->isImplicitDef()) {
+        MaskAgnostic = true;
         TailAgnostic = true;
       }
     }
+  }
 
   if (RISCVII::hasVLOp(TSFlags)) {
     const MachineOperand &VLOp = MI.getOperand(MI.getNumExplicitOperands() - 2);
     if (VLOp.isImm())
       InstrInfo.setAVLImm(VLOp.getImm());
     else
       InstrInfo.setAVLReg(VLOp.getReg());
   } else
     InstrInfo.setAVLReg(RISCV::NoRegister);
   InstrInfo.setVTYPE(VLMul, SEW, /*TailAgnostic*/ TailAgnostic,
-                     /*MaskAgnostic*/ false, MaskRegOp);
+                     /*MaskAgnostic*/ MaskAgnostic, MaskRegOp);
 
   return InstrInfo;
 }
 
 void RISCVInsertVSETVLI::insertVSETVLI(MachineBasicBlock &MBB, MachineInstr &MI,
                                        const VSETVLIInfo &Info,
                                        const VSETVLIInfo &PrevInfo) {
   DebugLoc DL = MI.getDebugLoc();

llvm/test/CodeGen/RISCV/rvv/maskedoff-undef.ll

+; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py
+; RUN: llc -mtriple=riscv64 -mattr=+experimental-v -verify-machineinstrs \
+; RUN:   --riscv-no-aliases < %s | FileCheck %s
+
+declare <vscale x 8 x i8> @llvm.riscv.vadd.mask.nxv8i8.nxv8i8(
+  <vscale x 8 x i8>,
+  <vscale x 8 x i8>,
+  <vscale x 8 x i8>,
+  <vscale x 8 x i1>,
+  i64);
+
+define <vscale x 8 x i8> @intrinsic_vadd_maskedoff_undef(<vscale x 8 x i8> %0, <vscale x 8 x i8> %1, <vscale x 8 x i1> %2, i64 %3) nounwind {
+; CHECK-LABEL: intrinsic_vadd_maskedoff_undef:
+; CHECK:       # %bb.0: # %entry
+; CHECK-NEXT:    vsetvli zero, a0, e8, m1, ta, ma
+; CHECK-NEXT:    vadd.vv v8, v8, v9, v0.t
+; CHECK-NEXT:    jalr zero, 0(ra)
+entry:
+  %a = call <vscale x 8 x i8> @llvm.riscv.vadd.mask.nxv8i8.nxv8i8(
+    <vscale x 8 x i8> undef,
+    <vscale x 8 x i8> %0,
+    <vscale x 8 x i8> %1,
+    <vscale x 8 x i1> %2,
+    i64 %3)
+
+  ret <vscale x 8 x i8> %a
+}