Page MenuHomePhabricator

[RISCV] Fold (sub constant, (setcc x, y, eq/neq)) -> (add constant - 1, (setcc x, y, neq/eq))
ClosedPublic

Authored by liaolucy on Aug 9 2022, 1:41 AM.

Details

Summary

(setcc x, y, eq/neq) are seqz, snez that set rd = 0/1.

addi is used to process immediate, which can save instructions for load immediate.

Diff Detail

Event Timeline

liaolucy created this revision.Aug 9 2022, 1:41 AM
Herald added a project: Restricted Project. · View Herald TranscriptAug 9 2022, 1:41 AM
liaolucy requested review of this revision.Aug 9 2022, 1:41 AM
craig.topper added inline comments.Aug 9 2022, 9:14 AM
llvm/lib/Target/RISCV/RISCVISelLowering.cpp
8275

Do we need to check that this is the only user of the setcc?

8279

Use ISD::isIntEqualitySetCC(CCVal)

8287

Put these in canonical order with constant on the RHS.

liaolucy updated this revision to Diff 451380.Aug 10 2022, 1:31 AM

Use ISD::isIntEqualitySetCC(CCVal) and put constant on the RHS.

llvm/lib/Target/RISCV/RISCVISelLowering.cpp
8275

At present, I think only setcc, ensure that rd is only 0/1, I have not found other SDNode have this function.

craig.topper added inline comments.Aug 10 2022, 8:44 AM
llvm/lib/Target/RISCV/RISCVISelLowering.cpp
8275

That wasn’t my question. What if the setcc is used by another instruction other than this sub. Then the original setcc won’t be removed and there will be two setccs with opposite conditions.

liaolucy updated this revision to Diff 451743.Aug 10 2022, 11:40 PM

Address craig.topper's comments and thanks. Add sub is the only user of the setcc.

craig.topper added inline comments.Aug 11 2022, 9:01 AM
llvm/lib/Target/RISCV/RISCVISelLowering.cpp
8276

You can use N1.hasOneUse() which should be cheaper. isOnlyUser is more useful if N had multiple operands that could each be N1. We know one operand of N is a constant so N1 can only be used by one operand.

8282

This line has undefined behavior if N0 happens to be 0x8000000000000000.

It's also incorrect if N is type i65 or larger. Probably best to use APInt which will fix both issues.

liaolucy updated this revision to Diff 452051.Aug 11 2022, 7:06 PM

Use N1.hasOneUse(), APInt and dyn_cast

craig.topper accepted this revision.Aug 12 2022, 9:50 AM

LGTM with that change.

llvm/lib/Target/RISCV/RISCVISelLowering.cpp
8284

Use DAG.getSetCC then you don't need to call getCondCode.

This revision is now accepted and ready to land.Aug 12 2022, 9:50 AM
This revision was landed with ongoing or failed builds.Aug 13 2022, 5:38 AM
This revision was automatically updated to reflect the committed changes.