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

[Hal Finkel via Phabricator via llvm-commits]

hfinkel added a comment.

In https://reviews.llvm.org/D28797#648781, @efriedma wrote:

> I'm not sure I really like the logic of "all these other intrinsics are broken, therefore we should break llvm.sqrt()", especially since we don't really use most of the intrinsics in question.
>
> If we're going to ignore the problem, at least get rid of the "(which allows for better optimization, because there is no need to worry about errno being set)" bit.
>
> On a mostly unrelated note, making llvm.sqrt() return undef rather than NaN for negative numbers is completely useless; we might as well just say that @llvm.sqrt is precisely the IEEE 754 squareRoot() operation minus any side-effects.  We can mark a call "nnan" to indicate that the backend doesn't need to worry about negative numbers and nans.


Yes! Please just remove it.

I had been under some impression that we'd have a potential problem doing this because the corresponding SDAG node inherits these undef properties, and who knows what all of the target lowering does... but it doesn't: SelectionDAGBuilder::visitCall has this:

  case LibFunc::sqrt:
  case LibFunc::sqrtf:
  case LibFunc::sqrtl:
  case LibFunc::sqrt_finite:
  case LibFunc::sqrtf_finite:
  case LibFunc::sqrtl_finite:
    if (visitUnaryFloatCall(I, ISD::FSQRT))
      return;
    break;

so this is just an IR issue. Let's rationalize this.


https://reviews.llvm.org/D28797