[RISCV] Don't use tail agnostic policy on instructions where destination is tied to source

[craig.topper]

Summary

If the destination is tied, then user has some control of the
register used for input. They would have the ability to control
the value of any tail elements. By using tail agnostic we take
this option away from them.

Its not clear that the intrinsics are defined such that this isn't
supposed to work. And undisturbed is a valid implementation for agnostic
so code wouldn't even fail to work on all systems if we always used
agnostic.

The vcompress intrinsic is defined to require tail undisturbed so
at minimum we need this for that instruction or need to redefine
the intrinsic.

I've made an exception here for vmv.s.x/fmv.s.f and reduction
instructions which only write to element 0 regardless of the tail
policy. This allows us to keep the agnostic policy on those which
should allow better redundant vsetvli removal.

An enhancement would be to check for undef input and keep the
agnostic policy, but we don't have good test coverage for that yet.

[khchen]

llvm/lib/Target/RISCV/RISCVISelLowering.cpp

2151

If the tail policy is ignored on WritesElement0 instructions, why not set tail policy as tu?
Is it because spec said If anything, the default should be tail-agnostic/mask-agnostic, so software has to specify when it cares about the non-participating elements?

If we set anything as tu except undefined maskedoff, could it also make vsetvli removal easier.

[craig.topper]

llvm/lib/Target/RISCV/RISCVISelLowering.cpp

2151

tu is bad for performance on a core implementing out of order or super scalar execution of vector instructions. It prevents an instruction from executing until the last instruction that wrote its destination register finishes executing even if they aren't related instructions. So I think it will be important for performance.

I left ta on the WriteElement0 instructions since that's what we use on vsetvli intrinsics so I was hoping to eliminate unnecessary vtype changes assuming ta is more common. But its admittedly a simple policy and we might need something smarter in the vsetvli removal pass to detect unneeded changes of the tail policy for instructions that don't care.

[khchen]

thanks for fixup, LGTM.

llvm/lib/Target/RISCV/RISCVISelLowering.cpp

2151

Got it, assuming ta is more common make sense to me.

llvm/lib/Target/RISCV/RISCVISelLowering.cpp

static MachineBasicBlock *emitSelectPseudo(MachineInstr &MI,
   }
 
   F->getProperties().reset(MachineFunctionProperties::Property::NoPHIs);
   return TailMBB;
 }
 
 static MachineBasicBlock *addVSetVL(MachineInstr &MI, MachineBasicBlock *BB,
                                     int VLIndex, unsigned SEWIndex,
-                                    unsigned VLMul) {
+                                    unsigned VLMul, bool WritesElement0) {
   MachineFunction &MF = *BB->getParent();
   DebugLoc DL = MI.getDebugLoc();
   const TargetInstrInfo &TII = *MF.getSubtarget().getInstrInfo();
 
   unsigned SEW = MI.getOperand(SEWIndex).getImm();
   assert(RISCVVType::isValidSEW(SEW) && "Unexpected SEW");
   RISCVVSEW ElementWidth = static_cast<RISCVVSEW>(Log2_32(SEW / 8));
 
if (VLIndex >= 0) {
     Register DestReg = MRI.createVirtualRegister(&RISCV::GPRRegClass);
     MIB.addReg(DestReg, RegState::Define | RegState::Dead)
         .addReg(MI.getOperand(VLIndex).getReg());
   } else
     // With no VL operator in the pseudo, do not modify VL (rd = X0, rs1 = X0).
     MIB.addReg(RISCV::X0, RegState::Define | RegState::Dead)
         .addReg(RISCV::X0, RegState::Kill);
 
+  // Default to tail agnostic unless the destination is tied to a source. In
+  // that case the user would have some control over the tail values. The tail
+  // policy is also ignored on instructions that only update element 0 like
+  // vmv.s.x or reductions so use agnostic there to match the common case.
+  // FIXME: This is conservatively correct, but we might want to detect that
+  // the input is undefined.
+  bool TailAgnostic = true;
+  if (MI.isRegTiedToUseOperand(0) && !WritesElement0)
+    TailAgnostic = false;
+
   // For simplicity we reuse the vtype representation here.
   MIB.addImm(RISCVVType::encodeVTYPE(Multiplier, ElementWidth,
-                                     /*TailAgnostic*/ true,
+                                     /*TailAgnostic*/ TailAgnostic,
                                      /*MaskAgnostic*/ false));
 
   // Remove (now) redundant operands from pseudo
   MI.getOperand(SEWIndex).setImm(-1);
   if (VLIndex >= 0) {
     MI.getOperand(VLIndex).setReg(RISCV::NoRegister);
     MI.getOperand(VLIndex).setIsKill(false);
   }
 
   return BB;
 }
 
 MachineBasicBlock *
 RISCVTargetLowering::EmitInstrWithCustomInserter(MachineInstr &MI,
                                                  MachineBasicBlock *BB) const {
 
   if (const RISCVVPseudosTable::PseudoInfo *RVV =
           RISCVVPseudosTable::getPseudoInfo(MI.getOpcode())) {
     int VLIndex = RVV->getVLIndex();
     int SEWIndex = RVV->getSEWIndex();
+    bool WritesElement0 = RVV->writesElement0();
 
     assert(SEWIndex >= 0 && "SEWIndex must be >= 0");
-    return addVSetVL(MI, BB, VLIndex, SEWIndex, RVV->VLMul);
+    return addVSetVL(MI, BB, VLIndex, SEWIndex, RVV->VLMul, WritesElement0);
   }
 
   switch (MI.getOpcode()) {
   default:
     llvm_unreachable("Unexpected instr type to insert");
   case RISCV::ReadCycleWide:
     assert(!Subtarget.is64Bit() &&
            "ReadCycleWrite is only to be used on riscv32");