[PATCH] D120953: [AArch64][SelectionDAG] Supports unpklo/hi instructions to reduce the number of loads

[Paul Walker via Phabricator via llvm-commits]

paulwalker-arm added a comment.

Sorry for the delay.  Yes @david-arm that is what I meant.  The current set of exclusions looks incomplete, for example, based on the intent of the patch I'd suggest `nxv16i8 -> nxv16i32` also wants to be prevented.  So it seems safest to exclude all scalable vector extending loads/truncating stores and then selectively enable those which we directly support.  The first part of this is currently done for floating point scalable vector types so I think we can just replace the `fp_scalable_vector_valuetypes` iterators it uses with `scalable_vector_valuetypes` and remove the floating point related comments.

With that all said I now have a bigger concern with this patch.  Although the code quality for masked loads and stores is improved, the same is unlikely for the normal loads and store to which this test is also applied. I say this because for those cases no explicit unpacking is required (for the data or the predicate) but after this patch they'll started to be generated.

For example

  ptrue	p0.d
  ld1sw	{ z0.d }, p0/z, [x0]
  ld1sw	{ z1.d }, p0/z, [x0, #1, mul vl]

will become

  ptrue	p0.s
  ld1w	{ z1.s }, p0/z, [x0]
  sunpklo	z0.d, z1.s
  sunpkhi	z1.d, z1.s

I've just tried this patch and there's an even bigger problem in that marking these operations as Expand triggers fixed length specific code in DAGCombine that results in a compiler assert/crash when passed

  %wide.load = load <vscale x 4 x i32>, <vscale x 4 x i32>* %base
  %res = sext <vscale x 4 x i32> %wide.load to <vscale x 4 x i64>

This makes me think we first need to tighten up the handling of normal loads and stores to maintain the existing code quality and then apply the restrictions necessary for the masked varieties.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D120953/new/

https://reviews.llvm.org/D120953