This is an archive of the discontinued LLVM Phabricator instance.

[SelectionDAG] Extend FoldConstantVectorArithmetic to SPLAT_VECTOR
ClosedPublic

Authored by frasercrmck on May 27 2021, 6:12 AM.

Details

Summary

This patch extends the SelectionDAG's ability to constant-fold vector
arithmetic to include support for SPLAT_VECTOR. This is not only for
scalable-vector types but also for fixed-length vector types, which
helps Hexagon in a couple of cases.

The original RISC-V test case was in fact an infinite DAGCombine loop.
The pattern and (truncate v1), (truncate v2) can be combined to
truncate (and v1, v2) but the truncate can similarly be combined back
to truncate (and v1, v2) (but, crucially, only when one of v1 or
v2 is a constant vector).

It wasn't exposed in on fixed-length types because a TRUNCATE of a
constant BUILD_VECTOR was folded into the BUILD_VECTOR itself, whereas
this did not happen for the equivalent (scalable-vector) SPLAT_VECTOR.

Diff Detail

Event Timeline

frasercrmck created this revision.May 27 2021, 6:12 AM
frasercrmck requested review of this revision.May 27 2021, 6:12 AM
Herald added a project: Restricted Project. · View Herald TranscriptMay 27 2021, 6:12 AM
RKSimon accepted this revision.May 28 2021, 3:51 AM

LGTM

This revision is now accepted and ready to land.May 28 2021, 3:51 AM

Thanks for the review, @RKSimon! I'm hoping to get someone like @kparzysz and @t.p.northover to review the changes in the other targets before I merge.

craig.topper added inline comments.May 28 2021, 10:31 AM
llvm/lib/CodeGen/SelectionDAG/SelectionDAG.cpp
5302

Why don't we need to check that the splat vector operand is constant/undef like we do for build_vector?

5374–5375

When I made similar changes to FoldConstantArithmetic, I was asked to create a splat vector if the input was splat vectors. Should we be consistent? Though maybe it's harder because you're dealing with more than 2 sources?

frasercrmck added inline comments.May 31 2021, 4:10 AM
llvm/lib/CodeGen/SelectionDAG/SelectionDAG.cpp
5302

Oh, good catch! I'm surprised nothing failed. I'll see to that.

5374–5375

Yeah that was my thinking, we may be dealing with mixed splat_vector and build_vector here. Hexagon would certainly exercise that path. We can't get it perfect here. We could certainly add heuristics like:

if all ScalarResults are equal, and:

  1. if any Ops was a splat
  2. if the majority of Ops were splats
  3. splat is legal for this data type
  4. splat is legal and custom for this data type

.. then produce a splat. Or a combination thereof. I'm not sure what's preferable. We could assume that the target would canonicalize any BUILD_VECTOR into SPLAT_VECTOR but I don't know if we can guarantee that either.

I'm also concerned that Hexagon seems to be the only target which uses SPLAT_VECTOR for fixed-length vectors and we don't have great coverage.

  • catch non-constant splat_vectors
  • update rvv test checks
frasercrmck added inline comments.May 31 2021, 4:44 AM
llvm/lib/CodeGen/SelectionDAG/SelectionDAG.cpp
5302

I've updated to use ISD::isConstantSplatVector which isn't ideal but I didn't want to add yet-another method for checking these nodes.

Matt added a subscriber: Matt.Jun 3 2021, 8:41 AM
craig.topper added inline comments.Jun 3 2021, 5:47 PM
llvm/lib/CodeGen/SelectionDAG/SelectionDAG.cpp
5374–5375

I believe there is a DAG combine that converts BUILD_VECTOR to SPLAT_VECTOR if it's legal/custom so it'll probably get taken care of.

It's kinda bold of Hexagon to use SPLAT_VECTOR with that DAG combine in there guaranteeing it will fold early with so many optimizations missing.