This is an archive of the discontinued LLVM Phabricator instance.

Revert D109159 "[amdgpu] Enable selection of `s_cselect_b64`."
AbandonedPublic

Authored by david-salinas on Dec 17 2021, 11:39 AM.

Details

Summary

This reverts commit 640beb38e7710b939b3cfb3f4c54accc694b1d30.

That commit caused performance degradtion in Quicksilver test QS:sGPU and a functional test failure in (rocPRIM rocprim.device_segmented_radix_sort).
Reverting until we have a better solution to s_cselect_b64 codegen cleanup

Change-Id: Ibf8e397df94001f248fba609f072088a46abae08

Diff Detail

Event Timeline

david-salinas created this revision.Dec 17 2021, 11:39 AM
david-salinas requested review of this revision.Dec 17 2021, 11:39 AM
Herald added a project: Restricted Project. · View Herald TranscriptDec 17 2021, 11:39 AM

Fix failing LIT tests

kzhuravl accepted this revision.Dec 20 2021, 9:33 AM
This revision is now accepted and ready to land.Dec 20 2021, 9:33 AM
MaskRay retitled this revision from Revert "[amdgpu] Enable selection of `s_cselect_b64`." to Revert D109159 "[amdgpu] Enable selection of `s_cselect_b64`.".Jan 4 2022, 2:23 PM
MaskRay added a subscriber: MaskRay.

"Revert D109159" makes sure when the revert lands, the commit is associated with D109159 and we know that D109159 has been reverted.

This revision was landed with ongoing or failed builds.Jan 5 2022, 9:58 AM
This revision was automatically updated to reflect the committed changes.
Herald added projects: Restricted Project, Restricted Project, Restricted Project, Restricted Project, Restricted Project, Restricted Project. · View Herald Transcript
Herald added a reviewer: Restricted Project. · View Herald Transcript
Herald added a reviewer: Restricted Project. · View Herald Transcript

I believe you landed the wrong commit. rG859ebca744e634dcc89a2294ffa41574f947bd62 looks like the previous versions of this diff, where lots of extraneous changes were made. Could you please revert and land the correct commit? Feel free to ask for assistance if you need it :)

MaskRay reopened this revision.EditedJan 5 2022, 10:02 AM
MaskRay added a subscriber: ldionne.

(CC @ldionne @smeenai)

The revert 859ebca744e634dcc89a2294ffa41574f947bd62 included many unintended changes.

Did you notice the comment https://reviews.llvm.org/D116547#3218089 that the change is very incorrect? It re-added many files which were removed a while ago.

Before reverting a change, please make sure you have rebased to the top of origin/main.

MaskRay requested changes to this revision.Jan 5 2022, 10:03 AM
This revision now requires changes to proceed.Jan 5 2022, 10:03 AM
thakis added a subscriber: thakis.Jan 5 2022, 10:11 AM

Not sure what happened here but this change added back a whole bunch of old code. I reverted this in 085f078307bac264301b07f6e47e2a04e90a6f1d . Please carefully check git diff origin/main before committing next time :)

ormris removed a subscriber: ormris.Jan 18 2022, 10:16 AM
int3 resigned from this revision.Jan 31 2022, 1:08 PM
MaskRay added a comment.EditedJan 31 2022, 2:45 PM

Please abandon this revision. If you want to revert the AMDGPU patch, create a clean revert.

david-salinas abandoned this revision.Jan 31 2022, 5:31 PM