[PATCH] D56957: GlobalISel: Handle some odd splits in fewerElementsVector

[Matt Arsenault via Phabricator via llvm-commits]

arsenm marked an inline comment as done.
arsenm added inline comments.


================
Comment at: lib/CodeGen/GlobalISel/LegalizerHelper.cpp:1398
+    for (unsigned Offset = 0; Offset < BitsForNumParts; Offset += NarrowSize) {
+      SmallVector<SrcOp, 4> SrcOps;
+      for (unsigned I = 1, E = MI.getNumOperands(); I != E; ++I) {
----------------
dsanders wrote:
> arsenm wrote:
> > dsanders wrote:
> > > Hi Matt,
> > > 
> > > I just came across the various uses of SrcOp in LegalizerHelper and thought I should mention that this type wasn't really meant to be used outside of MachineIRBuilder and its specializations. It's a bit of machinery to be able to factor out the common overloads.
> > > 
> > > What was the reason for introducing the SrcOp variables? Does MachineOperand or registers achieve the same end result?
> > I think this was because you can't implicitly convert from SmallVector<unsigned> to ArrayRef<SrcOp> for the final use
> That makes sense, we should be able to add a conversion for that. In later commits there's also some SrcOp variables that aren't in a container. What was the reason for those?
In the narrowScalarShiftByConstant for example, I think this was to have an uninitialized variable that the later build calls would set from the MachineInstrBuilder results. It would be a pain if this was a register value, since you don't really want to be referring to registers at all.


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

https://reviews.llvm.org/D56957