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&id=73202#toc
Repository:
rL LLVM
https://reviews.llvm.o
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 `
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/list
EricWF accepted this revision.
EricWF added a comment.
This revision is now accepted and ready to land.
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
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.
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
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
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/com
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.
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/list
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_i
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/
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
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/comple
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-com
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
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 fas
chandlerc added a reviewer: scanon.
chandlerc added a comment.
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 indic
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
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
20 matches
Mail list logo
