Revert "[RISCV] Add an GPR def to the Zvlseg SPILL/RELOAD pseudos"

This reverts commit 1f16191. We're seeing some issues with this internally. It seems that when the spill is created by register allocation, the GPR doesn't get allocated and an assertion fires during virtual register rewriting. The .mir test case contains the spill before register allocation so register allocation sees it as any other instruction.
llvm · Oct 2, 2021 · 33d2097 · 33d2097
1 parent ac21e39
commit 33d2097
Show file tree

Hide file tree

Showing 4 changed files with 22 additions and 41 deletions.
diff --git a/llvm/lib/Target/RISCV/RISCVExpandPseudoInsts.cpp b/llvm/lib/Target/RISCV/RISCVExpandPseudoInsts.cpp
@@ -287,10 +287,9 @@ bool RISCVExpandPseudo::expandVSPILL(MachineBasicBlock &MBB,
   const TargetRegisterInfo *TRI =
       MBB.getParent()->getSubtarget().getRegisterInfo();
   DebugLoc DL = MBBI->getDebugLoc();
-  Register AddrInc = MBBI->getOperand(0).getReg();
-  Register SrcReg = MBBI->getOperand(1).getReg();
-  Register Base = MBBI->getOperand(2).getReg();
-  Register VL = MBBI->getOperand(3).getReg();
+  Register SrcReg = MBBI->getOperand(0).getReg();
+  Register Base = MBBI->getOperand(1).getReg();
+  Register VL = MBBI->getOperand(2).getReg();
   auto ZvlssegInfo = TII->isRVVSpillForZvlsseg(MBBI->getOpcode());
   if (!ZvlssegInfo)
     return false;
@@ -319,12 +318,10 @@ bool RISCVExpandPseudo::expandVSPILL(MachineBasicBlock &MBB,
         .addReg(TRI->getSubReg(SrcReg, SubRegIdx + I))
         .addReg(Base)
         .addMemOperand(*(MBBI->memoperands_begin()));
-    if (I != NF - 1) {
-      BuildMI(MBB, MBBI, DL, TII->get(RISCV::ADD), AddrInc)
+    if (I != NF - 1)
+      BuildMI(MBB, MBBI, DL, TII->get(RISCV::ADD), Base)
           .addReg(Base)
           .addReg(VL);
-      Base = AddrInc;
-    }
   }
   MBBI->eraseFromParent();
   return true;
@@ -336,9 +333,8 @@ bool RISCVExpandPseudo::expandVRELOAD(MachineBasicBlock &MBB,
       MBB.getParent()->getSubtarget().getRegisterInfo();
   DebugLoc DL = MBBI->getDebugLoc();
   Register DestReg = MBBI->getOperand(0).getReg();
-  Register AddrInc = MBBI->getOperand(1).getReg();
-  Register Base = MBBI->getOperand(2).getReg();
-  Register VL = MBBI->getOperand(3).getReg();
+  Register Base = MBBI->getOperand(1).getReg();
+  Register VL = MBBI->getOperand(2).getReg();
   auto ZvlssegInfo = TII->isRVVSpillForZvlsseg(MBBI->getOpcode());
   if (!ZvlssegInfo)
     return false;
@@ -367,12 +363,10 @@ bool RISCVExpandPseudo::expandVRELOAD(MachineBasicBlock &MBB,
             TRI->getSubReg(DestReg, SubRegIdx + I))
         .addReg(Base)
         .addMemOperand(*(MBBI->memoperands_begin()));
-    if (I != NF - 1) {
-      BuildMI(MBB, MBBI, DL, TII->get(RISCV::ADD), AddrInc)
+    if (I != NF - 1)
+      BuildMI(MBB, MBBI, DL, TII->get(RISCV::ADD), Base)
           .addReg(Base)
           .addReg(VL);
-      Base = AddrInc;
-    }
   }
   MBBI->eraseFromParent();
   return true;

diff --git a/llvm/lib/Target/RISCV/RISCVInstrInfo.cpp b/llvm/lib/Target/RISCV/RISCVInstrInfo.cpp
@@ -311,17 +311,10 @@ void RISCVInstrInfo::storeRegToStackSlot(MachineBasicBlock &MBB,
         MemoryLocation::UnknownSize, MFI.getObjectAlign(FI));
 
     MFI.setStackID(FI, TargetStackID::ScalableVector);
-    auto MIB = BuildMI(MBB, I, DL, get(Opcode));
-    if (IsZvlsseg) {
-      // We need a GPR register to hold the incremented address for each subreg
-      // after expansion.
-      Register AddrInc =
-          MF->getRegInfo().createVirtualRegister(&RISCV::GPRRegClass);
-      MIB.addReg(AddrInc, RegState::Define);
-    }
-    MIB.addReg(SrcReg, getKillRegState(IsKill))
-        .addFrameIndex(FI)
-        .addMemOperand(MMO);
+    auto MIB = BuildMI(MBB, I, DL, get(Opcode))
+                   .addReg(SrcReg, getKillRegState(IsKill))
+                   .addFrameIndex(FI)
+                   .addMemOperand(MMO);
     if (IsZvlsseg) {
       // For spilling/reloading Zvlsseg registers, append the dummy field for
       // the scaled vector length. The argument will be used when expanding
@@ -412,15 +405,9 @@ void RISCVInstrInfo::loadRegFromStackSlot(MachineBasicBlock &MBB,
         MemoryLocation::UnknownSize, MFI.getObjectAlign(FI));
 
     MFI.setStackID(FI, TargetStackID::ScalableVector);
-    auto MIB = BuildMI(MBB, I, DL, get(Opcode), DstReg);
-    if (IsZvlsseg) {
-      // We need a GPR register to hold the incremented address for each subreg
-      // after expansion.
-      Register AddrInc =
-          MF->getRegInfo().createVirtualRegister(&RISCV::GPRRegClass);
-      MIB.addReg(AddrInc, RegState::Define);
-    }
-    MIB.addFrameIndex(FI).addMemOperand(MMO);
+    auto MIB = BuildMI(MBB, I, DL, get(Opcode), DstReg)
+                   .addFrameIndex(FI)
+                   .addMemOperand(MMO);
     if (IsZvlsseg) {
       // For spilling/reloading Zvlsseg registers, append the dummy field for
       // the scaled vector length. The argument will be used when expanding

diff --git a/llvm/lib/Target/RISCV/RISCVInstrInfoVPseudos.td b/llvm/lib/Target/RISCV/RISCVInstrInfoVPseudos.td
@@ -3498,11 +3498,11 @@ foreach lmul = MxList.m in {
     defvar vreg = SegRegClass<lmul, nf>.RC;
     let hasSideEffects = 0, mayLoad = 0, mayStore = 1, isCodeGenOnly = 1 in {
       def "PseudoVSPILL" # nf # "_" # lmul.MX :
-        Pseudo<(outs GPR:$addrinc), (ins vreg:$rs1, GPR:$rs2, GPR:$vlenb), []>;
+        Pseudo<(outs), (ins vreg:$rs1, GPR:$rs2, GPR:$vlenb), []>;
     }
     let hasSideEffects = 0, mayLoad = 1, mayStore = 0, isCodeGenOnly = 1 in {
       def "PseudoVRELOAD" # nf # "_" # lmul.MX :
-        Pseudo<(outs vreg:$rs1, GPR:$addrinc), (ins GPR:$rs2, GPR:$vlenb), []>;
+        Pseudo<(outs vreg:$rs1), (ins GPR:$rs2, GPR:$vlenb), []>;
     }
   }
 }

diff --git a/llvm/test/CodeGen/RISCV/rvv/zvlsseg-spill.mir b/llvm/test/CodeGen/RISCV/rvv/zvlsseg-spill.mir
@@ -30,10 +30,10 @@ body: |
     ; CHECK-NEXT: $v0_v1_v2_v3_v4_v5_v6 = PseudoVLSEG7E64_V_M1 renamable $x10, $noreg, 6, implicit $vl, implicit $vtype
     ; CHECK-NEXT: $x11 = ADDI $x2, 16
     ; CHECK-NEXT: $x12 = PseudoReadVLENB
-    ; CHECK-NEXT: dead renamable $x11 = PseudoVSPILL7_M1 killed renamable $v0_v1_v2_v3_v4_v5_v6, killed $x11, killed $x12
+    ; CHECK-NEXT: PseudoVSPILL7_M1 killed renamable $v0_v1_v2_v3_v4_v5_v6, killed $x11, killed $x12
     ; CHECK-NEXT: $x11 = ADDI $x2, 16
     ; CHECK-NEXT: $x12 = PseudoReadVLENB
-    ; CHECK-NEXT: dead renamable $v7_v8_v9_v10_v11_v12_v13, dead renamable $x11 = PseudoVRELOAD7_M1 killed $x11, killed $x12, implicit-def $v8
+    ; CHECK-NEXT: dead renamable $v7_v8_v9_v10_v11_v12_v13 = PseudoVRELOAD7_M1 killed $x11, killed $x12, implicit-def $v8
     ; CHECK-NEXT: VS1R_V killed $v8, killed renamable $x10
     ; CHECK-NEXT: $x10 = frame-destroy PseudoReadVLENB
     ; CHECK-NEXT: $x10 = frame-destroy SLLI killed $x10, 3
@@ -43,8 +43,8 @@ body: |
     %0:gpr = COPY $x10
     %1:gprnox0 = COPY $x11
     $v0_v1_v2_v3_v4_v5_v6 = PseudoVLSEG7E64_V_M1 %0, %1, 6
-    %2:gpr = PseudoVSPILL7_M1 killed renamable $v0_v1_v2_v3_v4_v5_v6, %stack.0, $x0
-    renamable $v7_v8_v9_v10_v11_v12_v13, %3:gpr = PseudoVRELOAD7_M1 %stack.0, $x0
+    PseudoVSPILL7_M1 killed renamable $v0_v1_v2_v3_v4_v5_v6, %stack.0, $x0
+    renamable $v7_v8_v9_v10_v11_v12_v13 = PseudoVRELOAD7_M1 %stack.0, $x0
     VS1R_V killed $v8, %0:gpr
     PseudoRET
 ...