[PATCH] D104355: [GlobalISel] Add a new artifact combiner for unmerge which looks through general artifact expressions.

Wed Jun 30 16:50:22 PDT 2021

aemerson added a comment.

In D104355#2851666 <https://reviews.llvm.org/D104355#2851666>, @arsenm wrote:

> This seems like a much more holistic solution. What we have now isn't manageable to guarantee every possible permutation of artifacts can legalize.

Yeah, although combining extensions isn't really possible with this, so for now we still need some of the old combiner code around. I would like to use it more places though.

> I still think legalizing G_INSERT/G_EXTRACT (especially for vectors) is problematic and probably shouldn't be done. I view them now as a thin abstraction over subregister indexes, and not really useful for vector legalization as they are used now. Having the hack for now is fine since it's definitely impossible to make any progress on this sort of thing all at once.

+1. The sooner we can move away from those to merge/unmerge the better.

> Are you actively working on removing the old artifact combines?

Not yet, but maybe in future. The first thing we need to fix is the AMDGPU infinite loops, because those are blocking deploying this in more obvious places.

================
Comment at: llvm/lib/CodeGen/GlobalISel/LegalizerHelper.cpp:4795-4797
+LegalizerHelper::moreElementsVectorShuffle(MachineInstr &MI,
+                                           unsigned int TypeIdx, LLT MoreTy) {
+  if (TypeIdx != 0)
----------------
arsenm wrote:
> Can this be split to a separate patch?
It was the motivating example for the combine, but yes it can be separated.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D104355