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

[Kerry McLaughlin via Phabricator via llvm-commits]

kmclaughlin marked 2 inline comments as done.
kmclaughlin 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 =
----------------
david-arm wrote:
> It might be cleaner to do this now:
> 
> TypeSize IncrementSize = LoVT.getSizeInBits() / 8;
> 
> then pass that directly to getMemBasePlusOffset?
I used your suggestion below to replace the call to `getMemBasePlusOffset` with a call to the new `IncrementPointer` function, which removed the need to calculate IncrementSize here.


================
Comment at: llvm/lib/CodeGen/SelectionDAG/LegalizeVectorTypes.cpp:1520
   Hi = DAG.getLoad(HiVT, dl, Store, StackPtr,
                    PtrInfo.getWithOffset(IncrementSize), SmallestAlign);
 
----------------
david-arm wrote:
> 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.
I've replaced the call here as suggested, though the changes I've made in this patch to getMemBasePlusOffset are still necessary in ensuring the immediate is clamped correctly (it is used by getVectorElementPointer)


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

https://reviews.llvm.org/D84874