[RISCV] Fix a bug where AVL is the last MI in MBB.

[NexMing]

When AVL is the last MI, std::next(II) equals MBB.end(), and calling II->getParent() at that point will cause an error.

[llvmbot]

@llvm/pr-subscribers-backend-risc-v

Author: MingYan (NexMing)

Changes

When AVL is the last MI, std::next(II) equals MBB.end(), and calling II->getParent() at that point will cause an error.

---

Full diff: https://github.com/llvm/llvm-project/pull/144668.diff

2 Files Affected:

• (modified) llvm/lib/Target/RISCV/RISCVInsertVSETVLI.cpp (+8-7)
• (modified) llvm/test/CodeGen/RISCV/rvv/vsetvli-insert-crossbb.mir (+67)

diff --git a/llvm/lib/Target/RISCV/RISCVInsertVSETVLI.cpp b/llvm/lib/Target/RISCV/RISCVInsertVSETVLI.cpp
index 9a513891b765d..78d64ea67324f 100644
--- a/llvm/lib/Target/RISCV/RISCVInsertVSETVLI.cpp
+++ b/llvm/lib/Target/RISCV/RISCVInsertVSETVLI.cpp
@@ -1119,25 +1119,26 @@ void RISCVInsertVSETVLI::insertVSETVLI(MachineBasicBlock &MBB,
     LIS->InsertMachineInstrInMaps(*MI);
     LiveInterval &LI = LIS->getInterval(AVLReg);
     SlotIndex SI = LIS->getInstructionIndex(*MI).getRegSlot();
+    const VNInfo *CurVNI = Info.getAVLVNInfo();
     // If the AVL value isn't live at MI, do a quick check to see if it's easily
     // extendable. Otherwise, we need to copy it.
-    if (LI.getVNInfoBefore(SI) != Info.getAVLVNInfo()) {
+    if (LI.getVNInfoBefore(SI) != CurVNI) {
       if (!LI.liveAt(SI) && LI.containsOneValue())
         LIS->extendToIndices(LI, SI);
       else {
         Register AVLCopyReg =
             MRI->createVirtualRegister(&RISCV::GPRNoX0RegClass);
+        MachineBasicBlock *MBB = LIS->getMBBFromIndex(CurVNI->def);
         MachineBasicBlock::iterator II;
-        if (Info.getAVLVNInfo()->isPHIDef())
-          II = LIS->getMBBFromIndex(Info.getAVLVNInfo()->def)->getFirstNonPHI();
+        if (CurVNI->isPHIDef())
+          II = MBB->getFirstNonPHI();
         else {
-          II = LIS->getInstructionFromIndex(Info.getAVLVNInfo()->def);
+          II = LIS->getInstructionFromIndex(CurVNI->def);
           II = std::next(II);
         }
         assert(II.isValid());
-        auto AVLCopy =
-            BuildMI(*II->getParent(), II, DL, TII->get(RISCV::COPY), AVLCopyReg)
-                .addReg(AVLReg);
+        auto AVLCopy = BuildMI(*MBB, II, DL, TII->get(RISCV::COPY), AVLCopyReg)
+                           .addReg(AVLReg);
         LIS->InsertMachineInstrInMaps(*AVLCopy);
         MI->getOperand(1).setReg(AVLCopyReg);
         LIS->createAndComputeVirtRegInterval(AVLCopyReg);
diff --git a/llvm/test/CodeGen/RISCV/rvv/vsetvli-insert-crossbb.mir b/llvm/test/CodeGen/RISCV/rvv/vsetvli-insert-crossbb.mir
index 140875c4b24ad..f6f0613d11502 100644
--- a/llvm/test/CodeGen/RISCV/rvv/vsetvli-insert-crossbb.mir
+++ b/llvm/test/CodeGen/RISCV/rvv/vsetvli-insert-crossbb.mir
@@ -142,6 +142,10 @@
     ret void
   }
 
+  define void @avl_is_last_instr() {
+    ret void
+  }
+
   declare i32 @llvm.vector.reduce.add.v4i32(<4 x i32>)
 
   declare <vscale x 1 x i64> @llvm.riscv.vadd.nxv1i64.nxv1i64.i64(<vscale x 1 x i64>, <vscale x 1 x i64>, <vscale x 1 x i64>, i64) #1
@@ -1099,3 +1103,66 @@ body: |
     renamable $v10m2 = PseudoVADD_VV_M2 undef renamable $v10m2, %v, %v, -1, 5, 0
     renamable $v8m2 = PseudoVADD_VV_M2 undef renamable $v8m2, killed renamable $v10m2, killed %v, %outvl:gprnox0, 5, 0
     PseudoRET implicit $v8m2
+...
+---
+name: avl_is_last_instr
+tracksRegLiveness: true
+body: |
+  ; CHECK-LABEL: name: avl_is_last_instr
+  ; CHECK: bb.0:
+  ; CHECK-NEXT:   successors: %bb.1(0x80000000)
+  ; CHECK-NEXT:   liveins: $x10
+  ; CHECK-NEXT: {{  $}}
+  ; CHECK-NEXT:   %base:gpr = COPY $x10
+  ; CHECK-NEXT:   %avl:gprnox0 = ADDI $x0, 256
+  ; CHECK-NEXT:   [[COPY:%[0-9]+]]:gprnox0 = COPY %avl
+  ; CHECK-NEXT: {{  $}}
+  ; CHECK-NEXT: bb.1:
+  ; CHECK-NEXT:   successors: %bb.2(0x80000000)
+  ; CHECK-NEXT: {{  $}}
+  ; CHECK-NEXT:   dead %prevl:gprnox0 = PseudoVSETVLI %avl, 195 /* e8, m8, ta, ma */, implicit-def $vl, implicit-def $vtype
+  ; CHECK-NEXT:   renamable $v8m8 = PseudoVMV_V_I_M8 undef renamable $v8m8, 0, $noreg, 3 /* e8 */, 0 /* tu, mu */, implicit $vl, implicit $vtype
+  ; CHECK-NEXT: {{  $}}
+  ; CHECK-NEXT: bb.2:
+  ; CHECK-NEXT:   successors: %bb.2(0x40000000), %bb.3(0x40000000)
+  ; CHECK-NEXT:   liveins: $v8m8
+  ; CHECK-NEXT: {{  $}}
+  ; CHECK-NEXT:   %vl:gprnox0 = PseudoVSETVLI %avl, 195 /* e8, m8, ta, ma */, implicit-def $vl, implicit-def $vtype
+  ; CHECK-NEXT:   renamable $v16m8 = PseudoVLE8_V_M8 undef renamable $v16m8, %base, $noreg, 3 /* e8 */, 1 /* ta, mu */, implicit $vl, implicit $vtype
+  ; CHECK-NEXT:   dead $x0 = PseudoVSETVLIX0X0 killed $x0, 131 /* e8, m8, tu, ma */, implicit-def $vl, implicit-def $vtype, implicit $vl
+  ; CHECK-NEXT:   renamable $v8m8 = PseudoVADD_VV_M8 killed renamable $v8m8, killed renamable $v16m8, renamable $v8m8, $noreg, 3 /* e8 */, 0 /* tu, mu */, implicit $vl, implicit $vtype
+  ; CHECK-NEXT:   %base:gpr = ADD %base, %vl
+  ; CHECK-NEXT:   %avl:gprnox0 = SUB %avl, %vl
+  ; CHECK-NEXT:   BNE %avl, $x0, %bb.2
+  ; CHECK-NEXT: {{  $}}
+  ; CHECK-NEXT: bb.3:
+  ; CHECK-NEXT:   liveins: $v8m8
+  ; CHECK-NEXT: {{  $}}
+  ; CHECK-NEXT:   dead $x0 = PseudoVSETIVLI 1, 195 /* e8, m8, ta, ma */, implicit-def $vl, implicit-def $vtype
+  ; CHECK-NEXT:   renamable $v16 = PseudoVMV_S_X undef renamable $v16, $x0, 1, 3 /* e8 */, implicit $vl, implicit $vtype
+  ; CHECK-NEXT:   dead $x0 = PseudoVSETVLI [[COPY]], 195 /* e8, m8, ta, ma */, implicit-def $vl, implicit-def $vtype
+  ; CHECK-NEXT:   renamable $v16 = PseudoVREDSUM_VS_M8_E8 killed renamable $v16, killed renamable $v8m8, renamable $v16, $noreg, 3 /* e8 */, 1 /* ta, mu */, implicit $vl, implicit $vtype
+  ; CHECK-NEXT:   dead %res:gpr = PseudoVMV_X_S killed renamable $v16, 3 /* e8 */, implicit $vtype
+  bb.0:
+    liveins: $x10
+    %base:gpr = COPY $x10
+    %avl:gprnox0 = ADDI $x0, 256
+
+  bb.1:
+    %prevl:gprnox0 = PseudoVSETVLI %avl:gprnox0, 195, implicit-def dead $vl, implicit-def dead $vtype
+    renamable $v8m8 = PseudoVMV_V_I_M8 undef renamable $v8m8, 0, %prevl:gprnox0, 3, 0
+
+  bb.2:
+    liveins: $v8m8
+    %vl:gprnox0 = PseudoVSETVLI %avl:gprnox0, 195, implicit-def dead $vl, implicit-def dead $vtype
+    renamable $v16m8 = PseudoVLE8_V_M8 undef renamable $v16m8, %base:gpr, %vl:gprnox0, 3, 1
+    renamable $v8m8 = PseudoVADD_VV_M8 killed renamable $v8m8, killed renamable $v16m8, renamable $v8m8, %vl:gprnox0, 3, 0
+    %base:gpr = ADD %base:gpr, %vl:gprnox0
+    %avl:gprnox0 = SUB %avl:gprnox0, %vl:gprnox0
+    BNE %avl:gprnox0, $x0, %bb.2
+
+  bb.3:
+    liveins: $v8m8
+    renamable $v16 = PseudoVMV_S_X undef renamable $v16, $x0, 1, 3
+    renamable $v16 = PseudoVREDSUM_VS_M8_E8 killed renamable $v16, killed renamable $v8m8, renamable $v16, %prevl:gprnox0, 3, 1
+    %res:gpr = PseudoVMV_X_S killed renamable $v16, 3

[wangpc-pp]

If it fixes a bug, then it is not NFC?

[lukel97]

Thanks, the fix makes sense to me

[mshockwave]

LGTM good catch. I vaguely remember we do account for AVL being last instruction in coalesceVSETVLI but seems like we missed this place.