[RISCVRVVInitUndef] Remove implicit single use assumption for IMPLICIT_DEF (try 2)

preames · preames · commit e93a8137d304 · 2023-08-01T10:50:03.000-07:00
Reapplying after revert due to sanitizer failure. Includes fix to avoid querying dead lanes for vreg introduced by previous transform. The code was written with the implicit assumption that each IMPLICIT_DEF either a) the tied operand, or b) an untied source, but not both. This is true right now, but an upcoming change may allow CSE of IMPLICIT_DEFs in some cases, so let's rewrite the code to handle that possibility. I added an MIR case which demonstrates the multiple use IMPLICIT_DEF. To my knowledge, this is not a reachable configuration from IR right now. As an aside, this makes the structure a much closer match with the sub-reg liveness case, and we can probably just merge these routines. (Future work.) Differential Revision: https://reviews.llvm.org/D156477
diff --git a/llvm/lib/Target/RISCV/RISCVRVVInitUndef.cpp b/llvm/lib/Target/RISCV/RISCVRVVInitUndef.cpp
@@ -49,6 +49,8 @@ class RISCVInitUndef : public MachineFunctionPass {
   const RISCVSubtarget *ST;
   const TargetRegisterInfo *TRI;
 
+  // Newly added vregs, assumed to be fully rewritten
+  SmallSet<Register, 8> NewRegs;
 public:
   static char ID;
 
@@ -118,65 +120,61 @@ static unsigned getUndefInitOpcode(unsigned RegClassID) {
   }
 }
 
+static bool isEarlyClobberMI(MachineInstr &MI) {
+  return llvm::any_of(MI.defs(), [](const MachineOperand &DefMO) {
+    return DefMO.isReg() && DefMO.isEarlyClobber();
+  });
+}
+
 bool RISCVInitUndef::handleImplicitDef(MachineBasicBlock &MBB,
                                        MachineBasicBlock::iterator &Inst) {
-  const TargetRegisterInfo &TRI =
-      *MBB.getParent()->getSubtarget().getRegisterInfo();
-
   assert(Inst->getOpcode() == TargetOpcode::IMPLICIT_DEF);
 
   Register Reg = Inst->getOperand(0).getReg();
   if (!Reg.isVirtual())
     return false;
 
-  bool NeedPseudoInit = false;
+  bool HasOtherUse = false;
   SmallVector<MachineOperand *, 1> UseMOs;
   for (MachineOperand &MO : MRI->use_nodbg_operands(Reg)) {
-    MachineInstr *UserMI = MO.getParent();
-
-    bool HasEarlyClobber = false;
-    bool TiedToDef = false;
-    for (MachineOperand &UserMO : UserMI->operands()) {
-      if (!UserMO.isReg())
-        continue;
-      if (UserMO.isEarlyClobber())
-        HasEarlyClobber = true;
-      if (UserMO.isUse() && UserMO.isTied() &&
-          TRI.regsOverlap(UserMO.getReg(), Reg))
-        TiedToDef = true;
-    }
-    if (HasEarlyClobber && !TiedToDef) {
-      NeedPseudoInit = true;
-      UseMOs.push_back(&MO);
+    if (isEarlyClobberMI(*MO.getParent())) {
+      if (MO.isUse() && !MO.isTied())
+        UseMOs.push_back(&MO);
+      else
+        HasOtherUse = true;
     }
   }
 
-  if (!NeedPseudoInit)
+  if (UseMOs.empty())
     return false;
 
   LLVM_DEBUG(
       dbgs() << "Emitting PseudoRVVInitUndef for implicit vector register "
              << Reg << '\n');
 
-  unsigned RegClassID = getVRLargestSuperClass(MRI->getRegClass(Reg))->getID();
-  unsigned Opcode = getUndefInitOpcode(RegClassID);
+  const TargetRegisterClass *TargetRegClass =
+    getVRLargestSuperClass(MRI->getRegClass(Reg));
+  unsigned Opcode = getUndefInitOpcode(TargetRegClass->getID());
 
-  BuildMI(MBB, Inst, Inst->getDebugLoc(), TII->get(Opcode), Reg);
+  Register NewDest = Reg;
+  if (HasOtherUse) {
+    NewDest = MRI->createVirtualRegister(TargetRegClass);
+    // We don't have a way to update dead lanes, so keep track of the
+    // new register so that we avoid querying it later.
+    NewRegs.insert(NewDest);
+  }
+  BuildMI(MBB, Inst, Inst->getDebugLoc(), TII->get(Opcode), NewDest);
 
-  Inst = MBB.erase(Inst);
+  if (!HasOtherUse)
+    Inst = MBB.erase(Inst);
 
-  for (auto MO : UseMOs)
+  for (auto MO : UseMOs) {
+    MO->setReg(NewDest);
     MO->setIsUndef(false);
-
+  }
   return true;
 }
 
-static bool isEarlyClobberMI(MachineInstr &MI) {
-  return llvm::any_of(MI.defs(), [](const MachineOperand &DefMO) {
-    return DefMO.isReg() && DefMO.isEarlyClobber();
-  });
-}
-
 bool RISCVInitUndef::handleSubReg(MachineFunction &MF, MachineInstr &MI,
                                   const DeadLaneDetector &DLD) {
   bool Changed = false;
@@ -188,6 +186,8 @@ bool RISCVInitUndef::handleSubReg(MachineFunction &MF, MachineInstr &MI,
       continue;
 
     Register Reg = UseMO.getReg();
+    if (NewRegs.count(Reg))
+      continue;
     DeadLaneDetector::VRegInfo Info =
         DLD.getVRegInfo(Register::virtReg2Index(Reg));
 
diff --git a/llvm/test/CodeGen/RISCV/rvv/undef-earlyclobber-chain.ll b/llvm/test/CodeGen/RISCV/rvv/undef-earlyclobber-chain.ll
@@ -1,6 +1,17 @@
 ; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py
 ; RUN: llc -mtriple riscv64 -mattr=+v -riscv-enable-subreg-liveness < %s  | FileCheck %s
 
+define <vscale x 2 x float> @vrgather_all_undef(ptr %p) {
+; CHECK-LABEL: vrgather_all_undef:
+; CHECK:       # %bb.0: # %entry
+; CHECK-NEXT:    vsetivli zero, 0, e32, m1, ta, ma
+; CHECK-NEXT:    vrgather.vi v8, v9, 0
+; CHECK-NEXT:    ret
+entry:
+  %0 = tail call <vscale x 2 x float> @llvm.riscv.vrgather.vx.nxv2f32.i64(<vscale x 2 x float> undef, <vscale x 2 x float> undef, i64 0, i64 0)
+  ret <vscale x 2 x float> %0
+}
+
 define dso_local signext i32 @undef_early_clobber_chain() {
 ; CHECK-LABEL: undef_early_clobber_chain:
 ; CHECK:       # %bb.0: # %entry
@@ -25,14 +36,14 @@ entry:
 define internal void @SubRegLivenessUndefInPhi(i64 %cond) {
 ; CHECK-LABEL: SubRegLivenessUndefInPhi:
 ; CHECK:       # %bb.0: # %start
-; CHECK-NEXT:    blez a0, .LBB1_2
+; CHECK-NEXT:    blez a0, .LBB2_2
 ; CHECK-NEXT:  # %bb.1: # %Cond1
 ; CHECK-NEXT:    vsetvli a0, zero, e16, mf4, ta, ma
 ; CHECK-NEXT:    vid.v v8
 ; CHECK-NEXT:    vadd.vi v10, v8, 1
 ; CHECK-NEXT:    vadd.vi v12, v8, 3
-; CHECK-NEXT:    j .LBB1_3
-; CHECK-NEXT:  .LBB1_2: # %Cond2
+; CHECK-NEXT:    j .LBB2_3
+; CHECK-NEXT:  .LBB2_2: # %Cond2
 ; CHECK-NEXT:    vsetvli a0, zero, e16, mf4, ta, ma
 ; CHECK-NEXT:    vid.v v9
 ; CHECK-NEXT:    csrr a0, vlenb
@@ -48,7 +59,7 @@ define internal void @SubRegLivenessUndefInPhi(i64 %cond) {
 ; CHECK-NEXT:    vadd.vi v9, v9, 3
 ; CHECK-NEXT:    vsetvli zero, a1, e16, m1, ta, ma
 ; CHECK-NEXT:    vslideup.vx v12, v9, a0
-; CHECK-NEXT:  .LBB1_3: # %UseSR
+; CHECK-NEXT:  .LBB2_3: # %UseSR
 ; CHECK-NEXT:    vl1r.v v14, (zero)
 ; CHECK-NEXT:    vsetivli zero, 4, e8, m1, ta, ma
 ; CHECK-NEXT:    vrgatherei16.vv v13, v14, v8
@@ -104,7 +115,7 @@ define internal void @SubRegLivenessUndef() {
 ; CHECK-NEXT:    vid.v v8
 ; CHECK-NEXT:    vadd.vi v10, v8, 1
 ; CHECK-NEXT:    vadd.vi v12, v8, 3
-; CHECK-NEXT:  .LBB2_1: # %loopIR3.i.i
+; CHECK-NEXT:  .LBB3_1: # %loopIR3.i.i
 ; CHECK-NEXT:    # =>This Inner Loop Header: Depth=1
 ; CHECK-NEXT:    vl1r.v v14, (zero)
 ; CHECK-NEXT:    vsetivli zero, 4, e8, m1, ta, ma
@@ -117,7 +128,7 @@ define internal void @SubRegLivenessUndef() {
 ; CHECK-NEXT:    vsetvli a0, zero, e8, m1, ta, ma
 ; CHECK-NEXT:    vand.vv v9, v9, v11
 ; CHECK-NEXT:    vs1r.v v9, (zero)
-; CHECK-NEXT:    j .LBB2_1
+; CHECK-NEXT:    j .LBB3_1
 loopIR.preheader.i.i:
   %v15 = tail call <vscale x 1 x i16> @llvm.experimental.stepvector.nxv1i16()
   %v17 = tail call <vscale x 8 x i16> @llvm.vector.insert.nxv8i16.nxv1i16(<vscale x 8 x i16> poison, <vscale x 1 x i16> %v15, i64 0)
diff --git a/llvm/test/CodeGen/RISCV/rvv/undef-earlyclobber-chain.mir b/llvm/test/CodeGen/RISCV/rvv/undef-earlyclobber-chain.mir
@@ -0,0 +1,91 @@
+# NOTE: Assertions have been autogenerated by utils/update_mir_test_checks.py
+# RUN: llc -mtriple=riscv32 -mattr=+v -riscv-enable-subreg-liveness -run-pass riscv-init-undef -run-pass machineverifier %s -o - | FileCheck %s
+
+--- |
+  source_filename = "<stdin>"
+  target datalayout = "e-m:e-p:64:64-i64:64-i128:128-n32:64-S128"
+  target triple = "riscv64"
+
+  define <vscale x 2 x float> @undef_early_clobber_chain(ptr %p) #0 {
+  entry:
+    %0 = tail call <vscale x 2 x float> @llvm.riscv.vrgather.vx.nxv2f32.i64(<vscale x 2 x float> undef, <vscale x 2 x float> undef, i64 0, i64 0)
+    ret <vscale x 2 x float> %0
+  }
+
+  declare <vscale x 2 x float> @llvm.riscv.vrgather.vx.nxv2f32.i64(<vscale x 2 x float>, <vscale x 2 x float>, i64, i64) #1
+
+  attributes #0 = { "target-features"="+v" }
+  attributes #1 = { nocallback nofree nosync nounwind willreturn memory(none) "target-features"="+v" }
+
+...
+---
+name:            undef_early_clobber_chain
+alignment:       4
+exposesReturnsTwice: false
+legalized:       false
+regBankSelected: false
+selected:        false
+failedISel:      false
+tracksRegLiveness: true
+hasWinCFI:       false
+callsEHReturn:   false
+callsUnwindInit: false
+hasEHCatchret:   false
+hasEHScopes:     false
+hasEHFunclets:   false
+isOutlined:      false
+debugInstrRef:   false
+failsVerification: false
+tracksDebugUserValues: false
+registers:
+  - { id: 0, class: gpr, preferred-register: '' }
+  - { id: 1, class: vr, preferred-register: '' }
+  - { id: 2, class: vr, preferred-register: '' }
+  - { id: 3, class: vr, preferred-register: '' }
+liveins:         []
+frameInfo:
+  isFrameAddressTaken: false
+  isReturnAddressTaken: false
+  hasStackMap:     false
+  hasPatchPoint:   false
+  stackSize:       0
+  offsetAdjustment: 0
+  maxAlignment:    1
+  adjustsStack:    false
+  hasCalls:        false
+  stackProtector:  ''
+  functionContext: ''
+  maxCallFrameSize: 4294967295
+  cvBytesOfCalleeSavedRegisters: 0
+  hasOpaqueSPAdjustment: false
+  hasVAStart:      false
+  hasMustTailInVarArgFunc: false
+  hasTailCall:     false
+  localFrameSize:  0
+  savePoint:       ''
+  restorePoint:    ''
+fixedStack:      []
+stack:           []
+entry_values:    []
+callSites:       []
+debugValueSubstitutions: []
+constants:       []
+machineFunctionInfo:
+  varArgsFrameIndex: 0
+  varArgsSaveSize: 0
+body:             |
+  bb.0.entry:
+    ; CHECK-LABEL: name: undef_early_clobber_chain
+    ; CHECK: [[PseudoRVVInitUndefM1_:%[0-9]+]]:vr = PseudoRVVInitUndefM1
+    ; CHECK-NEXT: [[DEF:%[0-9]+]]:vr = IMPLICIT_DEF
+    ; CHECK-NEXT: dead $x0 = PseudoVSETIVLI 0, 208 /* e32, m1, ta, ma */, implicit-def $vl, implicit-def $vtype
+    ; CHECK-NEXT: early-clobber %1:vr = PseudoVRGATHER_VI_M1 undef [[DEF]], [[PseudoRVVInitUndefM1_]], 0, 0, 5 /* e32 */, 0 /* tu, mu */, implicit $vl, implicit $vtype
+    ; CHECK-NEXT: $v8 = COPY %1
+    ; CHECK-NEXT: PseudoRET implicit $v8
+    %2:vr = IMPLICIT_DEF
+    dead $x0 = PseudoVSETIVLI 0, 208 /* e32, m1, ta, ma */, implicit-def $vl, implicit-def $vtype
+    early-clobber %1:vr = PseudoVRGATHER_VI_M1 undef %2, undef %2, 0, 0, 5 /* e32 */, 0 /* tu, mu */, implicit $vl, implicit $vtype
+    $v8 = COPY %1
+    PseudoRET implicit $v8
+
+...
