[PATCH] D148858: ValueTracking: Replace CannotBeNegativeZero

[Kevin P. Neal via Phabricator via llvm-commits]

kpn added a comment.

Are we certain that a constrained sqrt() folded away will be as accurate as not folded when the rounding mode is different? There's a lot of permutations of library calls and instructions that constrained sqrt() can be lowered to, and I'm worried that the results will be surprisingly bad with different rounding modes. If we don't fold then the end user can deal with library issues themselves at least, and hopefully square root instructions will be on CPUs that properly handle the different rounding modes.

I'm pretty sure I wrote those comments, and I think not folding due to the rounding mode is conservatively correct. I really don't think changing this behavior belongs in a patch titled "ValueTracking: Replace CannotBeNegativeZero". Changing sqrt() folding should be a different patch.

And if constrained sqrt is folded away then the "Negative test:" comments should be removed. With the fold the comments are incorrect.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D148858/new/

https://reviews.llvm.org/D148858