This is an archive of the discontinued LLVM Phabricator instance.

ValueTracking: Handle more instructions in isKnownNeverNaN
ClosedPublic

Authored by arsenm on Aug 10 2018, 2:24 PM.

Details

Reviewers
spatel
scanon

Diff Detail

Event Timeline

arsenm created this revision.Aug 10 2018, 2:24 PM
spatel added inline comments.Aug 12 2018, 2:47 PM
lib/Analysis/ValueTracking.cpp
2933–2935

Remove stale comment.

2945–2949

Why are these not the same as fdiv/frem? I think we need to account for inf math that produces nan with these operators:
inf + -inf = nan
inf - inf = nan
0.0 * inf = nan

arsenm updated this revision to Diff 161259.Aug 17 2018, 8:21 AM

Don't handle fadd etc.

Please add the tests with baseline CHECKs as a preliminary commit, then rebase this.

arsenm updated this revision to Diff 161341.Aug 17 2018, 2:59 PM
spatel added inline comments.Aug 19 2018, 10:46 AM
lib/Analysis/ValueTracking.cpp
2933–2935

Comment is still here. Also, we should have tests for sitofp/uitofp.

arsenm added inline comments.Aug 19 2018, 11:11 AM
lib/Analysis/ValueTracking.cpp
2933–2935

The tests are there for sitofp/uitofp, just apparently somehow the fcmp ord was already optimizing away before

arsenm added inline comments.Aug 19 2018, 11:47 PM
lib/Analysis/ValueTracking.cpp
2933–2935

It looks like foldFCmpIntToFPConst specifically handles the case of these with an fcmp ord

spatel accepted this revision.Aug 20 2018, 5:17 AM

LGTM

lib/Analysis/ValueTracking.cpp
2933–2935

Ah, I see. Sorry, I didn't see the existing tests. We can test this patch more directly and improve efficiency with a small edit in instsimplify, but that can be a follow-up. Please do remove or edit the TODO comment here if there are planned improvements.

This revision is now accepted and ready to land.Aug 20 2018, 5:17 AM
arsenm closed this revision.Aug 20 2018, 9:51 AM

r340187