This is an archive of the discontinued LLVM Phabricator instance.

[GlobalISel] Combine scalar unmerge(trunc)
ClosedPublic

Authored by gargaroff on May 7 2020, 4:29 AM.

Details

Summary

Combine unmerge(trunc) to enable other merge combines.
Without this combine, the scalar unmerge(trunc(merge))
pattern cannot be combined and easily lead to
hard-to-legalize merge/unmerge artifacts.

Diff Detail

Event Timeline

gargaroff created this revision.May 7 2020, 4:29 AM
Herald added a project: Restricted Project. · View Herald TranscriptMay 7 2020, 4:29 AM

Not sure whether this combine is desired. We need it for our out-of-tree backend, as otherwise there are quite a few cases with non-power-of-2 types which fail legalization due to non-combinable unmerge(trunc(merge)) patterns.

gargaroff updated this revision to Diff 263132.May 11 2020, 3:09 AM

Rebase.

I didn't bother to move your vector unmerge(trunc) combine to the new tryFoldUnmergeCast for now, @arsenm. I plan to do that after some initial feedback though

arsenm added inline comments.May 11 2020, 2:36 PM
llvm/include/llvm/CodeGen/GlobalISel/LegalizationArtifactCombiner.h
369

Needs example comment

370

Can set initial size to NewNumDefs and avoid push_back

529–531

Dead return

gargaroff marked 3 inline comments as done.

Add example comment
Set initial size to NewNumDefs
Remove dead return
Move vector unmerge(trunc) combine to tryFoldUnmergeCast

@arsenm Is this good to go or do you have any other feedback?

arsenm accepted this revision.May 28 2020, 6:55 AM
arsenm added inline comments.
llvm/include/llvm/CodeGen/GlobalISel/LegalizationArtifactCombiner.h
356–366

I think the diff is messed up and includes one of my recent patches now. I'm assuming much of this will disappear in the final version?

This revision is now accepted and ready to land.May 28 2020, 6:55 AM
gargaroff closed this revision.Jun 2 2020, 12:01 AM
gargaroff marked 2 inline comments as done.
llvm/include/llvm/CodeGen/GlobalISel/LegalizationArtifactCombiner.h
356–366

Kind-of. I extracted both of our code to a new function to keep the code more readable in tryCombineMerges and to make it more easy to add other merge-cast combines, which is why most of it appears in this diff now. I'm going to land that as a separate commit though.