This is an archive of the discontinued LLVM Phabricator instance.

Make the sqrt intrinsic return undef for a negative input.
ClosedPublic

Authored by spatel on Sep 29 2014, 10:11 AM.

Details

Summary

As discussed here:
http://lists.cs.uiuc.edu/pipermail/llvm-commits/Week-of-Mon-20140609/220598.html

And again here:
http://lists.cs.uiuc.edu/pipermail/llvmdev/2014-September/077168.html

The sqrt of a negative number when using the llvm intrinsic is undefined. We should return undef rather than 0.0 to match the definition in the LLVM IR lang ref.

This change should not affect any code that isn't using "no-nans-fp-math"; ie, no-nans is a requirement for generating the llvm intrinsic in place of a sqrt function call.

Unfortunately, the behavior introduced by this patch will not match current gcc, xlc, icc, and possibly other compilers. The current clang/llvm behavior of returning 0.0 doesn't either. We knowingly approve of this difference with the other compilers in an attempt to flag code that is invoking undefined behavior.

A front-end warning should also try to convince the user that the program will fail:
http://llvm.org/bugs/show_bug.cgi?id=21093

Diff Detail

Event Timeline

spatel updated this revision to Diff 14174.Sep 29 2014, 10:11 AM
spatel retitled this revision from to Make the sqrt intrinsic return undef for a negative input..
spatel updated this object.
spatel edited the test plan for this revision. (Show Details)
spatel added a subscriber: Unknown Object (MLST).
hfinkel accepted this revision.Sep 29 2014, 11:40 AM
hfinkel edited edge metadata.

LGTM.

lib/Analysis/ConstantFolding.cpp
1527

I'd say, "... should only be emitted in place of a call to libm's sqrt function when using..."

This revision is now accepted and ready to land.Sep 29 2014, 11:40 AM
spatel closed this revision.Oct 1 2014, 1:46 PM
spatel updated this revision to Diff 14291.

Closed by commit rL218803 (authored by @spatel).