[RISCV] Delete dead elideCopy code in InsertVSETVLI [nfc]

[reames]

Summary

This code appears to be dead. A simple whole register copy of an IMPLICIT_DEF, is simply an IMPLICIT_DEF of it's own. (This would not be true for freeze, but is for copy.)

If we find a case which gets here with vector operand copy of an IMPLICIT_DEF, we most likely have an earlier missed optimization anyways.

[craig.topper]

Looking at this more, this code may only be dead because we don't do the tied operand check if a policy operand is present. That's probably true for a significant number of instructions these days.

Ignoring that and looking at what the MIR looks like at the time we reach InsertVSETVLI for this test case

define <vscale x 8 x i64> @vpload_nxv8i64(<vscale x 8 x i64>* %ptr, <vscale x 8 x i1> %m, i32 zeroext %evl) #1 {
  %load = call <vscale x 8 x i64> @llvm.vp.load.nxv8i64.p0nxv8i64(<vscale x 8 x i64>* %ptr, <vscale x 8 x i1> %m, i32 %evl)
  ret <vscale x 8 x i64> %load                                                   
}

I see

# Machine code for function vpload_nxv8i64: IsSSA, TracksLiveness                
Function Live Ins: $x10 in %0, $v0 in %1, $x11 in %2                             
                                                                                 
bb.0 (%ir-block.0):                                                              
  liveins: $x10, $v0, $x11                                                       
  %2:gprnox0 = COPY $x11                                                         
  %1:vr = COPY $v0                                                               
  %0:gpr = COPY $x10                                                             
  $v0 = COPY %1:vr                                                               
  %4:vrm8 = IMPLICIT_DEF                                                         
  %5:vrm8nov0 = COPY %4:vrm8                                                     
  %3:vrm8nov0 = PseudoVLE64_V_M8_MASK %5:vrm8nov0(tied-def 0), %0:gpr, $v0, %2:gprnox0, 6, 1 :: (load unknown-size from %ir.ptr, align 64)
  $v8m8 = COPY %3:vrm8nov0                                                       
  PseudoRET implicit $v8m8                                                       
                                                                                 
# End machine code for function vpload_nxv8i64.

That shows an IMPLICIT_DEF behind a copy instruction. I think it is related to vrm8nov0 having 3 registers. It doesn't happen on an LMUL=4 test. I suspect it hit the MinRCSize limit of 4 in InstrEmitter::AddRegisterOperand.

[craig.topper]

In D127996#3590034, @craig.topper wrote:

Looking at this more, this code may only be dead because we don't do the tied operand check if a policy operand is present. That's probably true for a significant number of instructions these days.

Ignoring that and looking at what the MIR looks like at the time we reach InsertVSETVLI for this test case

define <vscale x 8 x i64> @vpload_nxv8i64(<vscale x 8 x i64>* %ptr, <vscale x 8 x i1> %m, i32 zeroext %evl) #1 {
  %load = call <vscale x 8 x i64> @llvm.vp.load.nxv8i64.p0nxv8i64(<vscale x 8 x i64>* %ptr, <vscale x 8 x i1> %m, i32 %evl)
  ret <vscale x 8 x i64> %load                                                   
}

I see

# Machine code for function vpload_nxv8i64: IsSSA, TracksLiveness                
Function Live Ins: $x10 in %0, $v0 in %1, $x11 in %2                             
                                                                                 
bb.0 (%ir-block.0):                                                              
  liveins: $x10, $v0, $x11                                                       
  %2:gprnox0 = COPY $x11                                                         
  %1:vr = COPY $v0                                                               
  %0:gpr = COPY $x10                                                             
  $v0 = COPY %1:vr                                                               
  %4:vrm8 = IMPLICIT_DEF                                                         
  %5:vrm8nov0 = COPY %4:vrm8                                                     
  %3:vrm8nov0 = PseudoVLE64_V_M8_MASK %5:vrm8nov0(tied-def 0), %0:gpr, $v0, %2:gprnox0, 6, 1 :: (load unknown-size from %ir.ptr, align 64)
  $v8m8 = COPY %3:vrm8nov0                                                       
  PseudoRET implicit $v8m8                                                       
                                                                                 
# End machine code for function vpload_nxv8i64.

That shows an IMPLICIT_DEF behind a copy instruction. I think it is related to vrm8nov0 having 3 registers. It doesn't happen on an LMUL=4 test. I suspect it hit the MinRCSize limit of 4 in InstrEmitter::AddRegisterOperand.

I think this might be a bug in InstrEmitter. InstrEmitter::getVR creates unique vreg for every IMPLICIT_DEF use, but then we aren't allowed to constrain it below MinRCSize.

[craig.topper]

In D127996#3590050, @craig.topper wrote:

In D127996#3590034, @craig.topper wrote:

Looking at this more, this code may only be dead because we don't do the tied operand check if a policy operand is present. That's probably true for a significant number of instructions these days.

Ignoring that and looking at what the MIR looks like at the time we reach InsertVSETVLI for this test case

define <vscale x 8 x i64> @vpload_nxv8i64(<vscale x 8 x i64>* %ptr, <vscale x 8 x i1> %m, i32 zeroext %evl) #1 {
  %load = call <vscale x 8 x i64> @llvm.vp.load.nxv8i64.p0nxv8i64(<vscale x 8 x i64>* %ptr, <vscale x 8 x i1> %m, i32 %evl)
  ret <vscale x 8 x i64> %load                                                   
}

I see

# Machine code for function vpload_nxv8i64: IsSSA, TracksLiveness                
Function Live Ins: $x10 in %0, $v0 in %1, $x11 in %2                             
                                                                                 
bb.0 (%ir-block.0):                                                              
  liveins: $x10, $v0, $x11                                                       
  %2:gprnox0 = COPY $x11                                                         
  %1:vr = COPY $v0                                                               
  %0:gpr = COPY $x10                                                             
  $v0 = COPY %1:vr                                                               
  %4:vrm8 = IMPLICIT_DEF                                                         
  %5:vrm8nov0 = COPY %4:vrm8                                                     
  %3:vrm8nov0 = PseudoVLE64_V_M8_MASK %5:vrm8nov0(tied-def 0), %0:gpr, $v0, %2:gprnox0, 6, 1 :: (load unknown-size from %ir.ptr, align 64)
  $v8m8 = COPY %3:vrm8nov0                                                       
  PseudoRET implicit $v8m8                                                       
                                                                                 
# End machine code for function vpload_nxv8i64.

That shows an IMPLICIT_DEF behind a copy instruction. I think it is related to vrm8nov0 having 3 registers. It doesn't happen on an LMUL=4 test. I suspect it hit the MinRCSize limit of 4 in InstrEmitter::AddRegisterOperand.

I think this might be a bug in InstrEmitter. InstrEmitter::getVR creates unique vreg for every IMPLICIT_DEF use, but then we aren't allowed to constrain it below MinRCSize.

https://reviews.llvm.org/D128005

[craig.topper]

LGTM

llvm/lib/Target/RISCV/RISCVInsertVSETVLI.cpp

 /// VL and only sets VTYPE.
 static bool isVLPreservingConfig(const MachineInstr &MI) {
   if (MI.getOpcode() != RISCV::PseudoVSETVLIX0)
     return false;
   assert(RISCV::X0 == MI.getOperand(1).getReg());
   return RISCV::X0 == MI.getOperand(0).getReg();
 }
 
-static MachineInstr *elideCopies(MachineInstr *MI,
-                                 const MachineRegisterInfo *MRI) {
-  while (true) {
-    if (!MI->isFullCopy())
-      return MI;
-    if (!Register::isVirtualRegister(MI->getOperand(1).getReg()))
-      return nullptr;
-    MI = MRI->getVRegDef(MI->getOperand(1).getReg());
-    if (!MI)
-      return nullptr;
-  }
-}
-
 static VSETVLIInfo computeInfoForInstr(const MachineInstr &MI, uint64_t TSFlags,
                                        const MachineRegisterInfo *MRI) {
   VSETVLIInfo InstrInfo;
 
   // If the instruction has policy argument, use the argument.
   // If there is no policy argument, 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 policy values.
if (RISCVII::hasVecPolicyOp(TSFlags)) {
     MaskAgnostic = Policy & RISCVII::MASK_AGNOSTIC;
   } else if (MI.isRegTiedToUseOperand(0, &UseOpIdx)) {
     TailAgnostic = false;
     if (UsesMaskPolicy)
       MaskAgnostic = false;
     // If the tied operand is an IMPLICIT_DEF we can keep TailAgnostic.
     const MachineOperand &UseMO = MI.getOperand(UseOpIdx);
     MachineInstr *UseMI = MRI->getVRegDef(UseMO.getReg());
-    if (UseMI) {
-      UseMI = elideCopies(UseMI, MRI);
-      if (UseMI && UseMI->isImplicitDef()) {
-        TailAgnostic = true;
-        if (UsesMaskPolicy)
-          MaskAgnostic = true;
-      }
-    }
     // Some pseudo instructions force a tail agnostic policy despite having a
     // tied def.
     if (RISCVII::doesForceTailAgnostic(TSFlags))
       TailAgnostic = true;
   }
 
   RISCVII::VLMUL VLMul = RISCVII::getLMul(TSFlags);