Page MenuHomePhabricator

[GISel]: Add missing opcodes for overflow intrinsics
ClosedPublic

Authored by aditya_nandakumar on Aug 23 2018, 4:43 PM.

Details

Summary

Currently, IRTranslator (and GISel) seems to be arbitrarily picking which overflow intrinsics get mapped into opcodes which either have a carry as an input or not.
For intrinsics such as Intrinsic::uadd_with_overflow, translate it to an opcode (G_UADDO) which doesn't have any carry inputs (similar to LLVM IR).

This patch adds 4 missing opcodes for completeness - G_UADDO, G_USUBO, G_SSUBE and G_SADDE.

As there is already some code that's selecting G_UADDE (X86) - I haven't changed X86's legalizerinfo. G_UADDEs can still be produced through legalization/combines etc.

Diff Detail

Repository
rL LLVM

Event Timeline

x86 is still using UADDE, and it's generated only by IRTranslator, so if you do this then x86 will lose support for compiling llvm.uadd.with.overflow intrinsics. @igorb what do you think?

Which is exposing a hole in our general GISel testing, where individual changes in passes can result in overall support for some input IR to be lost.

igorb added a comment.Aug 28 2018, 3:07 AM

x86 is still using UADDE, and it's generated only by IRTranslator, so if you do this then x86 will lose support for compiling llvm.uadd.with.overflow intrinsics. @igorb what do you think?

Which is exposing a hole in our general GISel testing, where individual changes in passes can result in overall support for some input IR to be lost.

Hi,
I think currently X86 use G_UADDE generated by Legalizer to support 64bit add on 32bit architecture.

Regards,
Igor

aemerson accepted this revision.Aug 28 2018, 9:01 AM

x86 is still using UADDE, and it's generated only by IRTranslator, so if you do this then x86 will lose support for compiling llvm.uadd.with.overflow intrinsics. @igorb what do you think?

Which is exposing a hole in our general GISel testing, where individual changes in passes can result in overall support for some input IR to be lost.

Hi,
I think currently X86 use G_UADDE generated by Legalizer to support 64bit add on 32bit architecture.

Regards,
Igor

Ah yes, I see it now.

This revision is now accepted and ready to land.Aug 28 2018, 9:01 AM

Submitted in r340865. Thanks.