This is an archive of the discontinued LLVM Phabricator instance.

[RISCV] Apply DeMorgan to (beqz (and/or (seteq), (xor Z, 1))) to remove the xor.
ClosedPublic

Authored by craig.topper on Aug 26 2022, 4:40 PM.

Details

Summary

We can rewrite to (bnez (or/and (setne), Z) is Z is 0/1.

Alternatively, we could canonicalize to (xor (or/and (setne), Z), 1)
even if there is no branch. The xor would not always get removed,
but it might enable other DeMorgan combines. I decided to be
conservative for this first patch and require the xor to be removed.

I have a couple other invertible setccs I will add in a follow up
patch.

Diff Detail

Event Timeline

craig.topper created this revision.Aug 26 2022, 4:40 PM
Herald added a project: Restricted Project. · View Herald TranscriptAug 26 2022, 4:40 PM
craig.topper requested review of this revision.Aug 26 2022, 4:40 PM
Herald added a project: Restricted Project. · View Herald TranscriptAug 26 2022, 4:40 PM
reames accepted this revision.Aug 29 2022, 11:24 AM

LGTM

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

I think you meant xor Z, 1?

9164

This bit of logic seems like it's getting repeated a bunch, can we factor out a utility? Something like matchLogicalNot?

llvm/test/CodeGen/RISCV/setcc-logic.ll
306

As a follow up, is it worth exploiting the fact that we can skip the snez on the xor and just use the xor result as a non-boolean?

This revision is now accepted and ready to land.Aug 29 2022, 11:24 AM
craig.topper added inline comments.Aug 29 2022, 12:02 PM
llvm/lib/Target/RISCV/RISCVISelLowering.cpp
9142

I did indeed.

9164

Just the constant value checking? I don't think matchLogicalNot is a good name for that. I'm not sure what a good name is. isOneOrAllOnes with a bool for AllowAllOnes?

reames added inline comments.Aug 29 2022, 12:04 PM
llvm/lib/Target/RISCV/RISCVISelLowering.cpp
9164

I was thinking to fold the masked value check in as well. The think we're trying for is to match the logical not of a boolean Z right?

craig.topper added inline comments.Aug 29 2022, 12:10 PM
llvm/lib/Target/RISCV/RISCVISelLowering.cpp
9164

Yes. My thought was that the name only makes sense if the Xor opcode check is also in it.