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

[Matt Arsenault via Phabricator via llvm-commits]

arsenm added inline comments.


================
Comment at: llvm/include/llvm/CodeGen/GlobalISel/InstructionSelectorImpl.h:583
+        MachineFunction *MF = State.MIs[InsnID]->getParent()->getParent();
+        const unsigned AddrSpace = Ty.isPointer() ? Ty.getAddressSpace() : 0;
+        SizeInBits = MF->getDataLayout().getPointerSizeInBits(AddrSpace);
----------------
I think this should assert isPointer. Using 0 address space if it's not a pointer is nonsensical


================
Comment at: llvm/include/llvm/Target/GenericOpcodes.td:1027-1028
 
+// Generic extractelement with signextend.
+def G_SEXT_EXTRACT_VECTOR_ELT : GenericInstruction {
+  let OutOperandList = (outs type0:$dst);
----------------
Adding these is a separate patch


================
Comment at: llvm/include/llvm/Target/GlobalISel/SelectionDAGCompat.td:120
 def : GINodeEquiv<G_FMAXNUM_IEEE, fmaxnum_ieee>;
+def : GINodeEquiv<G_INSERT_VECTOR_ELT, vector_insert>;
+def : GINodeEquiv<G_EXTRACT_VECTOR_ELT, vector_extract>;
----------------
I think there should be no compatibility for vector_insert/vector_extract see this comment:
// vector_extract/vector_insert are deprecated. extractelt/insertelt
// are preferred.

Really we should migrate all the uses away from the old relaxed nodes


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70858