This is an archive of the discontinued LLVM Phabricator instance.

GlobalISel: Artifact combine merge-like and unmerge into copy
ClosedPublic

Authored by Petar.Avramovic on Sep 3 2021, 9:19 AM.

Details

Summary

Recognize copy that is represented as split of a source register to
elements that were reassembled to another register with the same type.

Diff Detail

Event Timeline

Petar.Avramovic created this revision.Sep 3 2021, 9:19 AM
Petar.Avramovic requested review of this revision.Sep 3 2021, 9:19 AM
Herald added a project: Restricted Project. · View Herald TranscriptSep 3 2021, 9:19 AM
Petar.Avramovic retitled this revision from [GlobalISel] Build_vector artifact combine into copy to GlobalISel: Artifact combine merge-like opcode and unmerge into copy.
Petar.Avramovic edited the summary of this revision. (Show Details)
arsenm added inline comments.Jan 16 2022, 5:54 PM
llvm/include/llvm/CodeGen/GlobalISel/LegalizationArtifactCombiner.h
1081

Should this be merged in with ArtifactValueFinder?

llvm/include/llvm/CodeGen/GlobalISel/LegalizationArtifactCombiner.h
1081

Do you mean whole function or some parts?
To me this looks similar to other tryCombine functions.
build_vector combines should be very simple, two adjacent instructions starting from build_vector check for elements shuffles. ArtifactValueFinder does advanced look through calculations (mainly through G_INSERT).
@aemerson What do you think?

arsenm added inline comments.Jan 17 2022, 3:35 PM
llvm/include/llvm/CodeGen/GlobalISel/LegalizationArtifactCombiner.h
1081

I thought the plan was to move everything toward using ArtifactValueFinder, rather than continuing to try to add code to parse every permutation of artifacts

Herald added a project: Restricted Project. · View Herald TranscriptSep 29 2022, 8:03 AM
Herald added a subscriber: kosarev. · View Herald Transcript
arsenm accepted this revision.Sep 29 2022, 12:17 PM

LGTM in the name of making progress. We do need to start moving all of this stuff into ArtifactValueFinder though

llvm/include/llvm/CodeGen/GlobalISel/LegalizationArtifactCombiner.h
1085

Move this below to where instructions could actually be inserted

This revision is now accepted and ready to land.Sep 29 2022, 12:17 PM

We do need to start moving all of this stuff into ArtifactValueFinder though

Like this?

We do need to start moving all of this stuff into ArtifactValueFinder though

Like this?

You moved the code, but didn't make the fundamental change. The key point is to start using findValueFromDef to find the non-artifact source instead of trying to handle every permutation of legalization artifacts

I understand now. Could I add a look through bitcast in findValueFromDefImpl? I would expect similar result as in D117328.

I understand now. Could I add a look through bitcast in findValueFromDefImpl? I would expect similar result as in D117328.

I'd leave that for a separate patch. I'm still not sure about treating bitcast as an artifact

Petar.Avramovic retitled this revision from GlobalISel: Artifact combine merge-like opcode and unmerge into copy to GlobalISel: Artifact combine merge-like and unmerge into copy.

Use Value finder to search for unmerged elements. In some tests skips a few combine steps since value finder looks through artifacts.

arsenm accepted this revision.Oct 21 2022, 8:44 AM
This revision was landed with ongoing or failed builds.Oct 24 2022, 4:33 AM
This revision was automatically updated to reflect the committed changes.

Sorry I missed this @, was on parental leave at the time. This is a very nice improvement in code quality, thanks!