This is an archive of the discontinued LLVM Phabricator instance.

[RISCV] Add DAG combine to fold (sub 0, (setcc x, 0, setlt)) -> (sra x , xlen - 1)
ClosedPublic

Authored by liaolucy on Apr 4 2023, 8:20 AM.

Details

Summary

The result of sub + setcc is 0 or 1 for all bits.
The sra instruction get the same result.

Diff Detail

Event Timeline

liaolucy created this revision.Apr 4 2023, 8:20 AM
Herald added a project: Restricted Project. · View Herald TranscriptApr 4 2023, 8:20 AM
liaolucy requested review of this revision.Apr 4 2023, 8:20 AM
craig.topper requested changes to this revision.EditedApr 4 2023, 9:01 PM

This is not correct for the add cases

A correct transform would be

fold (add (setcc x, 0, setlt), -1) -> (xor (sra x, xlen - 1), -1)
fold (add (setcc 0, x, setgt), -1) -> (xor (sra x, xlen - 1), -1)

But I'm not sure that's profitable.

Alive2: https://alive2.llvm.org/ce/z/JQRoji

This revision now requires changes to proceed.Apr 4 2023, 9:01 PM
liaolucy updated this revision to Diff 511109.Apr 5 2023, 8:10 AM
liaolucy retitled this revision from [RISCV] Add DAG combine to fold (add (setcc x, 0, setlt), -1) -> (sra x, xlen - 1) to [RISCV] Add DAG combine to fold (sub 0, (setcc x, 0, setlt)) -> (sra x , xlen - 1).
liaolucy edited the summary of this revision. (Show Details)

delete the add cases

craig.topper added inline comments.Apr 5 2023, 10:12 AM
llvm/lib/Target/RISCV/RISCVISelLowering.cpp
9138

You can delete the setgt code. It will never exist in that form. Prior to LegalizeDAG we keep constants on the RHS of setcc. LegalizeDAG won't have any reason to change it.

The only case that should have 0 on the left hand side is (setcc 0, x, setlt) which we get from legalizing (setcc x, 0, setgt).

liaolucy updated this revision to Diff 511404.Apr 6 2023, 7:12 AM
liaolucy edited the summary of this revision. (Show Details)

Address comments, thanks

craig.topper accepted this revision.Apr 6 2023, 9:25 AM

LGTM with that comment addressed.

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

Move VT and DL inside the ifs since they aren't used until getNode.

This revision is now accepted and ready to land.Apr 6 2023, 9:25 AM