Page MenuHomePhabricator

[SelectionDAG] ComputeNumSignBits - support constant pool values from target
ClosedPublic

Authored by RKSimon on Jun 1 2019, 8:58 AM.

Details

Summary

As I mentioned on D61887 we don't get many hits on ComputeNumSignBits as we did on computeKnownBits.

The case we do get is interesting though - it allows us to use the 'ConditionalNegate' combine in combineLogicBlendIntoPBLENDV to remove a select.

It comes too late for SSE41 (BLENDV) cases, but SSE2 tests can hit it now. We should probably try to make use of this for SSE41+ targets as well - avoiding variable blends is usually a good idea. I'll investigate as a followup.

Diff Detail

Repository
rL LLVM

Event Timeline

RKSimon created this revision.Jun 1 2019, 8:58 AM
Herald added a project: Restricted Project. · View Herald TranscriptJun 1 2019, 8:58 AM
lebedev.ri accepted this revision.Jun 3 2019, 10:43 AM

This looks good to me, but i won't claim to know all the details here, so would be good for someone else to confirm.

lib/CodeGen/SelectionDAG/SelectionDAG.cpp
3882 ↗(On Diff #202556)

I realize this is not a new comment, but why '17'?
Does this only load 16-bit and extends to 32-bit?

3882–3887 ↗(On Diff #202556)

Precommit formatting-only changes?

3893 ↗(On Diff #202556)

Do you want to defer to scalar case if it's splat?

3911 ↗(On Diff #202556)
return 1; // Unhandled aggregate elt type. Conservatively assuming no bits match the sign bit.
This revision is now accepted and ready to land.Jun 3 2019, 10:43 AM
RKSimon added inline comments.Jun 3 2019, 11:19 AM
lib/CodeGen/SelectionDAG/SelectionDAG.cpp
3882 ↗(On Diff #202556)

Not limited but I agree its annoying that it doesn't make the context of the example clear.

3882–3887 ↗(On Diff #202556)

Sure.

3893 ↗(On Diff #202556)

Tricky - I went for handling it here as it makes it clearer whats going on, and is probably more performant than adding calls to the splat detection code and computeKnownBits

This revision was automatically updated to reflect the committed changes.