This is an archive of the discontinued LLVM Phabricator instance.

[ValueTracking] Add DemandedElts support to computeKnownBits/ComputeNumSignBits (PR36319)
ClosedPublic

Authored by RKSimon on Jan 26 2020, 11:25 AM.

Details

Summary

This patch adds initial support for a DemandedElts mask to the internal computeKnownBits/ComputeNumSignBits methods, matching the SelectionDAG and GlobalISel equivalents.

So far only a couple of instructions have been setup to handle the DemandedElts, the remainder still using the existing 'all elements' default. The plan is to extend support as we have test coverage.

Diff Detail

Event Timeline

RKSimon created this revision.Jan 26 2020, 11:25 AM
Herald added a project: Restricted Project. · View Herald TranscriptJan 26 2020, 11:25 AM
Herald added a subscriber: hiraditya. · View Herald Transcript
jdoerfert added inline comments.Jan 30 2020, 4:08 PM
llvm/lib/Analysis/ValueTracking.cpp
1819–1820

Do we need to update the comment here that talks about vectors given that there is another kind of "vector query" now possible.

2848–2852

The above loop is now duplicated in this file, right? If so, maybe a helper is preferable. Given the existing TODO above having 3 version of the code seems definitively too many.

RKSimon updated this revision to Diff 241704.Jan 31 2020, 5:22 AM

Pull out getShuffleDemandedElts helper.
Add DemandedElts description to comments.

RKSimon marked 2 inline comments as done.Jan 31 2020, 5:22 AM
lebedev.ri added inline comments.Jan 31 2020, 10:34 AM
llvm/lib/Analysis/ValueTracking.cpp
1729–1732

This case is really hard to follow.
This may be missing some comments.

1748–1749

If Idx is not a constant, or out-of-bounds, why do we still computeKnownBits() of Elt?
If Idx is in-bounds constant, why do we call computeKnownBits() only on Elt?

RKSimon marked an inline comment as done.Jan 31 2020, 11:07 AM
RKSimon added inline comments.
llvm/lib/Analysis/ValueTracking.cpp
1748–1749

If Idx is not a constant, or out-of-bounds, why do we still computeKnownBits() of Elt?

OK, I'll change this to early out.

If Idx is in-bounds constant, why do we call computeKnownBits() only on Elt?

We don't - as long as its demanded (DemandedVal == true) we check Elt first, early out if we don't know anything, and then check the base vector as well (if it has any remaining demanded elts) and merge the common known bits.

RKSimon updated this revision to Diff 241778.Jan 31 2020, 11:19 AM

InsertElementInst - clean up description and early-out for unknown/out-of-range insertion indices.

lebedev.ri added inline comments.Jan 31 2020, 11:27 AM
llvm/lib/Analysis/ValueTracking.cpp
174–175

Wait, don't you want DemandedElts[i] here?

RKSimon marked an inline comment as done.Jan 31 2020, 12:35 PM
RKSimon added inline comments.
llvm/lib/Analysis/ValueTracking.cpp
174–175

Nice catch!

RKSimon updated this revision to Diff 241869.Feb 1 2020, 1:56 AM

Fix getShuffleDemandedElts typo

lebedev.ri accepted this revision.Feb 1 2020, 2:07 AM

This seems reasonable to me.

This revision is now accepted and ready to land.Feb 1 2020, 2:07 AM
This revision was automatically updated to reflect the committed changes.