[PATCH] D70858: [GlobalISel][RFC] Importing insert/extract vector element patterns

[Petar Avramovic via Phabricator via llvm-commits]

Petar.Avramovic marked 4 inline comments as done.
Petar.Avramovic added inline comments.


================
Comment at: llvm/include/llvm/CodeGen/GlobalISel/LegalizerInfo.h:60-65
+  /// The operation should be implemented in terms of a wider scalar
+  /// base-type. Used for vector truncating insert or extending extract.
+  /// Changes opcode.
+  WidenVectorInsOrExtScalar,
+
   /// The (vector) operation should be implemented by splitting it into
----------------
arsenm wrote:
> Why is a new legalize action needed? Why can't this just specify the existing action on the scalar operand?
I thought that targets might need different widen scalar of `elt` operand in  vector insert extract. Now that I think of it custom should do just fine since it is action of only 2 opcodes. Did you have other existing action in mind ?


================
Comment at: llvm/include/llvm/Support/TargetOpcodes.def:570
+/// Generic extractelement with anyextend.
+HANDLE_TARGET_OPCODE(G_ANYEXT_EXTRACT_VECTOR_ELT)
+
----------------
arsenm wrote:
> Can you split out the new opcode handling into a separate patch?
Sure, will wait for your patches in emitter  first.


================
Comment at: llvm/utils/TableGen/CodeGenRegisters.cpp:1011-1014
+  // It's ok to have a few classes with same size as BiggestSuperRegRC.
+  assert(SuperRegRCs.front()->getMembers().size() ==
+             BiggestSuperRegRC->getMembers().size() &&
+         "Biggest class wasn't first");
----------------
arsenm wrote:
> I have a different workaround for this in D72771
Looks great.


================
Comment at: llvm/utils/TableGen/GlobalISelEmitter.cpp:2569
 
+class TempSubRegRenderer : public OperandRenderer {
+protected:
----------------
arsenm wrote:
> I reproduced approximately the same thing in D72788
I see, then iptr immediate index is all that is left in this patch concerning emitter.


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

https://reviews.llvm.org/D70858