This is an archive of the discontinued LLVM Phabricator instance.

ValueTracking: Handle frem in computeKnownFPClass
ClosedPublic

Authored by arsenm on Apr 18 2023, 4:39 PM.

Details

Summary

I barely understand what this does, but try to
handle the last case required to delete cannotBeOrderedLessThanZeroImpl.
Also improve by following fdiv handling for nans and identical operand case.

Diff Detail

Event Timeline

arsenm created this revision.Apr 18 2023, 4:39 PM
Herald added a project: Restricted Project. · View Herald TranscriptApr 18 2023, 4:39 PM
arsenm requested review of this revision.Apr 18 2023, 4:39 PM
Herald added a reviewer: sstefan1. · View Herald Transcript
Herald added a project: Restricted Project. · View Herald Transcript
Herald added a subscriber: wdng. · View Herald Transcript
kpn added inline comments.Apr 20 2023, 9:56 AM
llvm/lib/Analysis/ValueTracking.cpp
4746

754-2019 section 6.1 says "remainder(subnormal, Inf) signals underflow." Does that mean it produces a nan or a zero? I'm guessing zero, but don't hold me to that. Anybody?

Section 7.2 agrees with your comment and code here.

foad added inline comments.Apr 21 2023, 3:37 AM
llvm/lib/Analysis/ValueTracking.cpp
4726–4727

Is this just fixing an oversight for FDiv?

4743–4760

This part only works for FDiv. I can't think of anything useful you can prove about the sign of FRem.

arsenm added inline comments.Apr 24 2023, 1:50 PM
llvm/lib/Analysis/ValueTracking.cpp
4726–4727

Yes

arsenm added inline comments.Apr 24 2023, 4:01 PM
llvm/lib/Analysis/ValueTracking.cpp
4743–4760

"the result has the same sign as x and magnitude less than the magnitude of y."

So actually this is more conservative than required for free

arsenm updated this revision to Diff 516587.Apr 24 2023, 5:08 PM

Relax frem handling

foad added inline comments.Apr 25 2023, 1:06 AM
llvm/lib/Analysis/ValueTracking.cpp
4743–4760

"the result has the same sign as x and magnitude less than the magnitude of y."

Good point. I was looking at "man remainder" instead of "man fmod".

foad added inline comments.Apr 25 2023, 1:14 AM
llvm/lib/Analysis/ValueTracking.cpp
4699

fcPosZero seems wrong. Wouldn't the result be -0 if X is -ve?

4734

Remove REM part of comment?

4743–4760

Comment should just say that +ve / +ve is +ve.

arsenm added inline comments.Apr 25 2023, 5:28 AM
llvm/lib/Analysis/ValueTracking.cpp
4699

No, the signs cancel and the divide is exact. The only case to worry about would be frem -0, -0 but that's nan.

arsenm updated this revision to Diff 516761.Apr 25 2023, 5:30 AM
foad added inline comments.Apr 25 2023, 5:51 AM
llvm/lib/Analysis/ValueTracking.cpp
4699

That seems to contradict "the result has the same sign as x". Surely that implies that -1 frem -1 is -0.

arsenm updated this revision to Diff 517023.Apr 25 2023, 7:29 PM
arsenm marked an inline comment as done.

0

llvm/lib/Analysis/ValueTracking.cpp
4699

Yes, -1%-1 is -0.

foad accepted this revision.Apr 26 2023, 12:12 AM
This revision is now accepted and ready to land.Apr 26 2023, 12:12 AM