This is an archive of the discontinued LLVM Phabricator instance.

[RISCV] Fold (and (not (srl X, C)), 1) to (xor (bexti X, C), 1) when have Zbs extension.
ClosedPublic

Authored by jacquesguan on Dec 13 2021, 4:51 AM.

Details

Summary

When have Zbs extension, we could use bexti to fold (and (not (srl X, C)), 1) to (xor (bexti X, C), 1).

Diff Detail

Event Timeline

jacquesguan created this revision.Dec 13 2021, 4:51 AM
jacquesguan requested review of this revision.Dec 13 2021, 4:51 AM
Herald added a project: Restricted Project. · View Herald TranscriptDec 13 2021, 4:51 AM

Have you seen real cases of this pattern? InstCombine canonicalizes this to (xor (and (srl X, C), 1), 1) before we get to SelectionDAG.

Have you seen real cases of this pattern? InstCombine canonicalizes this to (xor (and (srl X, C), 1), 1) before we get to SelectionDAG.

Yes. As you said, InstCombine do canonicalize this to xor (and (srl X, C), 1), 1 , but DAGCombine will also fold (xor (and x, y), y) -> (and (not x), y). So we still get and (not (srl X, C)), 1 in SelectionDAG. I will add another case xor (and (srl X, C), 1), 1 to show this.

Add xor (and (srl X, C), 1), 1 test case.

craig.topper added inline comments.Dec 13 2021, 11:45 PM
llvm/lib/Target/RISCV/RISCVInstrInfoZb.td
795

I wonder if the (XORI (BEXTI GPR:$rs1, $shamt), 1) we would have gotten from the original InstCombine canonicalization would be better.

Guess it depends on whether the Zbs instructions are implemented as well as XORI or not. There is no c.xori or that would have been a good reason to favor xori.

jacquesguan added inline comments.Dec 14 2021, 10:59 PM
llvm/lib/Target/RISCV/RISCVInstrInfoZb.td
795

Thanks, so that I could change DAGCombine to keep xor (and(srl X, C),1), 1 when we have single-Bit extract instruction in backend. In this situation, at least, we fold (and(srl X, C),1) into BEXTI X, C. What's your option about this?

jacquesguan added inline comments.Dec 14 2021, 11:10 PM
llvm/lib/Target/RISCV/RISCVInstrInfoZb.td
795

Sorry, I misunderstanded your comment. Please ignore my last reply. I would simply change that pattern into

def : Pat<(and (not (srl GPR:$rs1, uimmlog2xlen:$shamt)), (XLenVT 1)),
                   (XORI (BEXTI GPR:$rs1, uimmlog2xlen:$shamt), (XLenVT 1))

use xori instead of binvi.

Should we have a test in rv64zbs.ll too?

Add RV64 test.

Should we have a test in rv64zbs.ll too?

Done, thanks a lot.

craig.topper added inline comments.Dec 15 2021, 10:45 AM
llvm/test/CodeGen/RISCV/rv32zbs.ll
360

sbexti->bexti I'll rename the existing test cases today.

Update test case and change commit message.

llvm/test/CodeGen/RISCV/rv32zbs.ll
360

Done.

craig.topper accepted this revision.Dec 15 2021, 10:55 PM

LGTM. Please fix the title to remove the mention of binvi since we aren't using that now.

This revision is now accepted and ready to land.Dec 15 2021, 10:55 PM
jacquesguan retitled this revision from [RISCV] Use binvi and bexti to fold and (not (srl X, C)), 1 to [RISCV] Fold (and (not (srl X, C)), 1) to (xor (bexti X, C), 1) when have Zbs extension..Dec 15 2021, 10:59 PM
jacquesguan edited the summary of this revision. (Show Details)
This revision was landed with ongoing or failed builds.Dec 15 2021, 11:15 PM
This revision was automatically updated to reflect the committed changes.