This revision was automatically updated to reflect the committed changes.
Closed by commit rL287012: [CUDA] Mark __libcpp_{isnan,isinf,isfinite} as
constexpr. (authored by jlebar).
Changed prior to commit:
https://reviews.llvm.org/D25403?vs=77271&id=78041#toc
Repository:
rL LLVM
https://rev
jlebar marked 6 inline comments as done.
jlebar added a comment.
Capturing an IRC conversation:
> **EricWF** jlebar: Did you test this patch with older Clangs w/o constexpr
> builtins?
> **jlebar** EricWF, Do you mean, did I test the test, or did I test that the
> non-test change does what I n
EricWF added inline comments.
Comment at: libcxx/test/libcxx/numerics/c.math/constexpr-fns.pass.cpp:17
+// true constexpr-ness.
+
+#include
jlebar wrote:
> EricWF wrote:
> > Does GCC offer these as contexpr? If not this needs a `// XFAIL: gcc`
> Looks like the r
EricWF added inline comments.
Comment at: libcxx/test/libcxx/numerics/c.math/constexpr-fns.pass.cpp:22
+
+#ifndef _LIBCPP_VERSION
+#error _LIBCPP_VERSION not defined
jlebar wrote:
> EricWF wrote:
> > You don't need this `_LIBCPP_VERSION` check here.
> Hm, there a
jlebar added a comment.
Thanks for the review.
Comment at: libcxx/test/libcxx/numerics/c.math/constexpr-fns.pass.cpp:17
+// true constexpr-ness.
+
+#include
EricWF wrote:
> Does GCC offer these as contexpr? If not this needs a `// XFAIL: gcc`
Looks like the re
EricWF accepted this revision.
EricWF added inline comments.
This revision is now accepted and ready to land.
Comment at: libcxx/test/libcxx/numerics/c.math/constexpr-fns.pass.cpp:17
+// true constexpr-ness.
+
+#include
Does GCC offer these as contexpr? If not t
hfinkel added a comment.
In https://reviews.llvm.org/D25403#590049, @jlebar wrote:
> Use TEST_STD_VER macro.
This is fine with me; @EricWF , @mclow.lists ?
https://reviews.llvm.org/D25403
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
ht
jlebar updated this revision to Diff 77271.
jlebar added a comment.
Use TEST_STD_VER macro.
https://reviews.llvm.org/D25403
Files:
libcxx/include/cmath
libcxx/test/libcxx/numerics/c.math/constexpr-fns.pass.cpp
Index: libcxx/test/libcxx/numerics/c.math/constexpr-fns.pass.cpp
==
hfinkel added inline comments.
Comment at: libcxx/test/libcxx/numerics/c.math/constexpr-fns.pass.cpp:24
+
+#if __cplusplus >= 201103L
+constexpr bool a = std::__libcpp_isnan(0.);
I think the preferred form here is:
#if TEST_STD_VER >= 11
https://reviews.llvm.o
jlebar updated this revision to Diff 77105.
jlebar added a comment.
Add a test.
https://reviews.llvm.org/D25403
Files:
libcxx/include/cmath
libcxx/test/libcxx/numerics/c.math/constexpr-fns.pass.cpp
Index: libcxx/test/libcxx/numerics/c.math/constexpr-fns.pass.cpp
==
jlebar added a comment.
Hal, whadya think?
https://reviews.llvm.org/D25403
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
jlebar added a comment.
So, good news -- these three builtins are already constexpr-evaluatable. :)
I'll add a test.
https://reviews.llvm.org/D25403
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/list
hfinkel added a comment.
In https://reviews.llvm.org/D25403#580444, @jlebar wrote:
> //Let "CE" mean "constexpr-evaluatable". //
>
> libc++ attempts to be backwards-compatible with old versions of clang, right?
Yea, we'd need to ifdef the test for older versions of Clang. I've just
summarized
hfinkel added a comment.
Here's what I suggest:
1. We commit this patch (I think it logically makes sense; the builtins should
be constexpr evaluatable, even if they're currently not)
2. We follow up by fixing Clang to make the builtins constexpr evaluatable
(since I think that makes sense rega
jlebar added a comment.
//Let "CE" mean "constexpr-evaluatable". //
libc++ attempts to be backwards-compatible with old versions of clang, right?
Old versions of clang are going to fail, since the builtin will not be CE. Is
the idea to write a test that checks that, if __builtin_isfinite is C
hfinkel added a comment.
In https://reviews.llvm.org/D25403#580439, @jlebar wrote:
> > I'm not sure about that. It seems like a useful feature for the builtins to
> > have. Logically speaking, they should be constexpr.
>
> I agree that it's logically correct for the builtins to be
> constexpr-e
jlebar added a comment.
> I'm not sure about that. It seems like a useful feature for the builtins to
> have. Logically speaking, they should be constexpr.
I agree that it's logically correct for the builtins to be
constexpr-evaluatable. My point is just that doing this work and then writing
hfinkel added a comment.
In https://reviews.llvm.org/D25403#580432, @jlebar wrote:
> In https://reviews.llvm.org/D25403#580422, @hfinkel wrote:
>
> > Okay. Why not fix the Clang builtins so that they're evaluatable for
> > constant inputs in a constexpr context? Then we can do this and test the
jlebar added a comment.
In https://reviews.llvm.org/D25403#580422, @hfinkel wrote:
> Okay. Why not fix the Clang builtins so that they're evaluatable for constant
> inputs in a constexpr context? Then we can do this and test the change.
I am not sure how much value we would derive from testing
hfinkel added a comment.
In https://reviews.llvm.org/D25403#580416, @jlebar wrote:
> > Is this because the functions are in instead of in are
> > you don't want to mark all of as host/device?
>
> Yes. cmath is its own beast; we need to have our own implementation of it in
> order to direct
jlebar added a comment.
> Is this because the functions are in instead of in are you
> don't want to mark all of as host/device?
Yes. cmath is its own beast; we need to have our own implementation of it in
order to direct the std functions to the appropriate low-level device
functions. (A
hfinkel added a comment.
...
>
>
>> I guess I don't understand the motivation for this change.
>
> To make work in CUDA, we do the following terrible, awful,
> horrible, no good thing: ...well, I can just show you the code.
> https://github.com/llvm-project/llvm-project/blob/master/clang/l
jlebar added a comment.
Friendly ping?
https://reviews.llvm.org/D25403
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
jlebar added a comment.
In https://reviews.llvm.org/D25403#573666, @mclow.lists wrote:
> Yesterday and today is the first time in a while that clang has been
> seriously broken for more than an hour or so.
> I'm not inclined to worry about it yet.
Oh, awesome. That sounds good to me.
The qu
mclow.lists added a comment.
In https://reviews.llvm.org/D25403#573104, @jlebar wrote:
> Likewise, should I update the documentation to indicate that check-cxx may
> fail with a clang built from tip of tree, due to c++17 support being
> experimental? Or do you all want to change the target so
jlebar added a comment.
Thank you, Marshall.
In https://reviews.llvm.org/D25403#572998, @mclow.lists wrote:
> My build setup is similar to yours (on Mac OS X):
>
> cd $LLVM_BUILD ; rm -rf libcxx ; mkdir libcxx ; cd libcxx
> CXX=$LLVM_BIN/clang++ cmake -DLLVM_PATH=$LLVM/llvm
> -DLIBCXX_CXX_
mclow.lists added a comment.
> error: undefined reference to '__gxx_personality_v0'
means that you're not linking to an ABI library.
That symbol comes out of libc++abi, but you can also get that from libsupc.
My build setup is similar to yours (on Mac OS X):
cd $LLVM_BUILD ; rm -rf libcxx ;
jlebar added a comment.
> The tests should be runnable with lit. I generally just do an in-tree build
> and run make check-libcxx. @EricWF , what's the recommended way of running
> the tests from an out-of-tree build?
Things seem broken at the moment with clang from tip of tree.
I did a clean
hfinkel added a comment.
In https://reviews.llvm.org/D25403#565603, @jlebar wrote:
> Although these pass the CUDA test-suite tests (which I haven't yet committed
> because they're broken without this change), I could use some help running
> the libcxx tests.
>
> I cannot find any documentation
jlebar added a comment.
Although these pass the CUDA test-suite tests (which I haven't yet committed
because they're broken without this change), I could use some help running the
libcxx tests.
I cannot find any documentation explaining how to run the libcxx tests with
just-built clang. Presu
jlebar created this revision.
jlebar added a reviewer: hfinkel.
jlebar added a subscriber: cfe-commits.
This makes these functions available on host and device, which is
necessary to compile for the device.
https://reviews.llvm.org/D25403
Files:
libcxx/include/cmath
Index: libcxx/include/c
31 matches
Mail list logo
