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

[David Sherwood via Phabricator via llvm-commits]

david-arm added inline comments.


================
Comment at: llvm/lib/CodeGen/SelectionDAG/LegalizeVectorTypes.cpp:1512
   // Increment the pointer to the other part.
-  unsigned IncrementSize = LoVT.getSizeInBits() / 8;
+  unsigned IncrementSize = LoVT.getSizeInBits().getKnownMinSize() / 8;
   StackPtr =
----------------
It might be cleaner to do this now:

TypeSize IncrementSize = LoVT.getSizeInBits() / 8;

then pass that directly to getMemBasePlusOffset?


================
Comment at: llvm/lib/CodeGen/SelectionDAG/LegalizeVectorTypes.cpp:1520
   Hi = DAG.getLoad(HiVT, dl, Store, StackPtr,
                    PtrInfo.getWithOffset(IncrementSize), SmallestAlign);
 
----------------
I think this might be broken now since the offset doesn't make sense for scalable vectors. Maybe you can replace the DAG.getMemBasePlusOffset call with the new IncrementPointer helper function? That would also give you a MachinePointerInfo object that you can pass to the load here? I've also started making use of IncrementPointer in my patches too.


================
Comment at: llvm/lib/CodeGen/SelectionDAG/TargetLowering.cpp:7245
                       DAG.getConstant(EltSize, dl, IdxVT));
+
   return DAG.getMemBasePlusOffset(VecPtr, Index, dl);
----------------
nit: Stray new line here.


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

https://reviews.llvm.org/D84874