[PATCH] D80208: [CodeGen] Add support for extracting elements of scalable vectors

[David Sherwood via Phabricator via llvm-commits]

david-arm marked 2 inline comments as done.
david-arm added inline comments.


================
Comment at: llvm/lib/CodeGen/SelectionDAG/SelectionDAG.cpp:5431
+    // returns a vector type that is guaranteed to only have one element,
+    // which is the case for fixed width vectors.
     if (N1.getOpcode() == ISD::EXTRACT_SUBVECTOR &&
----------------
efriedma wrote:
> This comment doesn't seem quite right.
> 
> For fixed-width vectors, we only do this for vectors with exactly one element, but that's not because it's semantically incorrect for vectors with more than one element. It's easy to convert indexing on a subvector to indexing on the original vector.  The issue is just that it would make legalization more complex: we don't want to do the transform for vectors with more than one element without a profitability check.
OK fair enough. For the extract_vector_elt we're completely ignoring the index, which is fine for fixed width vectors, but not fine for scalable vectors, since we'd then need to generate code taking the index into account, which isn't worth it here. If the index > 0 for fixed width vectors the result is undefined anyway so we can simply return anything. I'll try to update the comment to be indicate this,


================
Comment at: llvm/test/CodeGen/AArch64/sve-extract-element.ll:171
+; CHECK-NEXT:    mov w8, #9
+; CHECK-NEXT:    whilels p0.d, #0, x8
+; CHECK-NEXT:    lastb d0, p0, z0.d
----------------
efriedma wrote:
> This doesn't assemble.  (I assume you meant to refer to xzr?)
OK, I'll take a look. I built and ran a test for variants of v16i8, but missed this,


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

https://reviews.llvm.org/D80208