Page MenuHomePhabricator

[RISCV] Add DAG combine to optimize vXi64 all ones/zeros fixed vector on RV32.
AbandonedPublic

Authored by craig.topper on Mar 11 2021, 10:30 PM.

Details

Reviewers
frasercrmck
Summary

vXi64 build_vectors are converted to v2Xi32 during type legalization
on RV32. This causes the use of SEW=32 splat instead of SEW=64.

This patch adds a DAG combine to detect the bitcasted pattern
and turn it into a SEW=64 splat.

We could maybe fix this in type legalization, but I'm not sure
we want to lose all zeroes/ones vector knowledge that early since
it's used by negate and not idioms for example.

Diff Detail

Event Timeline

craig.topper created this revision.Mar 11 2021, 10:30 PM
craig.topper requested review of this revision.Mar 11 2021, 10:30 PM
Herald added a project: Restricted Project. · View Herald TranscriptMar 11 2021, 10:30 PM
Herald added a subscriber: MaskRay. · View Herald Transcript

Sorry for putting this off for so long. I do think we need to do something about this. I've tried to think about alternatives with a wider scope (presumably requiring type legalization) but you're right that all-zeroes and all-ones is important. I suppose I'm a bit a paralyzed on what to do.

Though we may not having any example tests right now, this pattern can probably also be created by DAG.getConstant after type legalization due to the NodesMustHaveLegalTypes check.

Sorry for dropping the ball on this. I still haven't managed to look into it further. Is this a pattern that only shows up for all-zeros/all-ones, or is that the only cases that this patch is interested in?

Just wondering: instead of generating this bitcast/v2Xi32 pattern, could we lower this to SPLAT_VECTOR_PARTS and teach LLVM about this node a little better where required? I feel like the last discussion I read was that SPLATs should be open to fixed-length vectors too. That would at least keep it generic for a little longer, and catch all kinds of splat rather than just ones and zeros.

I'm happy to look into this further if you think it's worthwhile. It doesn't have to hold up this patch.

craig.topper abandoned this revision.Mon, Apr 19, 8:52 PM