Skip to content

Crash in SIInstrInfo::isSafeToSink #89200

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.

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

Closed
jayfoad opened this issue Apr 18, 2024 · 7 comments · Fixed by #89224
Closed

Crash in SIInstrInfo::isSafeToSink #89200

jayfoad opened this issue Apr 18, 2024 · 7 comments · Fixed by #89224

Comments

@jayfoad
Copy link
Contributor

jayfoad commented Apr 18, 2024

New test case test/CodeGen/AMDGPU/machine-sink-crash.mir:

# RUN: llc -mtriple=amdgcn -mcpu=gfx1030 -run-pass=machine-sink %s | FileCheck %s

---
name: f
body: |
  bb.0:
    %81:sreg_32_xm0_xexec = IMPLICIT_DEF
    S_CBRANCH_SCC1 %bb.2, implicit undef $scc
    S_BRANCH %bb.1
  
  bb.1:
    S_BRANCH %bb.3
  
  bb.2:
    S_BRANCH %bb.6
  
  bb.3:
    successors: %bb.4, %bb.5
  
    %10:sreg_32_xm0_xexec = SI_IF undef %81, %bb.5, implicit-def $exec, implicit-def $scc, implicit $exec
    S_BRANCH %bb.4
  
  bb.4:
  
  bb.5:
    successors: %bb.2(0x00000000), %bb.3(0x80000000)
  
    %61:sreg_32 = COPY undef %10
    S_CBRANCH_VCCNZ %bb.3, implicit undef $vcc
    S_BRANCH %bb.2
  
  bb.6:
    ADJCALLSTACKUP 0, 0, implicit-def $scc
    ADJCALLSTACKDOWN 0, 0, implicit-def $scc
...

I get:

llc: lib/Target/AMDGPU/SIInstrInfo.cpp:220: virtual bool llvm::SIInstrInfo::isSafeToSink(llvm::MachineInstr &, llvm::MachineBasicBlock *, llvm::MachineCycleInfo *) const: Assertion `ExitBlocks[0]->getSinglePredecessor()' failed.

SIInstrInfo::isSafeToSink was introduced by #67456.

Cc @sameerds

@llvmbot
Copy link
Member

llvmbot commented Apr 18, 2024

@llvm/issue-subscribers-backend-amdgpu

Author: Jay Foad (jayfoad)

New test case `test/CodeGen/AMDGPU/machine-sink-crash.mir`: ``` # RUN: llc -mtriple=amdgcn -mcpu=gfx1030 -run-pass=machine-sink %s | FileCheck %s

name: f
body: |
bb.0:
%81:sreg_32_xm0_xexec = IMPLICIT_DEF
S_CBRANCH_SCC1 %bb.2, implicit undef $scc
S_BRANCH %bb.1

bb.1:
S_BRANCH %bb.3

bb.2:
S_BRANCH %bb.6

bb.3:
successors: %bb.4, %bb.5

%10:sreg_32_xm0_xexec = SI_IF undef %81, %bb.5, implicit-def $exec, implicit-def $scc, implicit $exec
S_BRANCH %bb.4

bb.4:

bb.5:
successors: %bb.2(0x00000000), %bb.3(0x80000000)

%61:sreg_32 = COPY undef %10
S_CBRANCH_VCCNZ %bb.3, implicit undef $vcc
S_BRANCH %bb.2

bb.6:
ADJCALLSTACKUP 0, 0, implicit-def $scc
ADJCALLSTACKDOWN 0, 0, implicit-def $scc
...

I get:

llc: lib/Target/AMDGPU/SIInstrInfo.cpp:220: virtual bool llvm::SIInstrInfo::isSafeToSink(llvm::MachineInstr &, llvm::MachineBasicBlock *, llvm::MachineCycleInfo *) const: Assertion `ExitBlocks[0]->getSinglePredecessor()' failed.


`SIInstrInfo::isSafeToSink` was introduced by #<!-- -->67456.

Cc @<!-- -->sameerds
</details>

@petar-avramovic
Copy link
Collaborator

I am aware of this, after structurize-cfg, cycle exit block should have exactly one predecessor, but again it could be removed in early-tailduplication I don't know if that block deletion is safe but again since it is late in the pipeline it is probably fine?
The quick fix is to search for predecessor that is in the cycle.

@petar-avramovic
Copy link
Collaborator

Can you also attach llvm-ir just to check if block was deleted in early-tailduplication

@jayfoad
Copy link
Contributor Author

jayfoad commented Apr 18, 2024

Here's the IR I used:

source_filename = "reduced.ll"
target datalayout = "e-p:64:64-p1:64:64-p2:32:32-p3:32:32-p4:64:64-p5:32:32-p6:32:32-p7:160:256:256:32-p8:128:128-p9:192:256:256:32-i64:64-v16:16-v24:32-v32:32-v48:64-v96:128-v192:256-v256:256-v512:512-v1024:1024-v2048:2048-n32:64-S32-A5-G1-ni:7:8:9"
target triple = "amdgcn--amdpal"

define amdgpu_ps void @_amdgpu_ps_main(i1 %arg) {
bb:
  %i = call i32 @llvm.amdgcn.s.buffer.load.i32(<4 x i32> zeroinitializer, i32 0, i32 0)
  %i1 = icmp slt i32 %i, 0
  br i1 %i1, label %bb2, label %bb12

bb2:                                              ; preds = %bb6, %bb
  %i3 = phi i1 [ %i9, %bb6 ], [ false, %bb ]
  %i4 = phi i32 [ %i10, %bb6 ], [ 0, %bb ]
  br i1 %arg, label %bb5, label %bb6

bb5:                                              ; preds = %bb2
  br label %bb6

bb6:                                              ; preds = %bb5, %bb2
  %i7 = phi i32 [ 0, %bb2 ], [ 1, %bb5 ]
  %i8 = icmp ne i32 %i7, 0
  %i9 = select i1 %i8, i1 %i3, i1 false
  %i10 = or i32 %i4, 1
  %i11 = icmp slt i32 %i4, 0
  br i1 %i11, label %bb2, label %bb12

bb12:                                             ; preds = %bb6, %bb
  %i13 = phi i1 [ false, %bb ], [ %i9, %bb6 ]
  %i14 = select i1 %i13, float 0.000000e+00, float 1.000000e+00
  %i15 = insertelement <4 x float> zeroinitializer, float %i14, i64 0
  call amdgpu_gfx addrspace(4) void null(<4 x float> %i15, i32 0)
  unreachable
}

; Function Attrs: nocallback nofree nosync nounwind willreturn memory(none)
declare i32 @llvm.amdgcn.s.buffer.load.i32(<4 x i32>, i32, i32 immarg) #0

attributes #0 = { nocallback nofree nosync nounwind willreturn memory(none) }

@petar-avramovic
Copy link
Collaborator

early-tail

it was early tail duplication, it deleted bb.7.Flow
I already have fix for this, will open a pr

@sameerds
Copy link
Contributor

@ssahasra

@jayfoad
Copy link
Contributor Author

jayfoad commented Apr 22, 2024

@ssahasra

Sorry. The problem is that when I type @ and then sameer, this id does not appear in the list of suggested completions.

petar-avramovic added a commit that referenced this issue May 10, 2024
isSafeToSink needs to check if machine cycle has divergent exit branch
but first it needs the MBB that contains cycle exit branch.
Early-tailduplication can delete exit block created by structurize-cfg
so there is still exactly one cycle exit block but the new cycle exit
block can have multiple predecessors.
Simplify search for MBBs that contain cycle exit branch by introducing
helper method getExitingBlocks in GenericCycle.

Fixes #89200
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants