This is an archive of the discontinued LLVM Phabricator instance.

[GlobalISel] Enable artifact combiner to combine starting from a G_MERGE_VALUES
ClosedPublic

Authored by aemerson on Apr 10 2020, 10:52 PM.

Details

Summary

We generally only combine starting from users to defs in the artifact combiner, but this doesn't catch cases where at the point of combining a G_UNMERGE we don't yet have the opposite G_MERGE on input yet since we haven't legalized that far.

This change adds the users of a G_MERGE to the artifact combiner worklist if one of the uses is a G_UNMERGE.

Diff Detail

Event Timeline

aemerson created this revision.Apr 10 2020, 10:52 PM
gargaroff added inline comments.Apr 14 2020, 2:55 AM
llvm/include/llvm/CodeGen/GlobalISel/LegalizationArtifactCombiner.h
655

Should this skip COPYs?

656

The artifact combiner can also combine trunc(merge). Could you add trunc users to the worklist as well?

aemerson updated this revision to Diff 257547.Apr 14 2020, 3:58 PM

Add TRUNC users to the list.

aemerson marked an inline comment as done.Apr 14 2020, 4:00 PM
aemerson added inline comments.
llvm/include/llvm/CodeGen/GlobalISel/LegalizationArtifactCombiner.h
655

I don't think it's quite that simple. Here we may have multiple users, and if we skip through COPYs then we have to potentially search through a tree rather than just walking up a single chain. This could have a compile time impact.

gargaroff accepted this revision.Apr 15 2020, 3:44 AM

LGTM with the failing tests fixed.

This revision is now accepted and ready to land.Apr 15 2020, 3:44 AM
This revision was automatically updated to reflect the committed changes.