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

[Eli Friedman via Phabricator via llvm-commits]

efriedma added inline comments.


================
Comment at: llvm/lib/CodeGen/SelectionDAG/LegalizeVectorTypes.cpp:1510
+  StackPtr = DAG.getMemBasePlusOffset(StackPtr, IncrementSize, dl,
+                                      N->getFlags(), LoVT.isScalableVector());
 
----------------
kmclaughlin wrote:
> efriedma wrote:
> > Can we pass a TypeSize here, instead of passing the offset and scalable bit separately?
> Since TypeSize uses uint64_t, I thought it would be better to pass in IsScalable as a separate argument so that we can keep the Offset argument as a signed value.
We could introduce a dedicated TypeOffset type if you think that's really an issue.  I suspect getMemBasePlusOffset is rarely, if ever, used with a negative offset, though.  Maybe add an overload that takes a TypeSize and go from there.

I really want to avoid getKnownMinSize() where it isn't necessary; it's going to be a lot easier to make mistakes passing around possibly-scaled offsets as bare integers.


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

https://reviews.llvm.org/D84874