This is an archive of the discontinued LLVM Phabricator instance.

[DAGCombiner] Fold `ty1 extract_vector(ty2 splat(V)) -> ty1 splat(V)`
ClosedPublic

Authored by sdesmalen on Feb 3 2022, 9:13 AM.

Details

Summary

This seems like an obvious fold, which leads to a few improvements.

Diff Detail

Event Timeline

sdesmalen created this revision.Feb 3 2022, 9:13 AM
sdesmalen requested review of this revision.Feb 3 2022, 9:13 AM
craig.topper retitled this revision from [DAGCombiner] Fold `ty1 extract_vector(ty2 splat(V)) -> t1 splat(V)` to [DAGCombiner] Fold `ty1 extract_vector(ty2 splat(V)) -> ty1 splat(V)`.Feb 3 2022, 9:15 AM
frasercrmck added inline comments.Feb 4 2022, 8:25 AM
llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp
21113

I'm wondering, do we really need both to be equally scalable? Or is this just to avoid introducing a fixed-length SPLAT_VECTOR which we don't have great support for? If it's a "workaround" of that sort, might be worth commenting.

llvm/test/CodeGen/RISCV/rvv/vadd-vp.ll
1574–1575

We can remove this FIXME.

sdesmalen updated this revision to Diff 406177.Feb 5 2022, 6:03 AM

Removed condition that result/input vector must match scalable property.

sdesmalen marked an inline comment as done.Feb 5 2022, 6:03 AM
sdesmalen added inline comments.
llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp
21113

Good point, I was mostly trying to be conservative, but when removing the condition everything still works as expected.

david-arm accepted this revision.Feb 7 2022, 2:30 AM

LGTM!

llvm/test/CodeGen/AArch64/sve-extract-fixed-vector.ll
427

Is it worth having a fixed-width and scalable vector test when extracting a v1i32 or nxv1i32?

This revision is now accepted and ready to land.Feb 7 2022, 2:30 AM
This revision was landed with ongoing or failed builds.Feb 9 2022, 6:31 AM
This revision was automatically updated to reflect the committed changes.
sdesmalen added inline comments.Feb 9 2022, 6:36 AM
llvm/test/CodeGen/AArch64/sve-extract-fixed-vector.ll
427

I'm not sure if that would be testing anything in this patch specifically.