[RISCV] Document overview of vector pseudos [nfc]

[reames]

Summary

I tried to give a rough overview of our current pseudo structure. I'm mostly focused on the policy handling bits - since that's what I'm in the process of changing - but touched on the other dimensions in the process of framing it.

[reames]

Fix a minor spelling mistake.

[craig.topper]

llvm/lib/Target/RISCV/RISCVInstrInfo.h

303 ↗

(On Diff #531424)

knownledge -> knowledge

328 ↗

(On Diff #531424)

How do we represent "undefined" for fma, integer multiple-accumulate where the destination is also an arithmetic source. Also vslideup where the destination is used for elements below offset and for the tail, but only tail follows the tail policy.

How do we represent tail "undefined" vs tail "agnostic" for _MASK?

[frasercrmck]

Generally not sure about the commenting style - I don't think this usage necessitates C-style comments as described here: https://llvm.org/docs/CodingStandards.html#comment-formatting

llvm/lib/Target/RISCV/RISCVInstrInfo.h

326 ↗

(On Diff #531424)

existance -> existence

[eopXD]

llvm/lib/Target/RISCV/RISCVInstrInfo.h

299 ↗

(On Diff #531424)

Adding upon this, the behavior of "bit pattern -1"" or "preserve" can be different among different lanes.

321 ↗

(On Diff #531424)

TA is abbreviation for tail agnostic.

I agree that the meaning is ambiguous, as

1. Unmasked intrinsics that model policy behavior, and set to tail-agnostic
2. Intrinsics that does not model a policy behavior

both share this same state.

[luke]

llvm/lib/Target/RISCV/RISCVInstrInfo.h

300 ↗

(On Diff #531424)

Is undefined actually a policy, or just a common representation within the backend? Because I don’t think we can mix both tail undefined and mask undisturbed policies, or that there’s no way to represent it at least

323 ↗

(On Diff #531424)

Not for this patch, but should we standardise the terminology of passthrough vs. merge throughout the backend and pick one? My weak preference would be for passthrough as it’s less confusing with vmerge etc.

[luke]

llvm/lib/Target/RISCV/RISCVInstrInfo.h

323 ↗

(On Diff #531424)

Not all TU pseudos have a policy operand (as of today). Should we document that without the policy operand it defaults to TUMU?

[eopXD]

llvm/lib/Target/RISCV/RISCVInstrInfo.h

323 ↗

(On Diff #531424)

I think all unmasked Pseudo does not have a policy operand.

When the merge operand is undef, the tail policy is agnostic. When the merge operand is not undef, the tail policy is undisturbed.

I don't quite understand what are the instructions are affected by the default setting you have mentioned. My understanding is that at Pseudo instruction level the vector instructions all have explicit policy information. May you elaborate more?

[luke]

llvm/lib/Target/RISCV/RISCVInstrInfo.h

323 ↗

(On Diff #531424)

Thanks for clarifying this, I just did a quick check: with the exception of the VPseudoTiedBinaryNoMask class, unmasked pseudos don't have a policy operand.

The bit that I'm getting confused about is the sentence below:

Otherwise, policy operand and tablegen flags drive the interpretation.

Since TU pseudos are unmasked and don't have a policy operand.

By "default" I meant what policy is computed in computeInfoForInstr in RISCVInsertVSETVLI.cpp when there's no policy operand, and this is the piece of logic here:

bool TailAgnostic = true;
bool MaskAgnostic = true;
if (!hasUndefinedMergeOp(MI, *MRI)) {
  // Start with undisturbed.
  TailAgnostic = false;
  MaskAgnostic = false;

  // If there is a policy operand, use it.
  if (RISCVII::hasVecPolicyOp(TSFlags)) {
    const MachineOperand &Op = MI.getOperand(MI.getNumExplicitOperands() - 1);
    uint64_t Policy = Op.getImm();
    assert(Policy <= (RISCVII::TAIL_AGNOSTIC | RISCVII::MASK_AGNOSTIC) &&
           "Invalid Policy Value");
    TailAgnostic = Policy & RISCVII::TAIL_AGNOSTIC;
    MaskAgnostic = Policy & RISCVII::MASK_AGNOSTIC;
  }

  // Some pseudo instructions force a tail agnostic policy despite having a
  // tied def.
  if (RISCVII::doesForceTailAgnostic(TSFlags))
    TailAgnostic = true;

  if (!RISCVII::usesMaskPolicy(TSFlags))
    MaskAgnostic = true;
}

So as you say, unmasked pseudos don't have a policy operand and so are TUMU "by default", unless the merge operand is undefined in which case it's TAMA.

[eopXD]

llvm/lib/Target/RISCV/RISCVInstrInfo.h

323 ↗

(On Diff #531424)

I think having undisturbed as default may not be a good choice, since the vector extension is targeting high performance, and possibly out-of-order cores. The undisturbed policy is likely to ruin the performance.

[reames]

Update based on comments and further digging.

[reames]

llvm/lib/Target/RISCV/RISCVInstrInfo.h

323 ↗

(On Diff #531424)

I'm trying to move away from a model where the default is meaningful. Always having an explicit policy operand means the instruction carries the flags, and we can simply setup an agnostic flag if useful.

328 ↗

(On Diff #531424)

Right now, we don't represent undefined for either of these cases. We could extend the policy operand with additional bits if desired, but for the moment, but I'm not sure this is warranted at this time.

[luke]

Nice to have the clarification on the policy families, LGTM

[asb]

Left a tiny nit comment, but looks good (and indeed, very helpful!) to me.

llvm/lib/Target/RISCV/RISCVInstrInfoVPseudos.td

47

Nit, should either change the item above to *, or change this one to 2) for consistency.

llvm/lib/Target/RISCV/RISCVInstrInfoVPseudos.td

 //===-- RISCVInstrInfoVPseudos.td - RISC-V 'V' Pseudos -----*- tablegen -*-===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 ///
 /// This file contains the required infrastructure to support code generation
 /// for the standard 'V' (Vector) extension, version 1.0.
 ///
 /// This file is included from RISCVInstrInfoV.td
 ///
+/// Overview of our vector instruction pseudos.  Many of the instructions
+/// have behavior which depends on the value of VTYPE.  Several core aspects of
+/// the compiler - e.g. register allocation - depend on fields in this
+/// configuration register.  The details of which fields matter differ by the
+/// specific instruction, but the common dimensions are:
+///
+/// LMUL/EMUL - Most instructions can write to differently sized register groups
+/// depending on LMUL.
+///
+/// Masked vs Unmasked - Many instructions which allow a mask disallow register
+/// overlap.  As a result, masked vs unmasked require different register
+/// allocation constraints.
+///
+/// Policy - For each of mask and tail policy, there are three options:
+/// * "Undisturbed" - As defined in the specification, required to preserve the
+/// exact bit pattern of inactive lanes.
+/// * "Agnostic" - As defined in the specification, required to either preserve
+/// the exact bit pattern of inactive lanes, or produce the bit pattern -1 for
+/// those lanes.  Note that each lane can make this choice independently.
+/// Instructions which produce masks (and only those instructions) also have the
+/// option of producing a result as-if VL had been VLMAX. 
+/// * "Undefined" - The bit pattern of the inactive lanes is unspecified, and
+/// can be changed without impacting the semantics of the program.  Note that
+/// this concept does not exist in the specification, and requires source
+/// knowledge to be preserved.
+///
+/// SEW - Some instructions have semantics which depend on SEW.  This is
+/// relatively rare, and mostly impacts scheduling and cost estimation.
+///
+/// We have two techniques we use to represent the impact of these fields:
+/// * For fields which don't impact register classes, we largely use
+/// dummy operands on the pseudo instructions which convey information
+/// about the value of VTYPE.
+/// * For fields which do impact register classes (and a few bits of
+/// legacy - see policy discussion below), we define a family of pseudo
+/// instructions for each actual instruction.  Said differently, we encode
+/// each of the preceding fields which are relevant for a given instruction
+/// in the opcode space.
+///
+/// Currently, the policy is represented via the following instrinsic families:
+/// * _MASK - Can represent all three policy states for both tail and mask.  If
+///   passthrough is IMPLICIT_DEF, then represents "undefined".  Otherwise,
+///   policy operand and tablegen flags drive the interpretation.  (If policy
+///   operand is not present - there are a couple, thought we're rapidly
+///   removing them - a non-undefined policy defaults to "tail agnostic", and
+///   "mask undisturbed".  Since this is the only variant with a mask, all
+///   other variants are "mask undefined".
+/// * Unsuffixed w/ both passthrough and policy operand. Can represent all
+///   three policy states.  If passthrough is IMPLICIT_DEF, then represents
+///   "undefined".  Otherwise, policy operand and tablegen flags drive the
+///   interpretation.
+/// * Unsuffixed w/o passthrough or policy operand -- Does not have a
+///   passthrough operand, and thus represents the "undefined" state.  Note
+///   that terminology in code frequently refers to these as "TA" which is
+///   confusing.  We're in the process of migrating away from this
+///   representation.
+/// * _TU w/o policy operand -- Has a passthrough operand, and always
+///   represents the tail undisturbed state.  
+/// * _TU w/policy operand - Can represent all three policy states.  If
+///   passthrough is IMPLICIT_DEF, then represents "undefined".  Otherwise,
+///   policy operand and tablegen flags drive the interpretation.
+///
 //===----------------------------------------------------------------------===//
 
 def riscv_vmv_x_s : SDNode<"RISCVISD::VMV_X_S",
                            SDTypeProfile<1, 1, [SDTCisInt<0>, SDTCisVec<1>,
                                                 SDTCisInt<1>]>>;
 def riscv_read_vlenb : SDNode<"RISCVISD::READ_VLENB",
                               SDTypeProfile<1, 0, [SDTCisVT<0, XLenVT>]>>;