Page MenuHomePhabricator

[InstCombine] Allow values with multiple users in SimplifyDemandedVectorElts
Needs ReviewPublic

Authored by piotr on Sep 9 2019, 3:46 AM.

Details

Summary

Allow for ignoring the check for a single use in SimplifyDemandedVectorElts
to be able to simplify operands if DemandedElts is known to contain
the union of elements used by all users.
It is a responsibility of a caller of SimplifyDemandedVectorElts to
supply correct DemandedElts.

Simplify a series of extractelement instructions if only a subset of
elements is used.

Diff Detail

Event Timeline

piotr created this revision.Sep 9 2019, 3:46 AM
Herald added a project: Restricted Project. · View Herald TranscriptSep 9 2019, 3:46 AM
arsenm added inline comments.Sep 9 2019, 8:17 AM
lib/Transforms/InstCombine/InstCombineVectorOps.cpp
358

dyn_cast_or_null

arsenm added inline comments.Sep 9 2019, 8:21 AM
lib/Transforms/InstCombine/InstCombineVectorOps.cpp
358

Nevermind, this isn't quite that

This is a useful change, but there is an unfortunate asymmetry here in how the code is structured: in addition to extractelement, we could also have shufflevector users (or masked stores etc.). Presumably we'd be able to handle all of those together without duplicating the code. Is there a way to take this into account?

Also, two comments inline.

lib/Transforms/InstCombine/InstCombineVectorOps.cpp
360

ult instead of ule.

368–369

Skip this if DemandedElts is allOnes.

piotr added a comment.Tue, Oct 1, 5:02 AM

This is a useful change, but there is an unfortunate asymmetry here in how the code is structured: in addition to extractelement, we could also have shufflevector users (or masked stores etc.). Presumably we'd be able to handle all of those together without duplicating the code. Is there a way to take this into account?

Also, two comments inline.

Good point, I will try extending the patch to also handle shufflevector.

piotr updated this revision to Diff 225165.Wed, Oct 16, 12:04 AM

As suggested by Nicolai, I extended the patch to also handle shufflevector users in addition to extractelement. The new generalized version also simplifies the case where three extract elements are used (the previous patch only handled two). The new code also makes it easy to extend the case list even more in the future.