[PATCH] D28797: [LangRef] Make @llvm.sqrt(x) return undef, rather than have UB, for negative x.

[Justin Lebar via Phabricator via llvm-commits]

jlebar added a comment.

> We do, but this seems somewhat accidental. Most of the libm intrinsics have this problem, and sqrt is the only one for which we have this undefined behavior.

Indeed.  Now that I look at LegalizeDAG, I don't think it makes a lot of sense to legalize ISD::FSQRT to `if (x >= -0) libm_sqrt(x) else NaN` while leaving every other ExpandFPLibCall alone.

This langref change at least brings sqrt into line with the rest of the llvm math functions, and makes the legalization code as broken for it as it is for most everything else.

Indeed, one could argue that the issue in the legalization code is unrelated to this issue in the langref.  ISD::FSQRT has defined behavior on negative values.  The fact that we're incorrectly legalizing it to libm_sqrt arguably has nothing to do with the semantics of @llvm.sqrt.f32.

@efriedma, are you OK with this change going in, on that basis?


https://reviews.llvm.org/D28797