[PATCH] D97835: [AArch64][GlobalISel] Add combine for extract_vector_elt(build_vector, cst)

[Jessica Paquette via Phabricator via llvm-commits]

paquette added inline comments.


================
Comment at: llvm/lib/CodeGen/GlobalISel/CombinerHelper.cpp:3664
+
+  if (!isLegalOrBeforeLegalizer({TargetOpcode::G_BUILD_VECTOR, SrcTy}))
+    return false;
----------------
I would guess this is faster than `getConstantVRegValWithLookThrough`? Maybe check this first?


================
Comment at: llvm/lib/CodeGen/GlobalISel/CombinerHelper.cpp:3672
+    BuildVecMI = getOpcodeDef(TargetOpcode::G_BUILD_VECTOR_TRUNC, SrcVec, MRI);
+    if (!BuildVecMI)
+      return false;
----------------
I guess this is slightly more concise


================
Comment at: llvm/lib/CodeGen/GlobalISel/CombinerHelper.cpp:3678
+
+  const auto &TLI = *MI.getMF()->getSubtarget().getTargetLowering();
+  EVT Ty(getMVTForLLT(SrcTy));
----------------
Can you use the `getTargetLowering()` helper function here?


================
Comment at: llvm/lib/CodeGen/GlobalISel/CombinerHelper.cpp:3681
+  if (!MRI.hasOneNonDBGUse(SrcVec) &&
+      !TLI.aggressivelyPreferBuildVectorSources(Ty))
+    return false;
----------------
Would it be better to make a LLT variant of this function?


================
Comment at: llvm/lib/Target/AArch64/GISel/AArch64LegalizerInfo.cpp:642
+  getActionDefinitionsBuilder(G_BUILD_VECTOR_TRUNC)
+      .lower();
+
----------------
Why no test for this?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97835