Page MenuHomePhabricator
Differential D122791

[DAGCombiner] Teach scalarizeBinOpOfSplats handle scalable splat.
ClosedPublic
Actions

Authored by jacquesguan on Mar 31 2022, 2:15 AM.

Details

Reviewers
RKSimon
craig.topper
frasercrmck
benshi001
abrachet
mkazantsev
Commits
rG58dfaaaace4e: [DAGCombiner] Teach scalarizeBinOpOfSplats handle scalable splat.
Summary

This revision supports to scalarize a binary operation of two scalable
splat vectors.

Diff Detail

Unit TestsFailed
View All

TimeTest
120 msx64 debian > LLVM.CodeGen/AArch64::active_lane_mask.ll
Script: -- : 'RUN: at line 2'; /var/lib/buildkite-agent/builds/llvm-project/build/bin/llc -mtriple=aarch64-linux-gnu -mattr=+sve < /var/lib/buildkite-agent/builds/llvm-project/llvm/test/CodeGen/AArch64/active_lane_mask.ll | /var/lib/buildkite-agent/builds/llvm-project/build/bin/FileCheck /var/lib/buildkite-agent/builds/llvm-project/llvm/test/CodeGen/AArch64/active_lane_mask.ll
100 msx64 debian > LLVM.CodeGen/AArch64::fdiv-combine.ll
Script: -- : 'RUN: at line 2'; /var/lib/buildkite-agent/builds/llvm-project/build/bin/llc -mtriple=aarch64-unknown-unknown < /var/lib/buildkite-agent/builds/llvm-project/llvm/test/CodeGen/AArch64/fdiv-combine.ll | /var/lib/buildkite-agent/builds/llvm-project/build/bin/FileCheck /var/lib/buildkite-agent/builds/llvm-project/llvm/test/CodeGen/AArch64/fdiv-combine.ll
210 msx64 debian > LLVM.CodeGen/AArch64::sve-fixed-length-fp-select.ll
Script: -- : 'RUN: at line 2'; /var/lib/buildkite-agent/builds/llvm-project/build/bin/llc -aarch64-sve-vector-bits-min=256 < /var/lib/buildkite-agent/builds/llvm-project/llvm/test/CodeGen/AArch64/sve-fixed-length-fp-select.ll | /var/lib/buildkite-agent/builds/llvm-project/build/bin/FileCheck /var/lib/buildkite-agent/builds/llvm-project/llvm/test/CodeGen/AArch64/sve-fixed-length-fp-select.ll -check-prefixes=CHECK,VBITS_GE_256
160 msx64 debian > LLVM.CodeGen/AArch64::sve-fixed-length-int-select.ll
Script: -- : 'RUN: at line 2'; /var/lib/buildkite-agent/builds/llvm-project/build/bin/llc -aarch64-sve-vector-bits-min=256 < /var/lib/buildkite-agent/builds/llvm-project/llvm/test/CodeGen/AArch64/sve-fixed-length-int-select.ll | /var/lib/buildkite-agent/builds/llvm-project/build/bin/FileCheck /var/lib/buildkite-agent/builds/llvm-project/llvm/test/CodeGen/AArch64/sve-fixed-length-int-select.ll -check-prefixes=CHECK,VBITS_GE_256
110 msx64 debian > LLVM.CodeGen/AArch64::sve-gep.ll
Script: -- : 'RUN: at line 2'; /var/lib/buildkite-agent/builds/llvm-project/build/bin/llc -mtriple=aarch64-linux-gnu -mattr=+sve < /var/lib/buildkite-agent/builds/llvm-project/llvm/test/CodeGen/AArch64/sve-gep.ll | /var/lib/buildkite-agent/builds/llvm-project/build/bin/FileCheck /var/lib/buildkite-agent/builds/llvm-project/llvm/test/CodeGen/AArch64/sve-gep.ll
View Full Test Results (7 Failed)

Event Timeline

jacquesguan created this revision.Mar 31 2022, 2:15 AM
Herald added a project: Restricted Project. · View Herald TranscriptMar 31 2022, 2:15 AM
Herald added subscribers: luke957, StephenFan, ecnelises and 21 others. · View Herald Transcript
jacquesguan requested review of this revision.Mar 31 2022, 2:15 AM
Herald added a project: Restricted Project. · View Herald TranscriptMar 31 2022, 2:15 AM
Herald added subscribers: llvm-commits, alextsao1999, pcwang-thead, MaskRay. · View Herald Transcript
Harbormaster completed remote builds in B157124: Diff 419371.Mar 31 2022, 9:48 AM
RKSimon added a comment.Apr 7 2022, 2:02 AM

precommit the new tests and rebase so the patch shows the diff?

jacquesguan added a parent revision: D123362: [RISCV][test] Precommit test for D122791..Apr 7 2022, 11:51 PM
jacquesguan updated this revision to Diff 421425.Apr 7 2022, 11:52 PM

rebase the precommit test.

Harbormaster completed remote builds in B158630: Diff 421425.Apr 8 2022, 12:39 AM
jacquesguan added a comment.Jun 14 2022, 8:18 PM

ping.

craig.topper added inline comments.Jun 14 2022, 9:49 PM
llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp
23389

Not sure we should be creating SPLAT_VECTOR on targets that don't support SPLAT_VECTOR. At the very least it's a waste of compile time to convert it back to a BUILD_VECTOR.

RKSimon added inline comments.Jun 15 2022, 4:31 AM
llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp
23360

Use DAG.isSplatValue instead?

23389

+1 at the very least we need a hasOperation() check for the getSplatVector path and getBuildVector fallback

jacquesguan updated this revision to Diff 443538.Jul 10 2022, 7:06 PM

Address comment.

jacquesguan updated this revision to Diff 443540.Jul 10 2022, 7:26 PM

Fix test case.

jacquesguan added inline comments.Jul 10 2022, 7:27 PM
llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp
23360

I tried to use DAG.isSplatValue, but this way would affect many no splat cases of X86 target, some of them are negative.

23389

Done, I only build splat_vector when is scalable now.

Harbormaster completed remote builds in B174593: Diff 443540.Jul 10 2022, 8:23 PM
jacquesguan added a comment.Jul 17 2022, 11:50 PM

ping.

RKSimon accepted this revision.Jul 18 2022, 1:53 AM

LGTM with one minor

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

OK - please can you add a TODO comment? It sounds like something that needs revisiting in the future.

This revision is now accepted and ready to land.Jul 18 2022, 1:53 AM
jacquesguan mentioned this in rG3fcaea176cbf: [RISCV][test] Precommit test for D122791..Jul 18 2022, 8:09 PM
jacquesguan updated this revision to Diff 445683.Jul 18 2022, 8:22 PM

Address comment.

This revision was landed with ongoing or failed builds.Jul 18 2022, 8:23 PM
Closed by commit rG58dfaaaace4e: [DAGCombiner] Teach scalarizeBinOpOfSplats handle scalable splat. (authored by jacquesguan). · Explain Why
This revision was automatically updated to reflect the committed changes.
jacquesguan added a commit: rG58dfaaaace4e: [DAGCombiner] Teach scalarizeBinOpOfSplats handle scalable splat..
Harbormaster completed remote builds in B176161: Diff 445683.Jul 18 2022, 9:07 PM
abrachet added a subscriber: abrachet.Jul 18 2022, 10:55 PM

We're seeing failures after this revision https://ci.chromium.org/ui/p/fuchsia/builders/toolchain.ci/clang-linux-x64/b8808242343748276609/overview, as well as upstream bots https://lab.llvm.org/buildbot/#/builders/188/builds/17108

mkazantsev added a reverting change: rG69b284aaf6a1: Revert "[DAGCombiner] Teach scalarizeBinOpOfSplats handle scalable splat.".Jul 18 2022, 11:42 PM
jacquesguan reopened this revision.Jul 19 2022, 1:24 AM
This revision is now accepted and ready to land.Jul 19 2022, 1:24 AM
jacquesguan updated this revision to Diff 445724.Jul 19 2022, 1:25 AM

Fix tests.

jacquesguan added reviewers: abrachet, mkazantsev.Jul 19 2022, 1:30 AM
In D122791#3661574, @abrachet wrote:

We're seeing failures after this revision https://ci.chromium.org/ui/p/fuchsia/builders/toolchain.ci/clang-linux-x64/b8808242343748276609/overview, as well as upstream bots https://lab.llvm.org/buildbot/#/builders/188/builds/17108

Thanks a lot, and I have fixed the tests.

Harbormaster completed remote builds in B176183: Diff 445724.Jul 19 2022, 2:10 AM
jacquesguan updated this revision to Diff 446066.Jul 20 2022, 1:03 AM

rebase main

Harbormaster completed remote builds in B176442: Diff 446066.Jul 20 2022, 1:48 AM
jacquesguan closed this revision.Jul 21 2022, 4:18 AM

recommit in https://reviews.llvm.org/rGe60eb7053dc91a23cc3b96652aeda7513dd4b082

Revision Contents
Changeset List

PathSize
llvm/
lib/
CodeGen/
SelectionDAG/
7 lines
test/
CodeGen/
RISCV/
rvv/
4 lines
4 lines
23 lines
4 lines
4 lines
4 lines

Diff 443540

View Options

llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp

Loading...
View Options

llvm/test/CodeGen/RISCV/rvv/vadd-sdnode.ll

Loading...
View Options

llvm/test/CodeGen/RISCV/rvv/vand-sdnode.ll

Loading...
View Options

llvm/test/CodeGen/RISCV/rvv/vmul-sdnode.ll

Loading...
View Options

llvm/test/CodeGen/RISCV/rvv/vor-sdnode.ll

Loading...
View Options

llvm/test/CodeGen/RISCV/rvv/vsub-sdnode.ll

Loading...
View Options

llvm/test/CodeGen/RISCV/rvv/vxor-sdnode.ll

Loading...