This is an archive of the discontinued LLVM Phabricator instance.

[AMDGPU] Strengthen export cluster ordering
ClosedPublic

Authored by critson on May 13 2020, 5:52 AM.

Details

Summary

When removing barrier edges on exports then dependencies need to
be propagated.

Diff Detail

Unit TestsFailed

Event Timeline

critson created this revision.May 13 2020, 5:52 AM
Herald added a project: Restricted Project. · View Herald TranscriptMay 13 2020, 5:52 AM
foad accepted this revision.May 13 2020, 6:22 AM

This is OK, and I can confirm it fixes some Vulkan CTS failures we were seeing.

For bonus marks:

  1. removeExportDependencies iterates the preds of every node. I think it could be a lot faster if it only iterated the preds and succs of export nodes.
  2. I still wonder if we could get the barriers right in the first place, perhaps by pretending that each kind of export instruction writes to a fake address space and teaching alias analysis that those address spaces are disjoint.
This revision is now accepted and ready to land.May 13 2020, 6:22 AM
critson updated this revision to Diff 263693.May 13 2020, 6:29 AM

Add test case.

critson updated this revision to Diff 263699.May 13 2020, 6:49 AM

Address (1) by only processing on export nodes.
Since there is no way to remove a successor directly, use the
existing removeExportDependencies code and simply apply it to
successors.

foad accepted this revision.May 13 2020, 7:08 AM

Address (1) by only processing on export nodes.
Since there is no way to remove a successor directly, use the
existing removeExportDependencies code and simply apply it to
successors.

Thanks. For a future cleanup it should be pretty easy to implement SUnit::removeSucc.

This revision was automatically updated to reflect the committed changes.