This is an archive of the discontinued LLVM Phabricator instance.

[AMDGPU] fixed scheduler crash in gfx908
ClosedPublic

Authored by rampitec on Jul 14 2019, 11:46 AM.

Details

Summary

For some reason scheduler can send down an SUnit without an
instruction.

Diff Detail

Repository
rL LLVM

Event Timeline

rampitec created this revision.Jul 14 2019, 11:46 AM
rampitec updated this revision to Diff 209744.Jul 14 2019, 11:59 AM
kerbowa added inline comments.
lib/Target/AMDGPU/AMDGPUSubtarget.cpp
769 ↗(On Diff #209744)

There is SUnit::isInstr() also

rampitec marked an inline comment as done.Jul 14 2019, 12:45 PM
rampitec added inline comments.
lib/Target/AMDGPU/AMDGPUSubtarget.cpp
769 ↗(On Diff #209744)

It's actually very strange to see anything but MI in the PostRA scheduler.

kerbowa added inline comments.Jul 14 2019, 12:48 PM
lib/Target/AMDGPU/AMDGPUSubtarget.cpp
769 ↗(On Diff #209744)

Yeah I just meant as a null check.

rampitec marked an inline comment as done.Jul 14 2019, 1:35 PM
rampitec added inline comments.
lib/Target/AMDGPU/AMDGPUSubtarget.cpp
769 ↗(On Diff #209744)

Same thing, but a little bit more source ;)

rampitec updated this revision to Diff 209754.Jul 14 2019, 8:09 PM
rampitec edited the summary of this revision. (Show Details)

Added test.

Where does the null come from exactly?

Where does the null come from exactly?

It is in the list of all dag nodes, somehow dag builder put it there. I suspect that this is for scheduling barrier. The node is neither MI nor SDNode (obviously), just plain zero initialized.

arsenm accepted this revision.Jul 15 2019, 7:56 AM

I vaguely remember this being how nops are represented, and one does seem to have been inserted

This revision is now accepted and ready to land.Jul 15 2019, 7:56 AM
This revision was automatically updated to reflect the committed changes.
Herald added a project: Restricted Project. · View Herald TranscriptJul 15 2019, 8:34 AM