[RISCV] Properly handle partial writes in isConvertibleToVMV_V_V.

[craig.topper]

Summary

We were only checking for the previous insructions to write exactly
the register or a super register. We ignored writes to a subregister
and continued searching for the producing instruction. We need to
abort instead.

There's another check inside the if body to abort if the registers
don't match exactly. So we just need to check for overlap so we
enter the if body.

The test is new and has not been commited yet.

[craig.topper]

@reames FYI, I think this entire transformation may be conflating tail agnostic and tail undefined.

[fakepaper56]

In D153490#4442663, @craig.topper wrote:

@reames FYI, I think this entire transformation may be conflating tail agnostic and tail undefined.

LGTM. But I am curious what does the "tail undefined" mean?

[craig.topper]

In D153490#4447068, @fakepaper56 wrote:

In D153490#4442663, @craig.topper wrote:

@reames FYI, I think this entire transformation may be conflating tail agnostic and tail undefined.

LGTM. But I am curious what does the "tail undefined" mean?

Philip tried to document that here https://reviews.llvm.org/D152937

The vector spec defines tail agnostic as being all ones or undisturbed (and an extra case for instructions that write masks).

If the user did something like

//vsetvli vlmax
vmv.v.i v8, v8, -1
//vsetvli less than vlmax with tail agnostic.
vadd.vi v8, v16, 1

They could expect the tail elements of vadd.vi to be all ones regardless of how hardware treats tail agnostic. The tail was preloaded with all ones. Tail agnostic for the vadd must either keep them undisturbed or replace them will all ones. I don't know if anyone would really do this. The spec discourages using the tail, but it defined how it behaves so a user could still depend on it.

Tail undefined is the concept that the upper bits can really be any value.

This transform uses the VL of a producing instruction to limit the VL of a copy. This means we don't copy the tail elements of the producing operation. So they're undefined. All we checked was the policy was "tail agnostic". We don't know if the user expected all ones.

For autovectorized code we never create anything where we expect the all ones value.

The C RVV intrinsic interface is way less clear on this. Any intrinsic that doesn't have a passthru operand, is "tail undefined" because the user has no control of register allocation. For intrinsics that "tail agnostic" but have an operand that is tied to the dest like maskedoff for masked instruction or the one of the sources for fma instructions, the user does have control of what register the tail comes from so could depend on the behavior.

I think we should document in the intrinsics API that agnostic really means undefined.

[fakepaper56]

Thank you for the elaboration.

llvm/lib/Target/RISCV/RISCVInstrInfo.cpp

if (MBBI->getOpcode() == RISCV::PseudoVSETVLI ||
       if (MBBI->modifiesRegister(RISCV::VL))
         return false;
 
       // Only converting whole register copies to vmv.v.v when the defining
       // value appears in the explicit operands.
       for (const MachineOperand &MO : MBBI->explicit_operands()) {
         if (!MO.isReg() || !MO.isDef())
           continue;
-        if (!FoundDef && TRI->isSubRegisterEq(MO.getReg(), SrcReg)) {
+        if (!FoundDef && TRI->regsOverlap(MO.getReg(), SrcReg)) {
           // We only permit the source of COPY has the same LMUL as the defined
           // operand.
           // There are cases we need to keep the whole register copy if the LMUL
           // is different.
           // For example,
           // $x0 = PseudoVSETIVLI 4, 73   // vsetivli zero, 4, e16,m2,ta,m
           // $v28m4 = PseudoVWADD_VV_M2 $v26m2, $v8m2
           // # The COPY may be created by vlmul_trunc intrinsic.

llvm/test/CodeGen/RISCV/rvv/vmv-copy.mir

bb.0:
     liveins: $x10, $x11, $v8, $v9
     ; CHECK-LABEL: name: copy_narrow_copies_in_between
     ; CHECK: liveins: $x10, $x11, $v8, $v9
     ; CHECK-NEXT: {{  $}}
     ; CHECK-NEXT: $x0 = PseudoVSETVLI $x10, 201 /* e16, m2, ta, ma */, implicit-def $vl, implicit-def $vtype
     ; CHECK-NEXT: $v10m2 = PseudoVLE16_V_M2 killed $x11, $noreg, 4 /* e16 */, implicit $vl, implicit $vtype
     ; CHECK-NEXT: $v10 = VMV1R_V $v8
     ; CHECK-NEXT: $v11 = VMV1R_V $v9
-    ; CHECK-NEXT: $v12m2 = PseudoVMV_V_V_M2 $v10m2, $noreg, 4 /* e16 */, implicit $vl, implicit $vtype
+    ; CHECK-NEXT: $v12m2 = VMV2R_V $v10m2
     $x0 = PseudoVSETVLI $x10, 201, implicit-def $vl, implicit-def $vtype
     $v10m2 = PseudoVLE16_V_M2 killed $x11, $noreg, 4, implicit $vl, implicit $vtype
     $v10 = COPY $v8
     $v11 = COPY $v9
     $v12m2 = COPY $v10m2
 ...