[SelectionDAG] Don't apply MinRCSize constraint in InstrEmitter::AddRegisterOperand for IMPLICIT_DEF sources.

[craig.topper]

Summary

MinRCSize is 4 and prevents constrainRegClass from changing the
register class if the new class has size less than 4.

IMPLICIT_DEF gets a unique vreg for each use and will be removed
by the ProcessImplicitDef pass before register allocation. I don't
think there is any reason to prevent constraining the virtual register
to whatever register class the use needs.

The attached test case was previously creating a copy of IMPLICIT_DEF
because vrm8nov0 has 3 registers in it.

[arsenm]

I've never understood the point of this parameter

[reames]

In D128005#3590328, @arsenm wrote:

I've never understood the point of this parameter

I went digging a bit, and as best I can tell this is a heuristic to avoid putting too much pressure on register allocation for virtual regs from small register classes. The idea appears to be that using values copied into a bigger register class is likely to lead to overall better results. As a guess, I think this is a workaround for the fact that we can't spill to other register classes.

For the implicit_def case, that heuristic makes no sense. So LGTM by me too.

[arsenm]

In D128005#3590452, @reames wrote:

As a guess, I think this is a workaround for the fact that we can't spill to other register classes.

But the allocator does look for shared common superclasses to copy to before spilling, so I don't understand this

[arsenm]

In D128005#3590458, @arsenm wrote:

In D128005#3590452, @reames wrote:

As a guess, I think this is a workaround for the fact that we can't spill to other register classes.

But the allocator does look for shared common superclasses to copy to before spilling, so I don't understand this

I guess this doesn't happen for fast regalloc which breaks every normal assumption

[reames]

In D128005#3590458, @arsenm wrote:

In D128005#3590452, @reames wrote:

As a guess, I think this is a workaround for the fact that we can't spill to other register classes.

But the allocator does look for shared common superclasses to copy to before spilling, so I don't understand this

Huh, didn't realize we did that actually. Cool.

In D128005#3590493, @arsenm wrote:

I guess this doesn't happen for fast regalloc which breaks every normal assumption

Sounds entirely plausible.

Which raises the question: what would be the impact of only applying this heuristic in fast regalloc? Or teaching fast reglloc how to do sub-class moves so we can avoid this?

llvm/lib/CodeGen/SelectionDAG/InstrEmitter.cpp

InstrEmitter::AddRegisterOperand(MachineInstrBuilder &MIB,
   // shrink VReg's register class within reason.  For example, if VReg == GR32
   // and II requires a GR32_NOSP, just constrain VReg to GR32_NOSP.
   if (II) {
     const TargetRegisterClass *OpRC = nullptr;
     if (IIOpNum < II->getNumOperands())
       OpRC = TII->getRegClass(*II, IIOpNum, TRI, *MF);
 
     if (OpRC) {
+      unsigned MinNumRegs = MinRCSize;
+      // Don't apply any RC size limit for IMPLICIT_DEF. Each use has a unique
+      // virtual register.
+      if (Op.isMachineOpcode() &&
+          Op.getMachineOpcode() == TargetOpcode::IMPLICIT_DEF)
+        MinNumRegs = 0;
+
       const TargetRegisterClass *ConstrainedRC
-        = MRI->constrainRegClass(VReg, OpRC, MinRCSize);
+        = MRI->constrainRegClass(VReg, OpRC, MinNumRegs);
       if (!ConstrainedRC) {
         OpRC = TRI->getAllocatableClass(OpRC);
         assert(OpRC && "Constraints cannot be fulfilled for allocation");
         Register NewVReg = MRI->createVirtualRegister(OpRC);
         BuildMI(*MBB, InsertPos, Op.getNode()->getDebugLoc(),
                 TII->get(TargetOpcode::COPY), NewVReg).addReg(VReg);
         VReg = NewVReg;
       } else {

llvm/test/CodeGen/RISCV/rvv/implicit-def-copy.ll

+; NOTE: Assertions have been autogenerated by utils/update_mir_test_checks.py
+; RUN: llc < %s -mtriple=riscv64 -mattr=+v -stop-after=finalize-isel | FileCheck %s
+
+; Make sure we don't create a COPY instruction for IMPLICIT_DEF.
+
+define <vscale x 8 x i64> @vpload_nxv8i64(<vscale x 8 x i64>* %ptr, <vscale x 8 x i1> %m, i32 zeroext %evl) #1 {
+  ; CHECK-LABEL: name: vpload_nxv8i64
+  ; CHECK: bb.0 (%ir-block.0):
+  ; CHECK-NEXT:   liveins: $x10, $v0, $x11
+  ; CHECK-NEXT: {{  $}}
+  ; CHECK-NEXT:   [[COPY:%[0-9]+]]:gprnox0 = COPY $x11
+  ; CHECK-NEXT:   [[COPY1:%[0-9]+]]:vr = COPY $v0
+  ; CHECK-NEXT:   [[COPY2:%[0-9]+]]:gpr = COPY $x10
+  ; CHECK-NEXT:   $v0 = COPY [[COPY1]]
+  ; CHECK-NEXT:   [[DEF:%[0-9]+]]:vrm8nov0 = IMPLICIT_DEF
+  ; CHECK-NEXT:   [[PseudoVLE64_V_M8_MASK:%[0-9]+]]:vrm8nov0 = PseudoVLE64_V_M8_MASK [[DEF]], [[COPY2]], $v0, [[COPY]], 6 /* e64 */, 1 :: (load unknown-size from %ir.ptr, align 64)
+  ; CHECK-NEXT:   $v8m8 = COPY [[PseudoVLE64_V_M8_MASK]]
+  ; CHECK-NEXT:   PseudoRET implicit $v8m8
+  %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
+}
+
+declare <vscale x 8 x i64> @llvm.vp.load.nxv8i64.p0nxv8i64(<vscale x 8 x i64>*, <vscale x 8 x i1>, i32)