Skip to content

Conversation

@slinder1
Copy link

@slinder1 slinder1 commented Nov 6, 2025

[AMDGPU] Handle empty-except-for-DI regions in PreRARematerialize

The existing check for this case only comes after a derefence of what
can be an iterator sentinel (leading to an assert).

This may not be purely NFC in that it also avoids queuing the
effectively-empty region for rescheduling, but AFAICT this should be
purely an optimization.

Testing this seems difficult, as the high-level scheduler avoids
scheduling these "empty" regions. This means a reproducer has to depend
on behavior of the scheduler passes before PreRARematStage in order to
craft a region which triggers the bug.

Since this is a release blocker I am posting a PR now, as both Shore
Shen and I have manually verified that this resolves the particular
crash from SWDEV-564142 but I am still working on making a reasonable
test.

The existing check for this case only comes after a derefence of what
can be an iterator sentinel (leading to an assert).

This may not be purely NFC in that it also avoids queuing the
effectively-empty region for rescheduling, but AFAICT this should be
purely an optimization.

Testing this seems difficult, as the high-level scheduler avoids
scheduling these "empty" regions. This means a reproducer has to depend
on behavior of the scheduler passes before PreRARematStage in order to
craft a region which triggers the bug.

Since this is a release blocker I am posting a PR now, as both Shore
Shen and I have manually verified that this resolves the particular
crash from SWDEV-564142 but I am still working on making a reasonable
test.
@slinder1 slinder1 changed the base branch from main to amd-staging November 6, 2025 22:20
@slinder1
Copy link
Author

slinder1 commented Nov 6, 2025

I will also post upstream, but want to put in a bit more effort into making a reasonable test first.

@z1-cciauto
Copy link
Collaborator

Copy link
Collaborator

@kzhuravl kzhuravl left a comment

Choose a reason for hiding this comment

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

If it takes a long time to come up with the test, suggesting to file a ticket.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants