[PATCH] D93416: [GlobalISel] Map extractelt to G_EXTRACT_VECTOR_ELT

Mon Jan 11 15:48:20 PST 2021

bjope added inline comments.

================
Comment at: llvm/lib/Target/AArch64/AArch64InstrGISel.td:167-168

+def : GINodeEquiv<G_EXTRACT_VECTOR_ELT, vector_extract>;
+
 // These are patterns that we only use for GlobalISel via the importer.
----------------
dsanders wrote:
> I'd rather this stay in SelectionDAGCompat.td. Having it there doesn't block backends migrating and doesn't hinder backends that have already migrated, whereas moving it to target specific files breaks any in-tree or downstream target that hasn't migrated yet.
I think it will be easier to remove the obsolete ISD nodes if it is obvious that for example only AArch64 depends on it. No in-tree target seem to have broken (or else I'd assume that they should have implemented some lit-tests verifying that vector_exract works).

Although, I'm a bit confused, as your answer in llvm-dev discussion indicated that there could be problems if defining both a relation for vector_extract and one for extractelt for the same target. So I thought it was a bit safer to only have the non-obsolete extractelt in SelectionDAGCompat.td. If this actually do cause problems for AArch64 then I guess it is time to migrate away from using the obsolete ISD node (or we can't have neither mapping in SelectionDAGCompat.td.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93416