Re: [PATCH] D18639: Use __builtin_isnan/isinf/isfinite in complex
Eugene.Zelenko added a subscriber: Eugene.Zelenko. Eugene.Zelenko added a comment. Looks like patch was not committed. https://reviews.llvm.org/D18639 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D18639: Use __builtin_isnan/isinf/isfinite in complex
hfinkel added a comment. In https://reviews.llvm.org/D18639#533807, @hfinkel wrote: > In https://reviews.llvm.org/D18639#527285, @hfinkel wrote: > > > In https://reviews.llvm.org/D18639#515000, @hfinkel wrote: > > > > > Updated to use scheme suggested by Marshall. > > > > > > Ping. > > > Ping. Ping. https://reviews.llvm.org/D18639 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D18639: Use __builtin_isnan/isinf/isfinite in complex
hfinkel added a comment. In https://reviews.llvm.org/D18639#527285, @hfinkel wrote: > In https://reviews.llvm.org/D18639#515000, @hfinkel wrote: > > > Updated to use scheme suggested by Marshall. > > > Ping. Ping. https://reviews.llvm.org/D18639 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D18639: Use __builtin_isnan/isinf/isfinite in complex
hfinkel added a comment. In https://reviews.llvm.org/D18639#515000, @hfinkel wrote: > Updated to use scheme suggested by Marshall. Ping. https://reviews.llvm.org/D18639 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D18639: Use __builtin_isnan/isinf/isfinite in complex
hfinkel updated this revision to Diff 67992.
hfinkel added a comment.
Updated to use scheme suggested by Marshall.
https://reviews.llvm.org/D18639
Files:
include/cmath
include/complex
Index: include/complex
===
--- include/complex
+++ include/complex
@@ -602,39 +602,39 @@
_Tp __bc = __b * __c;
_Tp __x = __ac - __bd;
_Tp __y = __ad + __bc;
-if (isnan(__x) && isnan(__y))
+if (__libcpp_isnan(__x) && __libcpp_isnan(__y))
{
bool __recalc = false;
-if (isinf(__a) || isinf(__b))
+if (__libcpp_isinf(__a) || __libcpp_isinf(__b))
{
-__a = copysign(isinf(__a) ? _Tp(1) : _Tp(0), __a);
-__b = copysign(isinf(__b) ? _Tp(1) : _Tp(0), __b);
-if (isnan(__c))
+__a = copysign(__libcpp_isinf(__a) ? _Tp(1) : _Tp(0), __a);
+__b = copysign(__libcpp_isinf(__b) ? _Tp(1) : _Tp(0), __b);
+if (__libcpp_isnan(__c))
__c = copysign(_Tp(0), __c);
-if (isnan(__d))
+if (__libcpp_isnan(__d))
__d = copysign(_Tp(0), __d);
__recalc = true;
}
-if (isinf(__c) || isinf(__d))
+if (__libcpp_isinf(__c) || __libcpp_isinf(__d))
{
-__c = copysign(isinf(__c) ? _Tp(1) : _Tp(0), __c);
-__d = copysign(isinf(__d) ? _Tp(1) : _Tp(0), __d);
-if (isnan(__a))
+__c = copysign(__libcpp_isinf(__c) ? _Tp(1) : _Tp(0), __c);
+__d = copysign(__libcpp_isinf(__d) ? _Tp(1) : _Tp(0), __d);
+if (__libcpp_isnan(__a))
__a = copysign(_Tp(0), __a);
-if (isnan(__b))
+if (__libcpp_isnan(__b))
__b = copysign(_Tp(0), __b);
__recalc = true;
}
-if (!__recalc && (isinf(__ac) || isinf(__bd) ||
- isinf(__ad) || isinf(__bc)))
+if (!__recalc && (__libcpp_isinf(__ac) || __libcpp_isinf(__bd) ||
+ __libcpp_isinf(__ad) || __libcpp_isinf(__bc)))
{
-if (isnan(__a))
+if (__libcpp_isnan(__a))
__a = copysign(_Tp(0), __a);
-if (isnan(__b))
+if (__libcpp_isnan(__b))
__b = copysign(_Tp(0), __b);
-if (isnan(__c))
+if (__libcpp_isnan(__c))
__c = copysign(_Tp(0), __c);
-if (isnan(__d))
+if (__libcpp_isnan(__d))
__d = copysign(_Tp(0), __d);
__recalc = true;
}
@@ -677,33 +677,33 @@
_Tp __c = __w.real();
_Tp __d = __w.imag();
_Tp __logbw = logb(fmax(fabs(__c), fabs(__d)));
-if (isfinite(__logbw))
+if (__libcpp_isfinite(__logbw))
{
__ilogbw = static_cast(__logbw);
__c = scalbn(__c, -__ilogbw);
__d = scalbn(__d, -__ilogbw);
}
_Tp __denom = __c * __c + __d * __d;
_Tp __x = scalbn((__a * __c + __b * __d) / __denom, -__ilogbw);
_Tp __y = scalbn((__b * __c - __a * __d) / __denom, -__ilogbw);
-if (isnan(__x) && isnan(__y))
+if (__libcpp_isnan(__x) && __libcpp_isnan(__y))
{
-if ((__denom == _Tp(0)) && (!isnan(__a) || !isnan(__b)))
+if ((__denom == _Tp(0)) && (!__libcpp_isnan(__a) || !__libcpp_isnan(__b)))
{
__x = copysign(_Tp(INFINITY), __c) * __a;
__y = copysign(_Tp(INFINITY), __c) * __b;
}
-else if ((isinf(__a) || isinf(__b)) && isfinite(__c) && isfinite(__d))
+else if ((__libcpp_isinf(__a) || __libcpp_isinf(__b)) && __libcpp_isfinite(__c) && __libcpp_isfinite(__d))
{
-__a = copysign(isinf(__a) ? _Tp(1) : _Tp(0), __a);
-__b = copysign(isinf(__b) ? _Tp(1) : _Tp(0), __b);
+__a = copysign(__libcpp_isinf(__a) ? _Tp(1) : _Tp(0), __a);
+__b = copysign(__libcpp_isinf(__b) ? _Tp(1) : _Tp(0), __b);
__x = _Tp(INFINITY) * (__a * __c + __b * __d);
__y = _Tp(INFINITY) * (__b * __c - __a * __d);
}
-else if (isinf(__logbw) && __logbw > _Tp(0) && isfinite(__a) && isfinite(__b))
+else if (__libcpp_isinf(__logbw) && __logbw > _Tp(0) && __libcpp_isfinite(__a) && __libcpp_isfinite(__b))
{
-__c = copysign(isinf(__c) ? _Tp(1) : _Tp(0), __c);
-__d = copysign(isinf(__d) ? _Tp(1) : _Tp(0), __d);
+__c = copysign(__libcpp_isinf(__c) ? _Tp(1) : _Tp(0), __c);
+__d = copysign(__libcpp_isinf(__d) ? _Tp(1) : _Tp(0), __d);
__x = _Tp(0) * (__a * __c + __b * __d);
__y = _Tp(0) * (__b * __c - __a * __d);
}
@@ -913,9 +913,9 @@
_Tp
norm(const complex<_Tp>& __c)
{
-if (isinf(__c.real()))
+if (__libcpp_isinf(__c.real()))
return abs(__c.real());
-if (isinf(__c.imag()))
+if (__libcpp_isinf(__c.imag()))
return abs(__c.imag());
return __c.real()
Re: [PATCH] D18639: Use __builtin_isnan/isinf/isfinite in complex
hfinkel added a comment. In https://reviews.llvm.org/D18639#491232, @mclow.lists wrote: > And is there any reason why `__libcpp_isinf` can't just return `false` for > non-fp types? For custom numeric types that have an isinf, etc. found by ADL, they should continue to work. https://reviews.llvm.org/D18639 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D18639: Use __builtin_isnan/isinf/isfinite in complex
mclow.lists added a comment. And is there any reason why `__libcpp_isinf` can't just return `false` for non-fp types? https://reviews.llvm.org/D18639 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D18639: Use __builtin_isnan/isinf/isfinite in complex
mclow.lists added a comment.
Maybe something like this? (untested)
template
_LIBCPP_ALWAYS_INLINE
typename std::enable_if::value, bool>::type
__libcpp_isfinite(_A1 __lcpp_x) _NOEXCEPT
{
return isfinite(__lcpp_x);
}
template
_LIBCPP_ALWAYS_INLINE
typename std::enable_if::value, bool>::type
__libcpp_isfinite(_A1 __lcpp_x) _NOEXCEPT
{
#if __has_builtin(__builtin_isfinite)
return __builtin_isfinite(__lcpp_x);
#else
return isfinite(__lcpp_x);
#endif
}
https://reviews.llvm.org/D18639
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D18639: Use __builtin_isnan/isinf/isfinite in complex
scanon added a comment. I am not the right person to review the C++ template details, but everything else seems OK to me. https://reviews.llvm.org/D18639 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D18639: Use __builtin_isnan/isinf/isfinite in complex
hfinkel added a comment. Ping. https://reviews.llvm.org/D18639 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D18639: Use __builtin_isnan/isinf/isfinite in complex
hfinkel updated this revision to Diff 63029.
hfinkel added a comment.
Thanks everyone. I've rebased this and changed the name to __libcpp_*.
Marshall, how do you recommend rewriting the functions to reduce the
boilerplate?
http://reviews.llvm.org/D18639
Files:
include/cmath
include/complex
Index: include/complex
===
--- include/complex
+++ include/complex
@@ -602,39 +602,39 @@
_Tp __bc = __b * __c;
_Tp __x = __ac - __bd;
_Tp __y = __ad + __bc;
-if (isnan(__x) && isnan(__y))
+if (__libcpp_isnan(__x) && __libcpp_isnan(__y))
{
bool __recalc = false;
-if (isinf(__a) || isinf(__b))
+if (__libcpp_isinf(__a) || __libcpp_isinf(__b))
{
-__a = copysign(isinf(__a) ? _Tp(1) : _Tp(0), __a);
-__b = copysign(isinf(__b) ? _Tp(1) : _Tp(0), __b);
-if (isnan(__c))
+__a = copysign(__libcpp_isinf(__a) ? _Tp(1) : _Tp(0), __a);
+__b = copysign(__libcpp_isinf(__b) ? _Tp(1) : _Tp(0), __b);
+if (__libcpp_isnan(__c))
__c = copysign(_Tp(0), __c);
-if (isnan(__d))
+if (__libcpp_isnan(__d))
__d = copysign(_Tp(0), __d);
__recalc = true;
}
-if (isinf(__c) || isinf(__d))
+if (__libcpp_isinf(__c) || __libcpp_isinf(__d))
{
-__c = copysign(isinf(__c) ? _Tp(1) : _Tp(0), __c);
-__d = copysign(isinf(__d) ? _Tp(1) : _Tp(0), __d);
-if (isnan(__a))
+__c = copysign(__libcpp_isinf(__c) ? _Tp(1) : _Tp(0), __c);
+__d = copysign(__libcpp_isinf(__d) ? _Tp(1) : _Tp(0), __d);
+if (__libcpp_isnan(__a))
__a = copysign(_Tp(0), __a);
-if (isnan(__b))
+if (__libcpp_isnan(__b))
__b = copysign(_Tp(0), __b);
__recalc = true;
}
-if (!__recalc && (isinf(__ac) || isinf(__bd) ||
- isinf(__ad) || isinf(__bc)))
+if (!__recalc && (__libcpp_isinf(__ac) || __libcpp_isinf(__bd) ||
+ __libcpp_isinf(__ad) || __libcpp_isinf(__bc)))
{
-if (isnan(__a))
+if (__libcpp_isnan(__a))
__a = copysign(_Tp(0), __a);
-if (isnan(__b))
+if (__libcpp_isnan(__b))
__b = copysign(_Tp(0), __b);
-if (isnan(__c))
+if (__libcpp_isnan(__c))
__c = copysign(_Tp(0), __c);
-if (isnan(__d))
+if (__libcpp_isnan(__d))
__d = copysign(_Tp(0), __d);
__recalc = true;
}
@@ -677,33 +677,33 @@
_Tp __c = __w.real();
_Tp __d = __w.imag();
_Tp __logbw = logb(fmax(fabs(__c), fabs(__d)));
-if (isfinite(__logbw))
+if (__libcpp_isfinite(__logbw))
{
__ilogbw = static_cast(__logbw);
__c = scalbn(__c, -__ilogbw);
__d = scalbn(__d, -__ilogbw);
}
_Tp __denom = __c * __c + __d * __d;
_Tp __x = scalbn((__a * __c + __b * __d) / __denom, -__ilogbw);
_Tp __y = scalbn((__b * __c - __a * __d) / __denom, -__ilogbw);
-if (isnan(__x) && isnan(__y))
+if (__libcpp_isnan(__x) && __libcpp_isnan(__y))
{
-if ((__denom == _Tp(0)) && (!isnan(__a) || !isnan(__b)))
+if ((__denom == _Tp(0)) && (!__libcpp_isnan(__a) || !__libcpp_isnan(__b)))
{
__x = copysign(_Tp(INFINITY), __c) * __a;
__y = copysign(_Tp(INFINITY), __c) * __b;
}
-else if ((isinf(__a) || isinf(__b)) && isfinite(__c) && isfinite(__d))
+else if ((__libcpp_isinf(__a) || __libcpp_isinf(__b)) && __libcpp_isfinite(__c) && __libcpp_isfinite(__d))
{
-__a = copysign(isinf(__a) ? _Tp(1) : _Tp(0), __a);
-__b = copysign(isinf(__b) ? _Tp(1) : _Tp(0), __b);
+__a = copysign(__libcpp_isinf(__a) ? _Tp(1) : _Tp(0), __a);
+__b = copysign(__libcpp_isinf(__b) ? _Tp(1) : _Tp(0), __b);
__x = _Tp(INFINITY) * (__a * __c + __b * __d);
__y = _Tp(INFINITY) * (__b * __c - __a * __d);
}
-else if (isinf(__logbw) && __logbw > _Tp(0) && isfinite(__a) && isfinite(__b))
+else if (__libcpp_isinf(__logbw) && __logbw > _Tp(0) && __libcpp_isfinite(__a) && __libcpp_isfinite(__b))
{
-__c = copysign(isinf(__c) ? _Tp(1) : _Tp(0), __c);
-__d = copysign(isinf(__d) ? _Tp(1) : _Tp(0), __d);
+__c = copysign(__libcpp_isinf(__c) ? _Tp(1) : _Tp(0), __c);
+__d = copysign(__libcpp_isinf(__d) ? _Tp(1) : _Tp(0), __d);
__x = _Tp(0) * (__a * __c + __b * __d);
__y = _Tp(0) * (__b * __c - __a * __d);
}
@@ -941,9 +941,9 @@
_Tp
norm(const complex<_Tp>& __c)
{
-if (isinf(__c.real()))
+if (__libcpp_isinf(__c.real()))
return abs(__c.real());
-if
Re: [PATCH] D18639: Use __builtin_isnan/isinf/isfinite in complex
scanon added a comment. I agree with Marshall. Aside from the points he raises, this approach looks fine. http://reviews.llvm.org/D18639 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D18639: Use __builtin_isnan/isinf/isfinite in complex
mclow.lists added a comment. Hal and I talked about this in Oulu. In general, I'm good with this approach. However, I think that the code could be made much clearer. (some naming changes, some code rearrangement) First off, I think the name `__fast_isinf` is a poor name. I think something like `__isinf` would be better, but Hal informs me that *everyone* uses that name for something, so I think that `__libcpp_isinf` would be better. Secondly, instead of duplicating all of the boilerplate code around the different versions of whatever we call it, I think we can bury the ifdefs inside the inline functions. I'll try to work up an example, and post it here. http://reviews.llvm.org/D18639 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D18639: Use __builtin_isnan/isinf/isfinite in complex
hfinkel added a comment. In http://reviews.llvm.org/D18639#472010, @chandlerc wrote: > I'm fine with this change, but we should also get Steve to comment on it, and > make sure we have a good way of explaining this to users. > > In particular, we probably need some documentation around these fast routines > that clearly indicates they should only be used when optimizations such as > `-ffast-math` and `-ffinite-math-only` should strip the checks. We want to be > careful to only use the optimizable forms when that behavior is appropriate. > And auditing the ones you've switched for that is something Steve might be > better suited to do... Also, I spoke to Marshall offline about this last week, and I expect he'll also comment. http://reviews.llvm.org/D18639 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D18639: Use __builtin_isnan/isinf/isfinite in complex
hfinkel added a comment. ping http://reviews.llvm.org/D18639 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
