This is an archive of the discontinued LLVM Phabricator instance.

[ARM] Look through concat when lowering in-place shuffles (VZIP, ..)
ClosedPublic

Authored by ab on Jun 12 2015, 3:49 PM.

Details

Summary

Currently, we canonicalize shuffles that produce a result larger than
their operands with:

shuffle(concat(v1, undef), concat(v2, undef))

->

shuffle(concat(v1, v2), undef)

because we can access quad vectors (see PerformVECTOR_SHUFFLECombine).

This is useful in the general case, but there are special cases where
native shuffles produce larger results: the two-result ops.

Look through the concat when lowering them:

shuffle(concat(v1, v2), undef)

->

concat(VZIP(v1, v2):0, :1)

This lets us generate the native shuffles instead of scalarizing to
dozens of VMOVs.

I'm a little worried about the disparity between the lowering and
isShuffleMaskLegal, but with the current API we have no way of looking
at the actual operands, and this isn't a problem in practice because
the ARM combine runs last.

The obvious alternative would be to stop doing the combine, but I
think it's useful. We can also avoid doing it for these masks, but
we'll still need to look through concat(v, undef) to avoid
generating needlessly-wide shuffles.

Diff Detail

Repository
rL LLVM

Event Timeline

ab updated this revision to Diff 27613.Jun 12 2015, 3:49 PM
ab retitled this revision from to [ARM] Look through concat when lowering in-place shuffles (VZIP, ..).
ab updated this object.
ab edited the test plan for this revision. (Show Details)
ab added subscribers: Unknown Object (MLST), jmolloy, t.p.northover and 2 others.
qcolombet accepted this revision.Jun 17 2015, 11:04 AM
qcolombet added a reviewer: qcolombet.

Hi Ahmed,

LGTM with some improvements on the tests patterns.

If you really really are motivated, you could fix all the patterns in a subsequent commit :).

Cheers,
-Quentin

test/CodeGen/ARM/vtrn.ll
17 ↗(On Diff #27613)

I know this is consistent with the surrounding tests, but I would prefer that we check that the arguments are what we expect. In other words, could you check that we are feeding the right arguments here?

This revision is now accepted and ready to land.Jun 17 2015, 11:04 AM
This revision was automatically updated to reflect the committed changes.
ab added a comment.Jun 18 2015, 7:38 PM

No need for motivation with Chandler's script ;)

r240114, r240116, r240118.

-Ahmed