This is an archive of the discontinued LLVM Phabricator instance.

[RISCV] Improve RV64 codegen for i32 ISD::SADDO when RHS is constant.
ClosedPublic

Authored by craig.topper on May 8 2023, 12:06 PM.

Details

Summary

This uses the same sequence we get from LegalizeDAG for i32 on RV32, but modified
to use W instructions.

When the RHS is constant one of the setccs simplifies to a constant and the xor will either
be an xori with 1 or get removed.

When the RHS is not a constant it was not an obvious improvement and it was a regression
when used with a branch. So I've restricted to the constant case.

Diff Detail

Event Timeline

craig.topper created this revision.May 8 2023, 12:06 PM
Herald added a project: Restricted Project. · View Herald TranscriptMay 8 2023, 12:06 PM
craig.topper requested review of this revision.May 8 2023, 12:06 PM
Herald added a project: Restricted Project. · View Herald TranscriptMay 8 2023, 12:06 PM
craig.topper edited the summary of this revision. (Show Details)May 8 2023, 12:07 PM
craig.topper added a reviewer: asb.
reames added inline comments.May 8 2023, 12:41 PM
llvm/lib/Target/RISCV/RISCVISelLowering.cpp
9041

The uaddo case below uses a comparison of two add flavors. Does the same notion work here for the non-constant case?

9046

The uaddo/usubo case just below uses any_extend here, can we get away with the same?

craig.topper added inline comments.May 8 2023, 12:56 PM
llvm/lib/Target/RISCV/RISCVISelLowering.cpp
9041

I don't think uaddo uses two add flavors. uaddo should be doing an ADDW and comparing the result to the sign extended version of the original LHS.

The default expansion for saddo does an i64 ADD with sign extended inputs and an ADDW. If the results from the ADD and ADDW don't match it overflowed. Is that were you suggesting?

9046

LHS and RHS are used against by the compares which do need SIGN_EXTEND. The uaddo code generates a separate SIGN_EXTEND for LHS later. There must have been some reason I didn't sign extend before the ADD in uaddo but I don't remember why.

reames accepted this revision.May 8 2023, 1:00 PM

LGTM - both responses make sense.

This revision is now accepted and ready to land.May 8 2023, 1:00 PM
This revision was landed with ongoing or failed builds.May 8 2023, 1:05 PM
This revision was automatically updated to reflect the committed changes.