This is an archive of the discontinued LLVM Phabricator instance.

[AMDGPU] Fix for the lost CarryOut/CarryIn register operands in S_ADD/SUB_CO_PSEUDO.
ClosedPublic

Authored by alex-t on May 18 2020, 1:57 PM.

Details

Summary

This fixes the 5b898bddff51 bug when the carry-in and carry-out registers became lost in lowering S_ADD/SUB_CO_PSEUDO.

Diff Detail

Event Timeline

alex-t created this revision.May 18 2020, 1:57 PM
Herald added a project: Restricted Project. · View Herald TranscriptMay 18 2020, 1:57 PM
arsenm requested changes to this revision.May 18 2020, 2:10 PM

Needs test

This revision now requires changes to proceed.May 18 2020, 2:10 PM
arsenm added inline comments.May 18 2020, 2:11 PM
llvm/lib/Target/AMDGPU/SIInstrInfo.cpp
5183

I'm pretty sure there's a constrain to regclass helper that will do this for you

arsenm added inline comments.May 18 2020, 2:13 PM
llvm/lib/Target/AMDGPU/SIInstrInfo.cpp
5183

This should try MRI.constrainRegClass rather than enforcing class equality

alex-t updated this revision to Diff 265395.May 20 2020, 5:41 PM

Explicit register class comparison changed to constrain reg class. Test added.

alex-t marked 2 inline comments as done.May 20 2020, 5:43 PM
arsenm added inline comments.May 21 2020, 7:55 AM
llvm/test/CodeGen/AMDGPU/s_add_co_pseudo_lowering.mir
5 ↗(On Diff #265395)

Doesn't test for the copy? I would also probably just use update_mir_test_checks here

8 ↗(On Diff #265395)

Better test name?

10 ↗(On Diff #265395)

Should be able to drop the register section

alex-t updated this revision to Diff 265568.May 21 2020, 12:05 PM

test reworked.

alex-t marked 3 inline comments as done.May 21 2020, 12:06 PM

Ping again. Could you please take a look?

arsenm accepted this revision.May 27 2020, 6:17 AM
This revision is now accepted and ready to land.May 27 2020, 6:17 AM
This revision was automatically updated to reflect the committed changes.