This is an archive of the discontinued LLVM Phabricator instance.

[AMDGPU] Fix typo in implicit operand lists
ClosedPublic

Authored by foad on Apr 21 2021, 7:55 AM.

Details

Summary

Several tests had a typo where they mentioned sgpr17 twice instead of
sgpr17 and sgpr27. This had a significant effect on the
"scavenge_sgpr_pei_no_sgprs" tests because there was actually an sgpr
available, namely sgpr27.

Diff Detail

Event Timeline

foad created this revision.Apr 21 2021, 7:55 AM
foad requested review of this revision.Apr 21 2021, 7:55 AM
Herald added a project: Restricted Project. · View Herald TranscriptApr 21 2021, 7:55 AM
foad added a comment.Apr 21 2021, 7:56 AM

Could someone please check that the tests are now testing what they are supposed to test? I am not very familiar with them, I just fixed the typo and re-auto-generated the checks.

arsenm added inline comments.Apr 21 2021, 8:52 AM
llvm/test/CodeGen/AMDGPU/pei-scavenge-sgpr-carry-out.mir
35

Why is this use undef, and why is it saving the VGPR just saved to memory?

Wow, how did you find that? The test changes look good to me.

llvm/test/CodeGen/AMDGPU/pei-scavenge-sgpr-carry-out.mir
35

v2 is used as an SGPR spill register, so all lanes of v2 are first saved to memory (killing v2, as we do not care about it anymore), then s33 is saved into the first lane of v2.

foad added a comment.Apr 21 2021, 9:45 AM

Wow, how did you find that?

I was looking for duplicate implicit use operands: egrep -r 'implicit (killed )?([^, ]+), .*implicit (killed )?\2\>' test/CodeGen/AMDGPU/

sebastian-ne accepted this revision.Apr 23 2021, 4:30 AM
This revision is now accepted and ready to land.Apr 23 2021, 4:30 AM
This revision was landed with ongoing or failed builds.Apr 23 2021, 7:44 AM
This revision was automatically updated to reflect the committed changes.