This is an archive of the discontinued LLVM Phabricator instance.

[TargetLowering] Teach BuildUDIV to take advantage of leading zeros in the dividend.
ClosedPublic

Authored by craig.topper on Dec 28 2022, 9:14 PM.

Details

Summary

If the dividend has leading zeros, we can use them to reduce the
size of the multiplier and avoid the fixup cases.

This patch is for scalars only, but we might be able to do this
for vectors in a follow up.

Diff Detail

Event Timeline

craig.topper created this revision.Dec 28 2022, 9:14 PM
craig.topper requested review of this revision.Dec 28 2022, 9:14 PM
Herald added a project: Restricted Project. · View Herald TranscriptDec 28 2022, 9:14 PM

Can you update the exhaustive test too so we know this is actually correct?

llvm/lib/CodeGen/SelectionDAG/TargetLowering.cpp
6025

Why not vectors?

Update exhaustive test. Add an assertion to check division by 1.

lebedev.ri added inline comments.Dec 29 2022, 11:54 AM
llvm/unittests/Support/DivisionByConstantTest.cpp
102–103

It happens first, so let's place it first

173
181–198
craig.topper added inline comments.Dec 29 2022, 11:55 AM
llvm/lib/CodeGen/SelectionDAG/TargetLowering.cpp
6025

Baby steps.

Wasn't sure if we need to apply the clip by divisor leading zeros uniformly across the whole vector when there are different divisors which would require another loop over the divisors. Or if we could treat each element individually and apply a clip inside BuildUDIVPattern.

craig.topper added inline comments.Dec 29 2022, 11:58 AM
llvm/unittests/Support/DivisionByConstantTest.cpp
181–198

Ok. I was trying to reduce test time since IsAdd is always false after we apply the leading zero optimization. But if you'd prefer to reduce the bit width instead we can do that.

Address review comments

lebedev.ri accepted this revision.Dec 29 2022, 12:25 PM

LGTM, thank you!
I was wondering if something like this was there while writing that test :)

llvm/unittests/Support/DivisionByConstantTest.cpp
173

I have checked this still passes at 14 bits.

This revision is now accepted and ready to land.Dec 29 2022, 12:25 PM
This revision was landed with ongoing or failed builds.Dec 29 2022, 1:59 PM
This revision was automatically updated to reflect the committed changes.

If you're interested in this area, there's known improvements that haven't been implemented in LLVM, as pointed out by https://ridiculousfish.com/blog/posts/labor-of-division-episode-iii.html. See https://www.ncbi.nlm.nih.gov/pmc/articles/PMC8258644/ for a more complete review of known techniques.

This change broke the test divide-by-constant.ll in the M68k backend, the test may have to be updated.

See: https://github.com/llvm/llvm-project/issues/59802