This is an archive of the discontinued LLVM Phabricator instance.

[RISCV] Teach DAG combine to fold (and (select_cc lhs, rhs, cc, -1, c), x) -> (select_cc lhs, rhs, cc, x, (and, x, c))
ClosedPublic

Authored by craig.topper on Apr 28 2021, 1:48 PM.

Details

Summary

Similar for or/xor with 0 in place of -1.

This is the canonical form produced by InstCombine for something like c ? x & y : x; Since we have to use control flow to expand select we'll usually end up with a mv in basic block. By folding this we may be able to pull the and/or/xor into the block instead and avoid a mv instruction.

The code here is based on code from ARM that uses this to create predicated instructions. I'm doing it on SELECT_CC so it happens late, but we could do it on select earlier which is what ARM does. I'm not sure if we lose any combine opportunities if we do it earlier.

I left out add and sub because this can separate sext.w from the add/sub. It also made a conditional subtract on i64 addition/subtraction RV32 worse. I guess both of those would be fixed by doing this earlier on select.

The select-binop-identity.ll test has not been commited yet, but I made the diff show the changes to it.

Diff Detail

Event Timeline

craig.topper created this revision.Apr 28 2021, 1:48 PM
craig.topper requested review of this revision.Apr 28 2021, 1:48 PM
Herald added a project: Restricted Project. · View Herald TranscriptApr 28 2021, 1:48 PM
Herald added a subscriber: MaskRay. · View Herald Transcript
luismarques accepted this revision.Apr 29 2021, 3:40 AM

LGTM.

llvm/lib/Target/RISCV/RISCVISelLowering.cpp
5183–5186

[AllOnes=0]?

5192–5193

Putting this here is a clean and safe way to write it, but I wonder if this should be moved to combineSelectCCAndUseCommutative to avoid repeated work, or if that would be confusing and error-prone for future changes?

llvm/test/CodeGen/RISCV/select-binop-identity.ll
10–11

Don't forget to remove the TODO.

This revision is now accepted and ready to land.Apr 29 2021, 3:40 AM
craig.topper added inline comments.Apr 29 2021, 7:53 AM
llvm/lib/Target/RISCV/RISCVISelLowering.cpp
5192–5193

It shouldn't be repeated work. The Slct and OtherOp operands are swapped for the second call from combineSelectCCAndUseCommutative

luismarques added inline comments.Apr 29 2021, 8:56 AM
llvm/lib/Target/RISCV/RISCVISelLowering.cpp
5192–5193

Yup, nevermind, I had a wrong mental model when I wrote that comment.