This is an archive of the discontinued LLVM Phabricator instance.

[DAGCombiner] Use SelectionDAG::isKnownToBeAPowerOfTwo instead of just APInt::isPowerOf2
ClosedPublic

Authored by RKSimon on Dec 13 2016, 7:31 AM.

Details

Summary

Generalize sdiv/udiv/srem/urem combines using APInt::isPowerOf2, which only works for const/splat-const values to call SelectionDAG::isKnownToBeAPowerOfTwo instead which recognises many more cases.

Added a DAGCombiner::BuildLogBase2 helper since PowerOf2 combines often involve taking the log2 of such a value.

Diff Detail

Repository
rL LLVM

Event Timeline

RKSimon updated this revision to Diff 81232.Dec 13 2016, 7:31 AM
RKSimon retitled this revision from to [DAGCombiner] Use SelectionDAG::isKnownToBeAPowerOfTwo instead of just APInt::isPowerOf2.
RKSimon updated this object.
RKSimon set the repository for this revision to rL LLVM.
RKSimon added a subscriber: llvm-commits.
andreadb accepted this revision.Dec 14 2016, 4:37 AM
andreadb edited edge metadata.

Hi Simon,

I only have two minor comments (see below). Otherwise the patch LGTM. Thanks!

About the udiv/urem/sdiv/srem tests. Would it be possible to add extra RUN lines for AVX (non AVX2)? You can probably do that in a separate commit.

Thanks,
Andrea

lib/CodeGen/SelectionDAG/DAGCombiner.cpp
2492–2502 ↗(On Diff #81232)

I noticed that you removed the check for opaque constants. Is that intentional?

This revision is now accepted and ready to land.Dec 14 2016, 4:37 AM
RKSimon added inline comments.Dec 14 2016, 6:23 AM
lib/CodeGen/SelectionDAG/DAGCombiner.cpp
2492–2502 ↗(On Diff #81232)

It made sense to replace the constant-only combine with a generic isPowerOfTwo call (which can detect many runtime cases as well) - no need for opaque constant detection.

andreadb added inline comments.Dec 14 2016, 6:41 AM
lib/CodeGen/SelectionDAG/DAGCombiner.cpp
2492–2502 ↗(On Diff #81232)

Thanks Simon. LGTM.

This revision was automatically updated to reflect the committed changes.