This is an archive of the discontinued LLVM Phabricator instance.

[KnownBits] Cleanup some misspelling / logic in {u,s}div
ClosedPublic

Authored by goldstein.w.n on May 18 2023, 5:37 PM.

Details

Summary

Chronically misspelled 'denominator' as 'denuminator' and a few other
cases.

On the logic side, no longer require RHS to be strictly positive in
sdiv. This in turn means we need to handle a possible zero denom
in the APInt division.

Diff Detail

Event Timeline

goldstein.w.n created this revision.May 18 2023, 5:37 PM
Herald added a project: Restricted Project. · View Herald TranscriptMay 18 2023, 5:37 PM
goldstein.w.n requested review of this revision.May 18 2023, 5:37 PM
Herald added a project: Restricted Project. · View Herald TranscriptMay 18 2023, 5:37 PM
goldstein.w.n added inline comments.
llvm/lib/Support/KnownBits.cpp
790

@foad, re:

At this point the sign of Res should match ResultSign, so you don't need a std::optional, you could just have a "bool ResultSignKnown".

Since Denom now can be zero (result of not using strictlypositive), we can't just grab the sign from the result so keeping as optional bool

790–791

@foad, re:

Don't need this, or the makeNegative call below.

Needed for the same reason as above.

foad added inline comments.May 19 2023, 1:51 AM
llvm/lib/Support/KnownBits.cpp
790

I think you can copy what you do in udiv - if the min value of denom is 0, treat it as 1 (since if the value was really 0, the result would be poison).

goldstein.w.n marked an inline comment as done.May 19 2023, 12:13 PM

Remove unneeded sign logic

nikic added inline comments.Jun 1 2023, 12:35 PM
llvm/lib/Support/KnownBits.cpp
844–845

I expect either this check should be dropped, or we don't need the clearBits above.

goldstein.w.n marked an inline comment as done.Jun 1 2023, 2:53 PM

Remove unneeded conflict check

nikic accepted this revision.Jun 6 2023, 5:32 AM

LGTM

This revision is now accepted and ready to land.Jun 6 2023, 5:32 AM