This is an archive of the discontinued LLVM Phabricator instance.

[Support] improve known bits analysis for multiply by power-of-2 (1 set bit)
ClosedPublic

Authored by spatel on Dec 2 2021, 8:35 AM.

Details

Summary

This can be viewed as recognizing that multiply-by-power-of-2 doesn't have a carry into the top bit of an M-bit * N-bit number.

Enhancing canonicalization of mul -> select might also handle some of these if we were ok with increasing instruction count with casts in some cases.

This doesn't help https://llvm.org/PR49055 , but it's a simpler pattern that we miss.
Note: "-sccp" already gets these examples using a constant range analysis.

Diff Detail

Event Timeline

spatel created this revision.Dec 2 2021, 8:35 AM
spatel requested review of this revision.Dec 2 2021, 8:35 AM
Herald added a project: Restricted Project. · View Herald TranscriptDec 2 2021, 8:35 AM
efriedma added inline comments.
llvm/lib/Support/KnownBits.cpp
442

You could generalize further in two ways:

  1. if one operand is known to be 0 or 1, we can copy all the known zero bits from the other operand to the result, not just the leading zeros.
  2. If either operand has at most one bit set, we can use LeadZ = std::max(LHSLeadZ + RHSLeadZ + 1, BitWidth) - BitWidth;.

Not sure either one is actually useful, though.

spatel added inline comments.Dec 2 2021, 2:46 PM
llvm/lib/Support/KnownBits.cpp
442

Thanks, Eli!
If we all bits are known, then we should simplify before we get here?
I like the 2nd idea - that might get me closer to the solution for the motivating problem, and if I'm seeing it correctly, it can be implemented with just one extra line of code. Let me come up with some more tests to exercise that...

spatel added inline comments.Dec 2 2021, 2:48 PM
llvm/lib/Support/KnownBits.cpp
442

Disregard the question - I misread the suggestion.

spatel updated this revision to Diff 391659.Dec 3 2021, 9:33 AM
spatel added reviewers: rampitec, foad, arsenm.

Patch updated:
Generalized to power-of-2 (one possibly set bit) and added tests to exercise.
This turned up a couple of AMDGPU codegen diffs because we use knownbits down there. Those look like improvements to me, but adding some more potential reviewers to confirm.

spatel retitled this revision from [Support] improve known bits analysis for multiply with 1-bit op (bool) to [Support] improve known bits analysis for multiply by power-of-2 (1 set bit).Dec 3 2021, 9:45 AM
spatel edited the summary of this revision. (Show Details)
spatel added a reviewer: efriedma.

AMDGPU changes are progressions. Thanks!

LGTM - @efriedma any more comments?

Is there an exhaustive test for this method?

Is there an exhaustive test for this method?

I found this:
https://github.com/llvm/llvm-project/blob/e9fae0f19eec1fce746101b410d2345f0fbf89b4/llvm/unittests/Support/KnownBitsTest.cpp#L233

And it did catch a bug in an early draft of this patch, so it seems to be working.

Is there an exhaustive test for this method?

Yes - in KnownBitsTest BinaryExhaustive

lebedev.ri accepted this revision.Dec 7 2021, 11:26 AM

LGTM, thank you.

This revision is now accepted and ready to land.Dec 7 2021, 11:26 AM
This revision was landed with ongoing or failed builds.Dec 8 2021, 8:50 AM
This revision was automatically updated to reflect the committed changes.