[PATCH] D106028: [AArch64][SelectionDAG] Add legalization for widening LOAD/MLOAD.

Mon Jul 19 13:34:58 PDT 2021

efriedma added inline comments.

================
Comment at: llvm/lib/CodeGen/SelectionDAG/LegalizeVectorTypes.cpp:4048
+
+  // FIXME: Figure out how to replace constant "2".
+  if (VT.isScalableVector() && VT.getVectorMinNumElements() % 2 != 0) {
----------------
sdesmalen wrote:
> Do we also want to do this for other element-counts that are not a power of 2? (e.g. `<vscale x 6 x i8>`)
> 
> Was this code intended to work for `<vscale x 1 x eltty>` ? (I tried that a few days ago with this patch and ran into some failures. I didn't really investigate further though)
Should work for 1, I guess?  Not sure I actually tried it.  But note we only support a very limited set of operations.  See also D105591.

For `<vscale x 6 x i8>`, we can use a different sequence: generate extloads for `<vscale x 2 x i64>` and `<vscale x 4 x i32>`, and merge them together.  GenWidenVectorLoads should handle that in theory; not sure how well it works in practice.

I'll add more testcases, in any case.

================
Comment at: llvm/lib/CodeGen/SelectionDAG/LegalizeVectorTypes.cpp:4135
+  MachineMemOperand *MemOp = MF.getMachineMemOperand(
+      N->getMemOperand(), 0, MemoryLocation::UnknownSize);
+
----------------
sdesmalen wrote:
> Should `MemoryLocation::UnknownSize` only be used for the scalable case?
There's a general restriction on memoperands in selectiondag: the size of the operand must be at least the size of the memory VT.  I didn't really want to think about precisely what MemoryLocation we could be using instead...

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D106028