This is an archive of the discontinued LLVM Phabricator instance.

[AMDGPU] Fix PreRARematerialize scheduler pass sinking subreg defs
ClosedPublic

Authored by vangthao on Mar 16 2022, 5:58 PM.

Details

Summary

When collecting trivially rematerializable defs, skip any subreg defs. We do not want to sink these.

Diff Detail

Event Timeline

vangthao created this revision.Mar 16 2022, 5:58 PM
Herald added a project: Restricted Project. · View Herald TranscriptMar 16 2022, 5:58 PM
vangthao requested review of this revision.Mar 16 2022, 5:58 PM
Herald added a project: Restricted Project. · View Herald TranscriptMar 16 2022, 5:58 PM

I thought we do not rematerialize subreg defs at all?

I thought we do not rematerialize subreg defs at all?

I incorrectly assumed when defining a subreg, there would be multiple defs for the other parts of the reg so checking hasOneDef() would be sufficient. However in this case, there was a single def of a subreg undef %23.sub1 without any def for sub0.

I thought we do not rematerialize subreg defs at all?

I incorrectly assumed when defining a subreg, there would be multiple defs for the other parts of the reg so checking hasOneDef() would be sufficient. However in this case, there was a single def of a subreg undef %23.sub1 without any def for sub0.

I would suggest to prohibit rematerialization of subreg defs completely instead.

vangthao updated this revision to Diff 416265.Mar 17 2022, 11:26 AM

Skip all defs containing subregs instead of just undefs.

Skip all defs containing subregs instead of just undefs.

Also retitle the change.

llvm/lib/Target/AMDGPU/GCNSchedStrategy.cpp
810

I would still pass the subreg here. It will be always 0, but not passing it 'spaghettifies' code.

vangthao updated this revision to Diff 416268.Mar 17 2022, 11:36 AM

Changed title and pass subreg to reMaterialze()

vangthao retitled this revision from [AMDGPU] Fix PreRARematerialize scheduler pass sinking undef instruction to [AMDGPU] Fix PreRARematerialize scheduler pass sinking subreg defs.Mar 17 2022, 11:37 AM
vangthao edited the summary of this revision. (Show Details)
This revision is now accepted and ready to land.Mar 17 2022, 11:38 AM
This revision was landed with ongoing or failed builds.Mar 17 2022, 11:40 AM
This revision was automatically updated to reflect the committed changes.
vangthao marked an inline comment as done.