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

[Amara Emerson via Phabricator via llvm-commits]

aemerson added inline comments.


================
Comment at: llvm/lib/CodeGen/GlobalISel/CombinerHelper.cpp:3681
+  if (!MRI.hasOneNonDBGUse(SrcVec) &&
+      !TLI.aggressivelyPreferBuildVectorSources(Ty))
+    return false;
----------------
paquette wrote:
> Would it be better to make a LLT variant of this function?
I'm not sure that's the right idea. Having 2 sets of hooks for everything sounds like a road we want to avoid as much as possible.


================
Comment at: llvm/lib/Target/AArch64/GISel/AArch64LegalizerInfo.cpp:642
+  getActionDefinitionsBuilder(G_BUILD_VECTOR_TRUNC)
+      .lower();
+
----------------
paquette wrote:
> Why no test for this?
Because it doesn't actually do anything. There's no lowering functionality implemented, but in order to test the combine we need something here that makes it seem like we have legalization support. So far I haven't seen this TRUNC variant generated in real code.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97835