This is an archive of the discontinued LLVM Phabricator instance.

[GlobalIsel][AMDGPU] Changing legalize rule for G_{UADDO|UADDE|USUBO|USUBE|SADDE|SSUBE}
ClosedPublic

Authored by yassingh on Nov 14 2022, 3:39 AM.

Details

Summary

Generic add and sub with carry are now legalized in a way to explicitly calculate carry/borrow output. i.e
%6:_(s64), %7:_(s1) = G_UADDO %0, %1
becomes,
%13:_(s32), %14:_(s1) = G_UADDO %2, %4
%15:_(s32), %16:_(s1) = G_UADDE %3, %5, %14
%6:_(s64) = G_MERGE_VALUES %13(s32), %15(s32)
%7:_(s1) = G_ICMP intpred(ult), %6(s64), %1

Here G_MERGE and G_ICMP instructions are redundant for recalculating carry output. (Similar case for sub with borrow)
This change fix this.

Diff Detail

Event Timeline

yassingh created this revision.Nov 14 2022, 3:39 AM
Herald added a project: Restricted Project. · View Herald TranscriptNov 14 2022, 3:39 AM
yassingh requested review of this revision.Nov 14 2022, 3:39 AM
Herald added a project: Restricted Project. · View Herald TranscriptNov 14 2022, 3:39 AM
Petar.Avramovic added inline comments.
llvm/test/CodeGen/AMDGPU/GlobalISel/legalize-sadde.mir
2

I think that -global-isel-abort=0 in no longer required since test should compile now with the addition of narrow scalar of s64 to s32.
Can you check the other tests?

yassingh added inline comments.Nov 14 2022, 7:16 AM
llvm/test/CodeGen/AMDGPU/GlobalISel/legalize-sadde.mir
2

Yes the tests compile without -global-isel-abort=0, updating both tests containing it.

arsenm accepted this revision.Nov 14 2022, 7:19 AM

LGTM with the abort=0 removed

This revision is now accepted and ready to land.Nov 14 2022, 7:19 AM
yassingh updated this revision to Diff 475142.Nov 14 2022, 7:22 AM

Removed -global-isel-abort=0

arsenm accepted this revision.Nov 14 2022, 7:41 AM
arsenm added inline comments.
llvm/test/CodeGen/AMDGPU/GlobalISel/legalize-sadde.mir
2

Can also drop -O0 here

yassingh updated this revision to Diff 475173.Nov 14 2022, 9:19 AM

Removed -O0 from run lines of all tests. legalize-uadde.mir, legalize-uaddo.mir, legalize-usube.mir, legalize-usubo.mir had to be updated

arsenm accepted this revision.Nov 14 2022, 9:38 AM
This revision was landed with ongoing or failed builds.Nov 14 2022, 10:13 AM
This revision was automatically updated to reflect the committed changes.