This is an archive of the discontinued LLVM Phabricator instance.

ValueTracking: Implement computeKnownFPClass for ldexp
ClosedPublic

Authored by arsenm on May 1 2023, 7:41 AM.

Diff Detail

Event Timeline

arsenm created this revision.May 1 2023, 7:41 AM
Herald added a project: Restricted Project. · View Herald TranscriptMay 1 2023, 7:41 AM
arsenm requested review of this revision.May 1 2023, 7:41 AM
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
foad added inline comments.May 19 2023, 1:10 AM
llvm/include/llvm/Analysis/ValueTracking.h
368

Rebase to pick this up from trunk

378

Typo "depending copied"? Not sure what was meant

384

Typo "flushed under"?

foad added inline comments.May 19 2023, 1:15 AM
llvm/lib/Analysis/ValueTracking.cpp
4222

This is discarding anything we already knew. The docs should make that clear (for this function and propagateCanonicalizingSrc) - or change it so it only &s new information into KnownFPClasses.

foad added inline comments.May 19 2023, 1:34 AM
llvm/lib/Analysis/ValueTracking.cpp
4239

I think this needs to be fcZero for correctness, because a negative subnormal could be flushed to +0. (I guess this is what the TODO alludes to, but (a) it should be a FIXME and (b) maybe just do it?)

4829–4830

Typo for "non-zero"? But in any case I'm not sure what the comment means or how it relates to this part of the code. Non-zeroness of the input won't be preserved if exp is negative - a non-zero input could easily underflow to a zero result.

4840
4849

This should have been simplified already to an explicit canonicalize, right?

arsenm marked 5 inline comments as done.May 19 2023, 8:42 AM
arsenm added inline comments.
llvm/lib/Analysis/ValueTracking.cpp
4829–4830

The point was this is a slightly stronger version of the previous block checking cannotBeOrdered{Less|Greater}ThanZero that covers 0. The sign isn't changing regardless of overflow

4840

This regresses a few cases. The independent +/- pieces can be inferred below

4849

No, non-constrained operations aren't guaranteed to canonicalize. Nothing folds to canonicalize. This should have folded to a direct x reference. We aren't reporting that the result is canonical, it's reporting that it could have been flushed.

The simplification for this also checks for a literal 0, this query is the full known class query. Theoretically a case could exist that's known 0 but isn't a constant 0

arsenm updated this revision to Diff 523797.May 19 2023, 8:42 AM
arsenm updated this revision to Diff 523830.May 19 2023, 9:50 AM
foad accepted this revision.Jul 11 2023, 3:14 AM
foad added inline comments.
llvm/lib/Analysis/ValueTracking.cpp
4238

Only do this if the input or output mode is preserve-sign or dynamic?

This revision is now accepted and ready to land.Jul 11 2023, 3:14 AM