[PATCH] D86458: GlobalISel: Artifact combine unmerge of unmerge

[Matt Arsenault via Phabricator via llvm-commits]

arsenm added inline comments.


================
Comment at: llvm/include/llvm/CodeGen/GlobalISel/LegalizationArtifactCombiner.h:548
+      // of an unmerge, this would involve the creation of an equivalent unmerge
+      // to copy back to the original result registers.
+      LegalizeActionStep ActionStep = LI.getAction(
----------------
aemerson wrote:
> I'm not sure what point this comment block is trying to get across? What do you mean "equivalent unmerge to copy back..."
> 
> ```// %0:_(<4 x s16>) = G_FOO
> // %1:_(<2 x s16>), %2:_(<2 x s16>) = G_UNMERGE_VALUES %0
> // %3:_(s16), %4:_(s16) = G_UNMERGE_VALUES %1
> //
> // %3:_(s16), %4:_(s16), %5:_(s16), %6:_(s16) = G_UNMERGE_VALUES %0
> ```
> For this case, the new unmerge is what this comment is referring to?
If you were to try to implement FewerElements for a G_UNMERGE_VALUES, you would end up producing the same G_UNMERGE_VALUES. This isn't produced here


================
Comment at: llvm/include/llvm/CodeGen/GlobalISel/LegalizationArtifactCombiner.h:569
+      // defs of the source unmerge are also unmerged, we end up with a separate
+      // unmerge for each one.
+      unsigned SrcDefIdx = getDefIndex(*SrcDef, SrcReg);
----------------
aemerson wrote:
> Is there a test that shows this case?
test_unmerge_values_s16_from_v3s16_from_v6s16 produces multiple identical unmerges


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

https://reviews.llvm.org/D86458