[PATCH] D18639: Use __builtin_isnan/isinf/isfinite in complex

[Hal Finkel via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL283051: Use __builtin_isnan/isinf/isfinite in complex 
(authored by hfinkel).

Changed prior to commit:
  https://reviews.llvm.org/D18639?vs=67992=73202#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D18639

Files:
  libcxx/trunk/include/cmath
  libcxx/trunk/include/complex

Index: libcxx/trunk/include/cmath
===
--- libcxx/trunk/include/cmath
+++ libcxx/trunk/include/cmath
@@ -558,6 +558,66 @@
 }
 #endif
 
+template 
+_LIBCPP_ALWAYS_INLINE
+typename enable_if::value, bool>::type
+__libcpp_isnan(_A1 __lcpp_x) _NOEXCEPT
+{
+#if __has_builtin(__builtin_isnan)
+return __builtin_isnan(__lcpp_x);
+#else
+return isnan(__lcpp_x);
+#endif
+}
+
+template 
+_LIBCPP_ALWAYS_INLINE
+typename enable_if::value, bool>::type
+__libcpp_isnan(_A1 __lcpp_x) _NOEXCEPT
+{
+return isnan(__lcpp_x);
+}
+
+template 
+_LIBCPP_ALWAYS_INLINE
+typename enable_if::value, bool>::type
+__libcpp_isinf(_A1 __lcpp_x) _NOEXCEPT
+{
+#if __has_builtin(__builtin_isinf)
+return __builtin_isinf(__lcpp_x);
+#else
+return isinf(__lcpp_x);
+#endif
+}
+
+template 
+_LIBCPP_ALWAYS_INLINE
+typename enable_if::value, bool>::type
+__libcpp_isinf(_A1 __lcpp_x) _NOEXCEPT
+{
+return isinf(__lcpp_x);
+}
+
+template 
+_LIBCPP_ALWAYS_INLINE
+typename 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
+}
+
+template 
+_LIBCPP_ALWAYS_INLINE
+typename enable_if::value, bool>::type
+__libcpp_isfinite(_A1 __lcpp_x) _NOEXCEPT
+{
+return isfinite(__lcpp_x);
+}
+
 _LIBCPP_END_NAMESPACE_STD
 
 #endif  // _LIBCPP_CMATH
Index: libcxx/trunk/include/complex
===
--- libcxx/trunk/include/complex
+++ libcxx/trunk/include/complex
@@ -599,39 +599,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;
 }
@@ -674,33 +674,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)) && 

[PATCH] D18639: Use __builtin_isnan/isinf/isfinite in complex

[Hal Finkel via cfe-commits]

hfinkel added a comment.

In https://reviews.llvm.org/D18639#545697, @EricWF wrote:

> LGTM.
>
> In https://reviews.llvm.org/D18639#514991, @hfinkel wrote:
>
> > 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.
>
>
> Do we already support custom numeric types? If so could you add a test for 
> this under `test/libcxx`? Just a simple test case that instantiates the 
> functions and shows it compiles.


As it turns out, the answer is: mostly. We should discuss this more: PR30589.


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

[Eugene Zelenko via cfe-commits]

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

[Hal Finkel via cfe-commits]

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

[Hal Finkel via cfe-commits]

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

[Hal Finkel via cfe-commits]

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

[Hal Finkel via cfe-commits]

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

[Hal Finkel via cfe-commits]

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

[Marshall Clow via cfe-commits]

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

[Marshall Clow via cfe-commits]

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

[Steve Canon via cfe-commits]

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

[Hal Finkel via cfe-commits]

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

[Hal Finkel via cfe-commits]

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

[Steve Canon via cfe-commits]

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

[Marshall Clow via cfe-commits]

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

[Hal Finkel via cfe-commits]

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

[Hal Finkel via cfe-commits]

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

[PATCH] D18639: Use __builtin_isnan/isinf/isfinite in complex

[Hal Finkel via cfe-commits]

hfinkel created this revision.
hfinkel added reviewers: mclow.lists, EricWF, chandlerc.
hfinkel added a subscriber: cfe-commits.
Herald added a subscriber: mcrosier.

The libc-provided isnan/isinf/isfinite macro implementations are specifically 
designed to function correctly, even in the presence of -ffast-math (or, more 
specifically, -ffinite-math-only). As such, on most implementation, these 
either always turn into external function calls (e.g. glibc) or are 
specifically function calls when __FINITE_MATH_ONLY__ is defined (e.g. Darwin).

Our implementation of complex arithmetic make heavy use of isnan/isinf/isfinite 
to deal with corner cases involving non-finite quantities. This is problematic 
in two respects:

 1. On systems where these are always function calls (e.g. Linux/glibc), there 
is a performance penalty
 2. When compiling with -ffast-math, there is a significant performance penalty 
(in fact, on Darwin and systems with similar implementations, the code may in 
fact be slower than not using -ffast-math, because the inline definitions 
provided by libc become unavailable to prevent the checks from being optimized 
out).

Eliding these inf/nan checks in -ffast-math mode is consistent with what 
happens with libstdc++, and in my experience, what users expect. This is 
critical to getting high-performance code when using complex. This patch 
replaces uses of those functions on basic floating-point types with calls to 
__builtin_isnan/isinf/isfinite, which Clang will always expand inline. When 
using -ffast-math (or  -ffinite-math-only), the optimizer will remove the 
checks as expected.

http://reviews.llvm.org/D18639

Files:
  include/cmath
  include/complex

Index: include/complex
===
--- include/complex
+++ include/complex
@@ -596,39 +596,39 @@
 _Tp __bc = __b * __c;
 _Tp __x = __ac - __bd;
 _Tp __y = __ad + __bc;
-if (isnan(__x) && isnan(__y))
+if (__fast_isnan(__x) && __fast_isnan(__y))
 {
 bool __recalc = false;
-if (isinf(__a) || isinf(__b))
+if (__fast_isinf(__a) || __fast_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(__fast_isinf(__a) ? _Tp(1) : _Tp(0), __a);
+__b = copysign(__fast_isinf(__b) ? _Tp(1) : _Tp(0), __b);
+if (__fast_isnan(__c))
 __c = copysign(_Tp(0), __c);
-if (isnan(__d))
+if (__fast_isnan(__d))
 __d = copysign(_Tp(0), __d);
 __recalc = true;
 }
-if (isinf(__c) || isinf(__d))
+if (__fast_isinf(__c) || __fast_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(__fast_isinf(__c) ? _Tp(1) : _Tp(0), __c);
+__d = copysign(__fast_isinf(__d) ? _Tp(1) : _Tp(0), __d);
+if (__fast_isnan(__a))
 __a = copysign(_Tp(0), __a);
-if (isnan(__b))
+if (__fast_isnan(__b))
 __b = copysign(_Tp(0), __b);
 __recalc = true;
 }
-if (!__recalc && (isinf(__ac) || isinf(__bd) ||
-  isinf(__ad) || isinf(__bc)))
+if (!__recalc && (__fast_isinf(__ac) || __fast_isinf(__bd) ||
+  __fast_isinf(__ad) || __fast_isinf(__bc)))
 {
-if (isnan(__a))
+if (__fast_isnan(__a))
 __a = copysign(_Tp(0), __a);
-if (isnan(__b))
+if (__fast_isnan(__b))
 __b = copysign(_Tp(0), __b);
-if (isnan(__c))
+if (__fast_isnan(__c))
 __c = copysign(_Tp(0), __c);
-if (isnan(__d))
+if (__fast_isnan(__d))
 __d = copysign(_Tp(0), __d);
 __recalc = true;
 }
@@ -671,33 +671,33 @@
 _Tp __c = __w.real();
 _Tp __d = __w.imag();
 _Tp __logbw = logb(fmax(fabs(__c), fabs(__d)));
-if (isfinite(__logbw))
+if (__fast_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 (__fast_isnan(__x) && __fast_isnan(__y))
 {
-if ((__denom == _Tp(0)) && (!isnan(__a) || !isnan(__b)))
+if ((__denom == _Tp(0)) && (!__fast_isnan(__a) || !__fast_isnan(__b)))
 {
 __x = copysign(_Tp(INFINITY), __c) * __a;
 __y = copysign(_Tp(INFINITY), __c) * __b;
 }
-else if ((isinf(__a) ||