This is an archive of the discontinued LLVM Phabricator instance.

[SCEV] Strengthen huge constant trip multiples.
ClosedPublic

Authored by caojoshua on Apr 8 2023, 6:15 PM.

Details

Summary

SCEV determines that loops with trip count >=2^32 have a trip multiple
of 1 to guard against huge multiples. This patch stregthens this to
instead find the greatest power of 2 divisor that is less than the
threshold.

Diff Detail

Event Timeline

caojoshua created this revision.Apr 8 2023, 6:15 PM
Herald added a project: Restricted Project. · View Herald TranscriptApr 8 2023, 6:15 PM
caojoshua updated this revision to Diff 511942.Apr 8 2023, 6:21 PM

code cleanup

We could technically find the largest multiple under the threshold that is not limited to powers of 2, but that is much more computation heavy and the benefits are questionable.

An interesting case where this is beneficial is in the LoopUnroll test. Before, we returned a trip multiple of 1, but this change returns a trip multiple of 2^32. Since its trying to unroll by a factor of 8, and 2^32 is divisible by eight, LoopUnroll knows that the loop epilogue is not required anymore.

caojoshua published this revision for review.Apr 8 2023, 6:29 PM
Herald added a project: Restricted Project. · View Herald TranscriptApr 8 2023, 6:29 PM
efriedma added inline comments.
llvm/lib/Analysis/ScalarEvolution.cpp
8250

If the Result is zero, countTrailingZeros() will return the bitwidth of the integer, I think, which doesn't match the comment.

I don't think the wrapping thing can actually happen since D110587 was merged, though, so we shouldn't see a trip count of zero. So maybe update the comment while you're here.

(Maybe it's theoretically possible that some combination of folding could end up folding a poison trip count to zero. In that case, though, it wouldn't really matter what multiple we return.)

Don't allow trip counts of zero.

caojoshua marked an inline comment as done.Apr 10 2023, 12:12 AM
caojoshua added inline comments.
llvm/lib/Analysis/ScalarEvolution.cpp
8250

You're right, TripCount should never be zero. I added an assertion for this and removed that section of the comment.

nikic accepted this revision.Apr 10 2023, 12:44 AM

LGTM

This revision is now accepted and ready to land.Apr 10 2023, 12:44 AM
caojoshua updated this revision to Diff 512321.Apr 10 2023, 8:10 PM
caojoshua marked an inline comment as done.

rebase on top of main

This revision was landed with ongoing or failed builds.Apr 10 2023, 8:12 PM
This revision was automatically updated to reflect the committed changes.