This is an archive of the discontinued LLVM Phabricator instance.

[RISCV] Support peephole optimization to fold vmerge.vvm that has tail agnostic policy and unmasked intrinsics.
ClosedPublic

Authored by jacquesguan on Aug 30 2022, 2:36 AM.

Diff Detail

Event Timeline

jacquesguan created this revision.Aug 30 2022, 2:36 AM
Herald added a project: Restricted Project. · View Herald TranscriptAug 30 2022, 2:36 AM
jacquesguan requested review of this revision.Aug 30 2022, 2:36 AM
fakepaper56 added inline comments.Aug 30 2022, 5:00 AM
llvm/lib/Target/RISCV/RISCVISelDAGToDAG.cpp
2809

How about using RISCVII::TAIL_AGNOSTIC to replace 1?

craig.topper added inline comments.Aug 30 2022, 12:44 PM
llvm/test/CodeGen/RISCV/rvv/fixed-vectors-peephole-vmerge-vops.ll
198

This comment is incorrect

Address comment.

llvm/lib/Target/RISCV/RISCVISelDAGToDAG.cpp
2809

Done.

llvm/test/CodeGen/RISCV/rvv/fixed-vectors-peephole-vmerge-vops.ll
198

Done.

fakepaper56 added inline comments.Sep 3 2022, 12:47 AM
llvm/lib/Target/RISCV/RISCVISelDAGToDAG.cpp
2801–2802

Why you test vector policy of MaskedOpc here?

Fix wrong check.

jacquesguan added inline comments.Sep 4 2022, 7:40 PM
llvm/lib/Target/RISCV/RISCVISelDAGToDAG.cpp
2801–2802

Thanks, it's a mistake, I am intended to check whether MaskedOpc has mergeop here. Fixed now.

fakepaper56 added inline comments.Sep 5 2022, 7:17 AM
llvm/lib/Target/RISCV/RISCVISelDAGToDAG.cpp
2801–2802

I think we should have merge operand and vector policy operand to make sure the result is same as the original VMERGE_TU nodes. I am sorry that I ignore it. I open a new revision D133302 to refine the code.

Can you land the new tests and rebase please? I want to be able to study the test diffs.

llvm/lib/Target/RISCV/RISCVISelDAGToDAG.cpp
2801–2802

Do we have an example of masked pseudo without a merge operand? If not, this if-clause should be assert instead.

2810

This else handles everything which isn't a vmerge right?

If so, the code would be cleaner if it could be written as:

if (not a vmerge)
  continue;

if (TA) {
} else {
}

precommit test and rebase main.

Can you land the new tests and rebase please? I want to be able to study the test diffs.

Done, I land the test first and rebase to it.

llvm/lib/Target/RISCV/RISCVISelDAGToDAG.cpp
2801–2802

Done, I changed to assert.

2810

Since rebase to main, these code form was changed.

fakepaper56 added inline comments.Sep 19 2022, 11:06 PM
llvm/lib/Target/RISCV/RISCVISelDAGToDAG.cpp
2635

Is it better to rewrite to /* TUMU */ 0?

fakepaper56 added inline comments.Sep 19 2022, 11:52 PM
llvm/lib/Target/RISCV/RISCVISelDAGToDAG.cpp
2635

Sorry that I am late to ask the question.

Address comment.

llvm/lib/Target/RISCV/RISCVISelDAGToDAG.cpp
2635

Done and thanks.

fakepaper56 accepted this revision.Sep 20 2022, 4:34 PM
This revision is now accepted and ready to land.Sep 20 2022, 4:34 PM