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

[David Sherwood via Phabricator via llvm-commits]

david-arm added inline comments.


================
Comment at: llvm/lib/CodeGen/SelectionDAG/SelectionDAG.cpp:11603
+
+  if (PossibleStride.isZero())
+    return None;
----------------
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;


================
Comment at: llvm/lib/CodeGen/SelectionDAG/SelectionDAG.cpp:11606
+
+  for (const SDValue &Op : op_values()) {
+    if (!isa<ConstantSDNode>(Op))
----------------
Would this be more efficient (and possibly simpler?) if you just did:

  if (!ExpectedValue.isZero() || PossibleStride.isZero())
    return None;

  for (unsigned i = 2; i < getNumOperands(); i++) {
    if (!isa<ConstantSDNode>(getOperand(i)))
      return None

    APInt Val = getConstantOperandAPInt(i).trunc(EltSize);
    if (Val != (i * PossibleStride))
      return None;
  }



================
Comment at: llvm/lib/CodeGen/SelectionDAG/SelectionDAG.cpp:11610
+
+    APInt Val = cast<ConstantSDNode>(Op)->getAPIntValue().trunc(EltSize);
+    if (Val != ExpectedValue)
----------------
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?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125194