Page MenuHomePhabricator

AMDGPU: Don't ignore carry out user when expanding add_co_pseudo
ClosedPublic

Authored by arsenm on Jun 19 2020, 5:42 PM.

Details

Reviewers
rampitec
alex-t
Summary

This was resulting in a missing vreg def in the use select
instruction.

copyPhysReg changes lifted out of
4067de569f119a81419fbf2e79d5f3307dfdda5b, which was reverted earlier
today.

The output of the pseudo doesn't make sense, since it really shouldn't
have the vreg output in the first place, and instead an implicit scc
def to match the real scalar behavior.

We could have easier to understand tests if we selected scalar
versions of the [us]{add|sub}.with.overflow intrinsics.

This does still end up producing vector code in the end, since it gets
moved later.

Diff Detail

Event Timeline

arsenm created this revision.Jun 19 2020, 5:42 PM
Herald added a project: Restricted Project. · View Herald TranscriptJun 19 2020, 5:42 PM
alex-t added inline comments.Jun 23 2020, 3:26 AM
llvm/lib/Target/AMDGPU/SIInstrInfo.cpp
640

https://reviews.llvm.org/D82194 contains same change.
I'd like it to land first. It also may affect the test.

arsenm marked an inline comment as done.Jun 23 2020, 7:59 AM
arsenm added inline comments.
llvm/lib/Target/AMDGPU/SIInstrInfo.cpp
640

This could be split out into an independent patch. This also was from the already committed patch which was reverted.

(I'm also not sure if situations like this should be inserting an and with exec; probably not from this context though)

rampitec accepted this revision.Jul 6 2020, 11:01 AM
This revision is now accepted and ready to land.Jul 6 2020, 11:01 AM