tavianator added inline comments.
================
Comment at: src/cxa_thread_atexit.cpp:47
@@ +46,3 @@
+ // called during the loop.
+ if (pthread_setspecific(dtors, ptr) != 0) {
+ abort_message("pthread_setspecific() failed during thread_local
destruction");
----------------
bcraig wrote:
> The loop doesn't read pthread_getspecific anymore. I get the need for the
> setspecific call here for your previous design, but I don't think it's needed
> now.
__cxa_thread_atexit() calls pthread_getspecific(), so it's still needed.
Otherwise it would create a new list instead of adding to the current one, and
the ordering would be wrong.
================
Comment at: src/cxa_thread_atexit.cpp:99
@@ +98,3 @@
+
+ if (__cxa_thread_atexit_impl) {
+ return __cxa_thread_atexit_impl(dtor, obj, dso_symbol);
----------------
bcraig wrote:
> I'm going to have to agree with @majnemer. I think that the config check for
> LIBCXXABI_HAS_CXA_THREAD_ATEXIT_IMPL should stay in place. If
> cxa_thread_atexit_impl exists, then all of the fallback code can disappear at
> preprocessing time.
>
> We do lose out on the minor benefit of avoiding some libc++ recompiles, but
> we also avoid code bloat.
>
> For what it's worth, I'm willing to keep the weak symbol check in place if
> __cxa_thread_atexit_impl isn't present, I just don't want to pay for the
> fallback when I know I'm not going to use it.
Makes sense, I'll do that.
================
Comment at: test/thread_local_destruction_order.pass.cpp:48
@@ +47,3 @@
+ thread_local OrderChecker fn_thread_local{0};
+}
+
----------------
bcraig wrote:
> Can we have a CreatesThreadLocalInDestructor in the thread_fn as well? That
> way we can test both the main function and a pthread. If I understand your
> code and comments correctly, those go through different code paths.
Yep, meant to do that actually!
================
Comment at: test/thread_local_destruction_order.pass.cpp:54
@@ +53,3 @@
+ std::thread{thread_fn}.join();
+
+ thread_local OrderChecker fn_thread_local{2};
----------------
bcraig wrote:
> In the places where you can, validate that dtors actually are getting called.
> This may be your only place where you can do that.
>
> So something like 'assert(seq == 1)' here.
Sounds good. What I wanted to do was print some output in the destructors, and
check for a certain expected output. But I couldn't figure out how to do that
with lit.
http://reviews.llvm.org/D21803
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
