[PATCH] D84754: GlobalISel: Implement fewerElementsVector for G_INSERT_VECTOR_ELT

[Amara Emerson via Phabricator via llvm-commits]

aemerson added inline comments.


================
Comment at: llvm/include/llvm/CodeGen/GlobalISel/LegalizerHelper.h:277
+  LegalizeResult fewerElementsVectorExtractInsertVectorElt(MachineInstr &MI,
+                                                           unsigned TypeIdx,
+                                                           LLT NarrowTy);
----------------
arsenm wrote:
> paquette wrote:
> > phab is complaining about these (>>), are they tabs? clang format?
> I don't think this is a complaint. It's definitely not a tab. Maybe this is just marking the line didn't change, only the indentation point?
Yeah I think that's it.


================
Comment at: llvm/lib/CodeGen/GlobalISel/LegalizerHelper.cpp:3311
+    assert(TypeIdx == 1 && "not a vector type index");
+  }
+
----------------
Can fold these into just a single assert: `assert(IsInsert ? TypeIdx == 0 : TypeIdx == 1)`


================
Comment at: llvm/lib/CodeGen/GlobalISel/LegalizerHelper.cpp:3349
+
+      auto InsertPart = MIRBuilder.buildInsertVectorElement(
+          PartTy, VecParts[PartIdx], InsertVal, NewIdx);
----------------
A comment in this block explaining the approach would be helpful.


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

https://reviews.llvm.org/D84754