Page MenuHomePhabricator

[DAGCombiner][VP] Add DAGCombine for VP_MUL.
Needs ReviewPublic

Authored by jacquesguan on Mar 8 2022, 12:06 AM.

Details

Summary

This patch contains two DAGCombine:
fold vp_mul X, (splat -1), Mask, EVL -> vp_sub (splat 0), X, Mask, EVL
fold vp_mul X, (1 << C), Mask, EVL -> vp_shl X, C, Mask, EVL

Diff Detail

Unit TestsFailed

TimeTest
60,460 msx64 debian > Clang.CodeGen/RISCV/rvv-intrinsics-overloaded::vluxseg_mask.c
Script: -- : 'RUN: at line 3'; /var/lib/buildkite-agent/builds/llvm-project/build/bin/clang -cc1 -internal-isystem /var/lib/buildkite-agent/builds/llvm-project/build/lib/clang/15.0.0/include -nostdsysteminc -triple riscv64 -target-feature +f -target-feature +d -target-feature +v -disable-O0-optnone -emit-llvm /var/lib/buildkite-agent/builds/llvm-project/clang/test/CodeGen/RISCV/rvv-intrinsics-overloaded/vluxseg_mask.c -o - | /var/lib/buildkite-agent/builds/llvm-project/build/bin/opt -S -mem2reg | /var/lib/buildkite-agent/builds/llvm-project/build/bin/FileCheck --check-prefix=CHECK-RV64 /var/lib/buildkite-agent/builds/llvm-project/clang/test/CodeGen/RISCV/rvv-intrinsics-overloaded/vluxseg_mask.c

Event Timeline

jacquesguan created this revision.Mar 8 2022, 12:06 AM
Herald added a project: Restricted Project. · View Herald TranscriptMar 8 2022, 12:06 AM
jacquesguan requested review of this revision.Mar 8 2022, 12:06 AM
RKSimon added inline comments.Mar 8 2022, 9:06 AM
llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp
22858

(style) Don't use auto unless the type is obvious (i.e. a cast<>)

craig.topper added inline comments.Mar 8 2022, 9:21 AM
llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp
22858

Don't call isConstOrConstSplat(N1) twice

22860

What about mul by positive 1 or 0?

Address comment.

llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp
22858

Done.

22860

Done.

craig.topper added inline comments.Mar 9 2022, 11:53 AM
llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp
22861

If one of the true/false values of the VP_SELECT is undef can we ignore the mask and EVL and return the other operand? @simoll @frasercrmck

simoll added inline comments.Mar 11 2022, 8:15 AM
llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp
22861

I believe so. Also for vp ops other than select or merge, if the mask is undef, we can scratch the operation entirely. If evl is undef, we can cut short to unreachable.

Address comment.

jacquesguan added inline comments.Mar 13 2022, 7:24 PM
llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp
22861

Done, thanks.

craig.topper added inline comments.Mar 13 2022, 7:44 PM
llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp
22863

I think we should only use getSplatVector for scalable vectors. Fixed vectors should use getSplatBuildVector.

22863

You can just call DAG.getConstant(0, SDLoc(N), VT); It will do the right thing.

22874

DAG.getConstant(0, SDLoc(N), VT)

22882

Same comment as above.

Address comment.

jacquesguan added inline comments.Mar 13 2022, 8:45 PM
llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp
22863

Done.

22874

Done.

22882

Done.

@jacquesguan Thanks for working on VP optimizations! We just discussed on the VP syncup call where we want to go with this.
The thing is that this patch is essentially replicating the existing MUL DAGCombiner patterns for VP_MUL. Ideally, we would lift the existing DAGCombiner logic to work on both VP and non-VP SDNodes - automatically.

I've implemented something like this in NEC's open source LLVM stack:
https://github.com/sx-aurora-dev/llvm-project/blob/develop/llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp

The idea is to templatize the pattern-matching and rewriting code and instantiate that for each VP and non-VP. In essence, https://reviews.llvm.org/D92086 but on the SelectionDAG.

Would you be interested in working on a more general approach? Please contact me directly so we can get you on the VP Discord server or arrange a call to talk about this.