[PATCH] D102766: [SelectionDAG] Implement PromoteIntRes_INSERT_SUBVECTOR

[Peter Waller via Phabricator via llvm-commits]

peterwaller-arm added a comment.

Some suggestions.



================
Comment at: llvm/lib/CodeGen/SelectionDAG/LegalizeIntegerTypes.cpp:4704
+SDValue DAGTypeLegalizer::PromoteIntRes_INSERT_SUBVECTOR(SDNode *N) {
+
+  EVT OutVT = N->getValueType(0);
----------------
nit: Extraneous blank.


================
Comment at: llvm/lib/CodeGen/SelectionDAG/LegalizeIntegerTypes.cpp:4717
+
+  // Spill the vector to the stack.
+  Align SmallestAlign = DAG.getReducedAlign(SubVecVT, /*UseABI=*/false);
----------------
For me, the term 'spilling' is usually associated with running out of registers and needing to create register space by spilling them to the stack.

I think a comment here instead should express the intent of the code, something like "To insert SubVec into Vec, store the wider vector to memory, overwrite the lower half with the narrower vector, and reload". The other comments can probably be removed.



================
Comment at: llvm/test/CodeGen/AArch64/insert-subvector-res-legalization.ll:14
+; CHECK-NEXT:    .cfi_escape 0x0f, 0x0c, 0x8f, 0x00, 0x11, 0x10, 0x22, 0x11, 0x08, 0x92, 0x2e, 0x00, 0x1e, 0x22 // sp + 16 + 8 * VG
+; CHECK-NEXT:    .cfi_offset w29, -16
+; CHECK-NEXT:    ptrue p0.h
----------------
I think you can use -asm-verbose=0 in the run line to eliminate the CFI escapes.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D102766