This is an archive of the discontinued LLVM Phabricator instance.

[RISCV][GISel] Lower G_UADDE, G_UADDO, G_USUBE, and G_USUBO
ClosedPublic

Authored by craig.topper on Aug 11 2023, 11:12 PM.

Details

Summary

RISC-V doesn't have flag registers, we need to implement these
with add/sub and compares.

Remove the untested legalization for the signed versions. We can
add it back when we write tests.

Diff Detail

Event Timeline

craig.topper created this revision.Aug 11 2023, 11:12 PM
Herald added a project: Restricted Project. · View Herald TranscriptAug 11 2023, 11:12 PM
craig.topper requested review of this revision.Aug 11 2023, 11:12 PM
Herald added a project: Restricted Project. · View Herald TranscriptAug 11 2023, 11:12 PM
craig.topper added inline comments.Aug 11 2023, 11:54 PM
llvm/test/CodeGen/RISCV/GlobalISel/legalizer/rv64/legalize-add.mir
110–112

@arsenm Can you check this expansion? It doesn't look correct to me.

Here's the code from LegalizerHelper.

auto TmpRes = MIRBuilder.buildAdd(Ty, LHS, RHS);                             
auto ZExtCarryIn = MIRBuilder.buildZExt(Ty, CarryIn);                        
MIRBuilder.buildAdd(Res, TmpRes, ZExtCarryIn);                               
MIRBuilder.buildICmp(CmpInst::ICMP_ULT, CarryOut, Res, LHS);

If LHS is 0 and RHS is all ones, and CarryIn is 1. TmpRes will be all ones and Res will be 0. Res will be 0. The ICMP_ULT will be false, but we there was an overflow.

craig.topper marked an inline comment as not done.Aug 14 2023, 6:30 PM
craig.topper added inline comments.
llvm/test/CodeGen/RISCV/GlobalISel/legalizer/rv64/legalize-add.mir
110–112

Patch to fix G_UADDE legalization https://reviews.llvm.org/D157943

craig.topper marked an inline comment as not done.

Rebase

nitinjohnraj accepted this revision.Aug 18 2023, 3:58 PM

Thank you for the patch, LGTM

This revision is now accepted and ready to land.Aug 18 2023, 3:58 PM
This revision was landed with ongoing or failed builds.Aug 18 2023, 5:29 PM
This revision was automatically updated to reflect the committed changes.