This is an archive of the discontinued LLVM Phabricator instance.

[DAGCombiner] Use rsqrt to estimate sqrt only when ninf flag set
ClosedPublic

Authored by qiucf on Mar 26 2020, 8:31 AM.

Details

Summary

Currently we use rsqrt to build estimation of sqrt. But this would produce wrong result (NaN) when x is +infinity.

Diff Detail

Event Timeline

qiucf created this revision.Mar 26 2020, 8:31 AM
craig.scott resigned from this revision.Mar 26 2020, 1:20 PM
spatel added inline comments.Mar 27 2020, 9:41 AM
llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp
13096–13098

Can we add the 'ninf' check here instead?

21193

We can make this comment more exact; something like:

// Require 'ninf' because sqrt(Inf) == Inf, but an estimate computes:
// sqrt(Inf) == rqsrt(Inf) * Inf == 0 * Inf == NaN.
qiucf updated this revision to Diff 253425.Mar 29 2020, 9:00 AM

Address comments to move check into visitFSQRT.

qiucf marked 2 inline comments as done.Mar 29 2020, 9:01 AM
spatel accepted this revision.Mar 29 2020, 10:31 AM

LGTM

This revision is now accepted and ready to land.Mar 29 2020, 10:31 AM
This revision was automatically updated to reflect the committed changes.