[PATCH] D125194: [SVE][SelectionDAG] Use INDEX to generate matching instances of BUILD_VECTOR.

[Paul Walker via Phabricator via llvm-commits]

paulwalker-arm added inline comments.


================
Comment at: llvm/lib/CodeGen/SelectionDAG/SelectionDAG.cpp:11603
+
+  if (PossibleStride.isZero())
+    return None;
----------------
david-arm wrote:
> Maybe I've missed something, but don't you also need to ensure the first value is zero too, i.e.
> 
>   if (!ExpectedValue.isZero() || PossibleStride.isZero())
>     return None;
I had thought the "starting at 0" restriction was baked in.  Given this is not the case I think it's likely just as easy to support it as disable it so I'll do the former and thus fix one of the missed optimisations I've highlighted with the tests.


================
Comment at: llvm/lib/CodeGen/SelectionDAG/SelectionDAG.cpp:11610
+
+    APInt Val = cast<ConstantSDNode>(Op)->getAPIntValue().trunc(EltSize);
+    if (Val != ExpectedValue)
----------------
david-arm wrote:
> I don't think there are any tests that exercise the case where we find a valid sequence as a result of truncating values to the element size. I'm imagining something like this:
> 
> <i64 0x4000000000000000, 0x4000000000000002, 0x4000000000000004, ...>
> 
> If it's not too much trouble is it possible to write a unit test for this case to make sure we do the right thing?
The `trunc` is really a quirk of type legalisation whereby a vector of i8s is legal but i8 scalars are not.  The example you're proposing is likely to be optimised away before we get this far.  The code itself is unconditional though so there is no untested code path.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125194