This is an archive of the discontinued LLVM Phabricator instance.

[AArch64][SelectionDAG] Refactor to support more scalable vector extending loads
ClosedPublic

Authored by Allen on Mar 22 2022, 8:24 PM.

Details

Summary

Accord the discussion in D120953, we should firstly exclude all scalable vector
extending loads and then selectively enable those which we directly support.

This patch is intend to refactor for above (truncating stores is not touched),and
more scalable vector types will try to reduce the number of masked loads in favour
of more unpklo/hi instructions.

Diff Detail

Event Timeline

Allen created this revision.Mar 22 2022, 8:24 PM
Allen requested review of this revision.Mar 22 2022, 8:24 PM
Herald added a project: Restricted Project. · View Herald TranscriptMar 22 2022, 8:24 PM
llvm/lib/Target/AArch64/AArch64ISelLowering.cpp
1250

MVT::nxv2i32 is not a legal type and so shouldn't be listed here.

Allen edited the summary of this revision. (Show Details)Mar 24 2022, 6:54 PM
Allen marked an inline comment as done.Mar 26 2022, 9:00 AM

ping?

llvm/lib/Target/AArch64/AArch64ISelLowering.cpp
1250

I add type MVT::nxv2i32 because the case masked_zload_2i16_2f64 in CodeGen/AArch64/sve-masked-ldst-zext.ll.
If I don't set it Legal, then there will be an redundant and, the complete assemble is followed.

        ld1h	{ z0.d }, p0/z, [x0]
	ptrue	p1.d
	and	z0.d, z0.d, #0xffff   --- redundant ?
	ucvtf	z0.d, p1/m, z0.d

so both 64 and 128-bit vectors are supported extend ?

1250

hi, @paulwalker-arm:

do you think we need not define MVT::nxv2i32 legal, and eliminate the **and** with other method ?
Allen marked an inline comment as done.Mar 27 2022, 3:03 AM

hi, @paulwalker-arm

I find that without this refactor patch, the MVT::nxv2i32 type is also lega default, so the 'and' insn can be deleted in tryToFoldExtOfMaskedLoad.

then, if we set it expand, the condition !TLI.isLoadExtLegalOrCustom(ExtLoadType, VT, Ld->getValueType(0)) in above function tryToFoldExtOfMaskedLoad will prevent it from optimizing.
paulwalker-arm accepted this revision.Mar 27 2022, 3:58 AM
paulwalker-arm added inline comments.
llvm/lib/Target/AArch64/AArch64ISelLowering.cpp
1250

It does feel like we're missing an ISD::AND combine somewhere, but given you're just maintaining the existing behaviour I'll not worry about it for this patch. We can circle back to the MVT::nxv2i32 issue later.

This revision is now accepted and ready to land.Mar 27 2022, 3:58 AM
This revision was landed with ongoing or failed builds.Mar 27 2022, 6:20 AM
This revision was automatically updated to reflect the committed changes.