[PATCH] D109241: [GlobalISel] Build_vector artifact combine into unmerge_values

[Jay Foad via Phabricator via llvm-commits]

foad added inline comments.


================
Comment at: llvm/include/llvm/CodeGen/GlobalISel/LegalizationArtifactCombiner.h:1118
+
+      // Total size of elements in Unmerge before FirstElt has to be N x DstTy.
+      unsigned NumBitsBeforeFirstElt =
----------------
Wouldn't it be simpler to check that FirstEltIdx is a multiple of DstTy.getNumElements()?


================
Comment at: llvm/include/llvm/CodeGen/GlobalISel/LegalizationArtifactCombiner.h:1132
+      auto NewUnmerge = Builder.buildUnmerge(DstTy, Unmerge->getSourceReg());
+      unsigned DstIdx = NumBitsBeforeFirstElt / DstSize;
+      replaceRegOrBuildCopy(Dst, NewUnmerge.getReg(DstIdx), MRI, Builder,
----------------
... and `FirstEltIdx / DstTy.getNumElements()` here.


================
Comment at: llvm/lib/CodeGen/GlobalISel/MachineIRBuilder.cpp:616
   unsigned NumReg = Op.getLLTTy(*getMRI()).getSizeInBits() / Res.getSizeInBits();
-  SmallVector<Register, 8> TmpVec;
+  SmallVector<DstOp, 8> TmpVec;
   for (unsigned I = 0; I != NumReg; ++I)
----------------
Just use a constructor that initialized this to N copies of Res?


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

https://reviews.llvm.org/D109241