This is an archive of the discontinued LLVM Phabricator instance.

[AMDGPU] Remove SI_MASK_BRANCH
ClosedPublic

Authored by ruiling on Feb 26 2021, 5:26 AM.

Details

Summary

This is already deprecated, so remove code working on this.
Also update the tests by using S_CBRANCH_EXECZ instead of SI_MASK_BRANCH.
The tests to specifically test the insert-skip feature are not valid
anymore, so just remove them.

Diff Detail

Event Timeline

ruiling created this revision.Feb 26 2021, 5:26 AM
ruiling requested review of this revision.Feb 26 2021, 5:26 AM
Herald added a project: Restricted Project. · View Herald TranscriptFeb 26 2021, 5:26 AM
arsenm added inline comments.Feb 26 2021, 5:44 AM
llvm/lib/Target/AMDGPU/SIInsertSkips.cpp
213

If SIInsertSkips is no longer inserting skips, it needs a new name

ruiling updated this revision to Diff 326678.Feb 26 2021, 6:28 AM

I find it is not a good idea to remove the tests. So I adjust the lit-test to make sure RemoveShortExecBranches works correctly.

ruiling added inline comments.Feb 26 2021, 6:35 AM
llvm/lib/Target/AMDGPU/SIInsertSkips.cpp
213

I don't have good idea on that. I am happy if some one could came up with a good name. And I believe that should be in a separate patch. This patch just aims to remove "SI_MASK_BRANCH".

I agree with renaming being a follow up patch.
I wonder if this entire pass could be merged with SIRemoveShortExecBranches and the result renamed SILateBranchLowering?

ping, anybody would like to take a look?

foad accepted this revision.Mar 4 2021, 2:32 AM
foad added a subscriber: foad.

LGTM, with renaming or moving the pass as a follow-up.

This revision is now accepted and ready to land.Mar 4 2021, 2:32 AM
This revision was automatically updated to reflect the committed changes.