This is an archive of the discontinued LLVM Phabricator instance.

[KnownBits][RISCV] Improve known bits for srem.
ClosedPublic

Authored by craig.topper on Feb 20 2021, 1:32 PM.

Details

Summary

The result must be less than or equal to the LHS side, so any
leading zeros in the left hand side must also exist in the result.
This is stronger than the previous behavior where we only considered
the sign bit being 0.

The affected test case used the sign bit being known 0 to change
a sign extend to a zero extend pre type legalization. After type
legalization the types were promoted to i64, but we no longer
knew bit 31 was zero. This shifts are are the equivalent of an
AND with 0xffffffff or zext_inreg X, i32. This patch allows us to
see that bit 31 is zero and remove the shifts.

Diff Detail

Event Timeline

craig.topper created this revision.Feb 20 2021, 1:32 PM
craig.topper requested review of this revision.Feb 20 2021, 1:32 PM
Herald added a project: Restricted Project. · View Herald TranscriptFeb 20 2021, 1:32 PM
Herald added a subscriber: MaskRay. · View Herald Transcript

Improve the comment about why this is valid.

Improve the comment again

RKSimon added inline comments.Feb 21 2021, 3:00 AM
llvm/lib/Support/KnownBits.cpp
480

I don't suppose this lets us make the KnownBitsTest exhaustive srem test more accurate? Most binops we still only checks for common subset bits when we might be able to check for exact equality now.

craig.topper added inline comments.Feb 21 2021, 11:10 AM
llvm/lib/Support/KnownBits.cpp
480

I tried equality. It failed for both zeros and ones. We don't infer ones unless all bits of the RHS are known. For zeros we're only calculating an upper bound on the result.

RKSimon accepted this revision.Feb 21 2021, 1:19 PM

LGTM

llvm/lib/Support/KnownBits.cpp
480

Fair enough, thanks for trying.

This revision is now accepted and ready to land.Feb 21 2021, 1:19 PM
This revision was automatically updated to reflect the committed changes.