[PATCH] D84874: [SVE][CodeGen] Legalisation of INSERT_VECTOR_ELT for scalable vectors

[Eli Friedman via Phabricator via llvm-commits]

efriedma added a comment.

There's probably some argument for writing some aarch64-specific code to do a double-wide insert in registers, but maybe not common enough to be worth worrying about.



================
Comment at: llvm/lib/CodeGen/SelectionDAG/LegalizeVectorTypes.cpp:1499
+      commonAlignment(SmallestAlign,
+                      EltVT.getSizeInBits().getKnownMinSize() / 8));
 
----------------
getFixedSize()?


================
Comment at: llvm/lib/CodeGen/SelectionDAG/LegalizeVectorTypes.cpp:1510
+  StackPtr = DAG.getMemBasePlusOffset(StackPtr, IncrementSize, dl,
+                                      N->getFlags(), LoVT.isScalableVector());
 
----------------
Can we pass a TypeSize here, instead of passing the offset and scalable bit separately?


================
Comment at: llvm/lib/CodeGen/SelectionDAG/TargetLowering.cpp:7195
+    SDValue VS = DAG.getVScale(dl, IdxVT,
+                               APInt(IdxVT.getSizeInBits().getKnownMinSize(),
+                                     NElts));
----------------
getFixedSize()?


================
Comment at: llvm/lib/CodeGen/SelectionDAG/TargetLowering.cpp:7200
+
+    return DAG.getNode(ISD::UMIN, dl, IdxVT, Idx, Sub);
+  } else {
----------------
The non-scalable path uses masking; I guess you can't do that here because the element count might not be a power of two?  Given that, the end result here isn't great, but I can't come up with anything better.


================
Comment at: llvm/lib/CodeGen/SelectionDAG/TargetLowering.cpp:7225
+  unsigned EltSize = EltVT.getSizeInBits().getKnownMinSize() / 8; // FIXME: should be ABI size.
+  assert(EltSize * 8 == EltVT.getSizeInBits().getKnownMinSize() &&
          "Converting bits to bytes lost precision");
----------------
getFixedSize()?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84874