[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/CombinerHelper.h:96-103
+  /// If \p MI is extend that consumes the result of a extract_vector_elt,
+  /// try to combine it. Returns true if MI changed.
+  bool tryCombineExtendingVectorExtracts(MachineInstr &MI);
+  bool matchCombineExtendingVectorExtracts(MachineInstr &MI,
+                                           PreferredTuple &MatchInfo);
+  void applyCombineExtendingVectorExtracts(MachineInstr &MI,
+                                           PreferredTuple &MatchInfo);
----------------
This belongs in a separate patch 


================
Comment at: llvm/lib/CodeGen/GlobalISel/CombinerHelper.cpp:598-604
+  unsigned PreferredOpcode =
+     (MI.getOpcode() == TargetOpcode::G_ANYEXT_EXTRACT_VECTOR_ELT ||
+       MI.getOpcode() == TargetOpcode::G_EXTRACT_VECTOR_ELT)
+          ? TargetOpcode::G_ANYEXT
+          : MI.getOpcode() == TargetOpcode::G_SEXT_EXTRACT_VECTOR_ELT
+                ? TargetOpcode::G_SEXT
+                : TargetOpcode::G_ZEXT;
----------------
This is too ugly and needs to be broken up


================
Comment at: llvm/lib/CodeGen/GlobalISel/CombinerHelper.cpp:655-659
+      Preferred.ExtendOpcode == TargetOpcode::G_SEXT
+          ? TargetOpcode::G_SEXT_EXTRACT_VECTOR_ELT
+          : Preferred.ExtendOpcode == TargetOpcode::G_ZEXT
+                ? TargetOpcode::G_ZEXT_EXTRACT_VECTOR_ELT
+                : TargetOpcode::G_ANYEXT_EXTRACT_VECTOR_ELT));
----------------
Ditto


================
Comment at: llvm/lib/CodeGen/GlobalISel/CombinerHelper.cpp:664
+  SmallVector<MachineOperand *, 4> Uses;
+  for (auto &UseMO : MRI.use_operands(ExtractValue.getReg()))
+    Uses.push_back(&UseMO);
----------------
use_nodbg?


================
Comment at: llvm/lib/CodeGen/GlobalISel/CombinerHelper.cpp:675
+      Register UseDstReg = UseMI->getOperand(0).getReg();
+      MachineOperand &UseSrcMO = UseMI->getOperand(1);
+      const LLT &UseDstTy = MRI.getType(UseDstReg);
----------------
No reference


================
Comment at: llvm/lib/Target/Mips/MipsLegalizerInfo.cpp:105-108
+        if (ST.hasMSA() && CheckTyN(0, Query, {v16s8, v8s16, v4s32, v2s64}) &&
+            CheckTyN(1, Query, {s32, s64}) && CheckTyN(2, Query, {s32}))
+          return true;
+        return false;
----------------
return the logical expression


================
Comment at: llvm/lib/Target/Mips/MipsLegalizerInfo.cpp:114-115
+      .legalIf([=, &ST](const LegalityQuery &Query) {
+        if (ST.hasMSA() && CheckTyN(0, Query, {v16s8, v8s16}) &&
+            CheckTyN(1, Query, {s32}) && CheckTyN(2, Query, {s32}))
+          return true;
----------------
Ditto


================
Comment at: llvm/lib/Target/Mips/MipsLegalizerInfo.cpp:122-125
+        if (ST.hasMSA() && CheckTyN(0, Query, {s32, s64}) &&
+            CheckTyN(1, Query, {v16s8, v8s16, v4s32, v2s64}) &&
+            CheckTyN(2, Query, {s32}))
+          return true;
----------------
Ditto x3 more


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

https://reviews.llvm.org/D70858