Skip to content

[RISCV] Fix crash when trying to remove segment #146524

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 5 commits into from
Jul 3, 2025
Merged

[RISCV] Fix crash when trying to remove segment #146524

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
a809af5
[RISCV] Fix crash when trying to remove segment
sc-clulzze Jul 1, 2025
c1b6e12
Reduced and moved vleff-crash.ll to vsetvli-insert.ll
sc-clulzze Jul 1, 2025
ae439a6
Added MIR test
sc-clulzze Jul 2, 2025
5e89c26
Addressed comments
sc-clulzze Jul 3, 2025
795ac6b
Apply suggested changes
sc-clulzze Jul 3, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 3 additions & 2 deletions llvm/lib/Target/RISCV/RISCVInsertVSETVLI.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1768,8 +1768,9 @@ void RISCVInsertVSETVLI::insertReadVL(MachineBasicBlock &MBB) {
SlotIndex NewDefSI =
LIS->InsertMachineInstrInMaps(*ReadVLMI).getRegSlot();
LiveInterval &DefLI = LIS->getInterval(VLOutput);
VNInfo *DefVNI = DefLI.getVNInfoAt(DefLI.beginIndex());
DefLI.removeSegment(DefLI.beginIndex(), NewDefSI);
const auto *DefSeg = DefLI.getSegmentContaining(NewDefSI);
Copy link
Contributor

@lukel97 lukel97 Jul 2, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit

Suggested change
const auto *DefSeg = DefLI.getSegmentContaining(NewDefSI);
LiveRange::Segment *DefSeg = DefLI.getSegmentContaining(NewDefSI);

Copy link
Contributor Author

@sc-clulzze sc-clulzze Jul 3, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Addressed

VNInfo *DefVNI = DefLI.getVNInfoAt(DefSeg->start);
DefLI.removeSegment(DefSeg->start, NewDefSI);
DefVNI->def = NewDefSI;
}
}
Expand Down
32 changes: 32 additions & 0 deletions llvm/test/CodeGen/RISCV/rvv/vsetvli-insert.ll
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -722,3 +722,35 @@ define i64 @avl_undef2() {
%1 = tail call i64 @llvm.riscv.vsetvli(i64 poison, i64 2, i64 7)
ret i64 %1
}

define i64 @vsetvli_vleff() {
; CHECK-LABEL: vsetvli_vleff:
; CHECK: # %bb.0: # %entry
; CHECK-NEXT: vsetvli a1, zero, e16, m1, ta, ma
; CHECK-NEXT: vmv.v.i v8, 0
; CHECK-NEXT: .LBB37_1: # %while.body
; CHECK-NEXT: # =>This Inner Loop Header: Depth=1
; CHECK-NEXT: vsetivli zero, 0, e16, m1, tu, ma
; CHECK-NEXT: vmv1r.v v9, v8
; CHECK-NEXT: vle16ff.v v9, (zero)
; CHECK-NEXT: csrr a0, vl
; CHECK-NEXT: beqz a0, .LBB37_1
; CHECK-NEXT: # %bb.2: # %while.end
; CHECK-NEXT: li a0, 0
; CHECK-NEXT: ret
entry:
br label %while.cond

while.cond:
%new_vl.0 = phi i64 [ 0, %entry ], [ %1, %while.body ]
%cmp = icmp eq i64 %new_vl.0, 0
br i1 %cmp, label %while.body, label %while.end

while.body:
%0 = tail call { <vscale x 4 x i16>, i64 } @llvm.riscv.vleff.nxv4i16.i64(<vscale x 4 x i16> zeroinitializer, ptr null, i64 0)
Copy link
Contributor

@lukel97 lukel97 Jul 2, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you pass in a pointer + evl operand to this test just so the vleff doesn't get optimised away?

Copy link
Contributor Author

@sc-clulzze sc-clulzze Jul 3, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Addressed

%1 = extractvalue { <vscale x 4 x i16>, i64 } %0, 1
br label %while.cond

while.end:
ret i64 0
}
90 changes: 90 additions & 0 deletions llvm/test/CodeGen/RISCV/rvv/vsetvli-insert.mir
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,24 @@
ret <vscale x 1 x i64> %b
}

define i64 @vsetvli_vleff() {
entry:
br label %while.cond

while.cond: ; preds = %while.body, %entry
%new_vl.0 = phi i64 [ 0, %entry ], [ %1, %while.body ]
%cmp = icmp eq i64 %new_vl.0, 0
br i1 %cmp, label %while.body, label %while.end

while.body: ; preds = %while.cond
%0 = tail call { <vscale x 4 x i16>, i64 } @llvm.riscv.vleff.nxv4i16.i64(<vscale x 4 x i16> zeroinitializer, ptr null, i64 0)
%1 = extractvalue { <vscale x 4 x i16>, i64 } %0, 1
br label %while.cond

while.end: ; preds = %while.cond
ret i64 0
}

define void @vmv_v_i_different_lmuls() {
ret void
}
Expand Down Expand Up @@ -622,3 +640,75 @@ body: |
dead $x0 = PseudoVSETIVLI 1, 208, implicit-def $vl, implicit-def $vtype
%v:vr = COPY $v8, implicit $vtype
%x = PseudoVSETVLI %x, 208, implicit-def $vl, implicit-def $vtype
...
---
name: vsetvli_vleff
alignment: 4
tracksRegLiveness: true
registers:
- { id: 0, class: gpr, preferred-register: '' }
- { id: 1, class: gpr, preferred-register: '' }
- { id: 2, class: gpr, preferred-register: '' }
- { id: 3, class: gpr, preferred-register: '' }
- { id: 4, class: gpr, preferred-register: '' }
- { id: 5, class: vr, preferred-register: '%7' }
- { id: 6, class: gpr, preferred-register: '' }
- { id: 7, class: vr, preferred-register: '%5' }
- { id: 8, class: vr, preferred-register: '' }
- { id: 9, class: gpr, preferred-register: '' }
frameInfo:
maxAlignment: 1
machineFunctionInfo: {}
body: |
; CHECK-LABEL: name: vsetvli_vleff
; CHECK: bb.0.entry:
; CHECK-NEXT: successors: %bb.1(0x80000000)
; CHECK-NEXT: {{ $}}
; CHECK-NEXT: [[COPY:%[0-9]+]]:gpr = COPY $x0
; CHECK-NEXT: dead [[PseudoVSETVLIX0_:%[0-9]+]]:gprnox0 = PseudoVSETVLIX0 killed $x0, 200 /* e16, m1, ta, ma */, implicit-def $vl, implicit-def $vtype
; CHECK-NEXT: renamable $v8 = PseudoVMV_V_I_M1 undef renamable $v8, 0, -1, 4 /* e16 */, 0 /* tu, mu */, implicit $vl, implicit $vtype
; CHECK-NEXT: {{ $}}
; CHECK-NEXT: bb.1.while.cond:
; CHECK-NEXT: successors: %bb.2(0x7c000000), %bb.3(0x04000000)
; CHECK-NEXT: liveins: $v8
; CHECK-NEXT: {{ $}}
; CHECK-NEXT: BNE [[COPY]], $x0, %bb.3
; CHECK-NEXT: PseudoBR %bb.2
; CHECK-NEXT: {{ $}}
; CHECK-NEXT: bb.2.while.body:
; CHECK-NEXT: successors: %bb.1(0x80000000)
; CHECK-NEXT: liveins: $v8
; CHECK-NEXT: {{ $}}
; CHECK-NEXT: $x0 = PseudoVSETIVLI 0, 136 /* e16, m1, tu, ma */, implicit-def $vl, implicit-def $vtype
; CHECK-NEXT: renamable $v9 = COPY renamable $v8, implicit $vtype
; CHECK-NEXT: dead renamable $v9, $x0 = PseudoVLE16FF_V_M1 killed renamable $v9, $x0, 0, 4 /* e16 */, 2 /* tu, ma */, implicit-def dead $vl, implicit $vl, implicit $vtype :: (load unknown-size from `ptr null`, align 2)
; CHECK-NEXT: [[COPY:%[0-9]+]]:gpr = PseudoReadVL implicit $vl
; CHECK-NEXT: PseudoBR %bb.1
; CHECK-NEXT: {{ $}}
; CHECK-NEXT: bb.3.while.end:
; CHECK-NEXT: $x10 = COPY $x0
; CHECK-NEXT: PseudoRET implicit killed $x10
bb.0.entry:
successors: %bb.1(0x80000000)

%9:gpr = COPY $x0
Copy link
Contributor

@lukel97 lukel97 Jul 3, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
%9:gpr = COPY $x0
%vl:gpr = COPY $x0

Copy link
Contributor Author

@sc-clulzze sc-clulzze Jul 3, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Addressed

renamable $v8 = PseudoVMV_V_I_M1 undef renamable $v8, 0, -1, 4 /* e16 */, 0 /* tu, mu */

bb.1.while.cond:
successors: %bb.2(0x7c000000), %bb.3(0x04000000)
liveins: $v8

BNE %9, $x0, %bb.3
PseudoBR %bb.2

bb.2.while.body:
successors: %bb.1(0x80000000)
liveins: $v8

renamable $v9 = COPY renamable $v8
dead renamable $v9, %9:gpr = PseudoVLE16FF_V_M1 killed renamable $v9, $x0, 0, 4 /* e16 */, 2 /* tu, ma */, implicit-def dead $vl :: (load unknown-size from `ptr null`, align 2)
PseudoBR %bb.1

bb.3.while.end:
$x10 = COPY $x0
PseudoRET implicit killed $x10
Copy link
Contributor

@lukel97 lukel97 Jul 2, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this can be made more minimal, something like this should reproduce the crash?

---
name: vsetvli_vleff
tracksRegLiveness: true
body: |
  bb.0:
    PseudoBR %bb.2
  bb.1:
    $x8 = COPY %vl
    PseudoRET implicit $x8
  bb.2:
    $noreg, %vl:gpr = PseudoVLE16FF_V_M1 $noreg, $noreg, 0, 4 /* e16 */, 2 /* tu, ma */
    PseudoBR %bb.1

Copy link
Contributor Author

@sc-clulzze sc-clulzze Jul 3, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Crash does reproduce on this MIR, but you can write an exact IR for it since bb.0 is not a predecessor of bb.1. So I've modified it a little bit

Copy link
Contributor

@lukel97 lukel97 Jul 3, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh you can just leave the IR as "ret void" which lets you write whatever for the MIR