[libcxx] r307751 - Fix unrepresentable enum for clang-cl unstable ABI

2017-07-11 Thread Ben Craig via cfe-commits
Author: bcraig Date: Tue Jul 11 18:45:13 2017 New Revision: 307751 URL: http://llvm.org/viewvc/llvm-project?rev=307751=rev Log: Fix unrepresentable enum for clang-cl unstable ABI When using LIBCXX_ABI_UNSTABLE=YES, clang-cl gave the following warning:

[libcxx] r302497 - Fix Windows tests when __config_site is present.

2017-05-08 Thread Ben Craig via cfe-commits
Author: bcraig Date: Mon May 8 20:34:12 2017 New Revision: 302497 URL: http://llvm.org/viewvc/llvm-project?rev=302497=rev Log: Fix Windows tests when __config_site is present. Previously, the force includes would complain about a missing _DEBUG symbol. Now we dump macros before adding the force

[libcxx] r302496 - Revert "Fix Windows tests when __config_site is present."

2017-05-08 Thread Ben Craig via cfe-commits
Author: bcraig Date: Mon May 8 20:26:39 2017 New Revision: 302496 URL: http://llvm.org/viewvc/llvm-project?rev=302496=rev Log: Revert "Fix Windows tests when __config_site is present." It's 2017, and line endings are still an issue. Modified: libcxx/trunk/utils/libcxx/test/config.py

[libcxx] r302421 - Fix Windows tests when __config_site is present.

2017-05-08 Thread Ben Craig via cfe-commits
Author: bcraig Date: Mon May 8 08:15:22 2017 New Revision: 302421 URL: http://llvm.org/viewvc/llvm-project?rev=302421=rev Log: Fix Windows tests when __config_site is present. Previously, the force includes would complain about a missing _DEBUG symbol. Now we dump macros before adding the force

[libcxx] r299942 - [libc++] Fix unknown pragma warning on MSVC

2017-04-11 Thread Ben Craig via cfe-commits
Author: bcraig Date: Tue Apr 11 09:06:39 2017 New Revision: 299942 URL: http://llvm.org/viewvc/llvm-project?rev=299942=rev Log: [libc++] Fix unknown pragma warning on MSVC Modified: libcxx/trunk/include/limits Modified: libcxx/trunk/include/limits URL:

Re: [PATCH] D24278: [analyzer] Extend bug reports with extra notes.

2016-09-09 Thread Ben Craig via cfe-commits
bcraig added a subscriber: bcraig. bcraig added a comment. Neat! I would have liked to have had this for the Excess Padding Checker. Currently, the padding checker has a very long diagnostic that recommends a new order for data members. I think a note (or fixit) would be more appropriate,

Re: [PATCH] D24374: [libc++] Avoid include in locale_win32.h

2016-09-09 Thread Ben Craig via cfe-commits
bcraig added a comment. In https://reviews.llvm.org/D24374#538521, @smeenai wrote: > @bcraig thanks for pointing me to that diff; there's a lot of nice cleanup > going on there. Were you planning on updating and following up on it? > > I also realized I forgot to adjust `locale_win32.cpp` for

Re: [PATCH] D24374: [libc++] Avoid include in locale_win32.h

2016-09-09 Thread Ben Craig via cfe-commits
bcraig added a subscriber: bcraig. bcraig added a comment. This seems related: https://reviews.llvm.org/D20596 In that (stale) review, I switch away from unique_ptr in general for locale related operations. https://reviews.llvm.org/D24374 ___

Re: [PATCH] D24159: Fix PR30202 - notify_all_at_thread_exit seg faults if run from a raw pthread context.

2016-09-02 Thread Ben Craig via cfe-commits
bcraig added a comment. LGTM Comment at: src/condition_variable.cpp:86 @@ +85,3 @@ +if (tl_ptr.get() == nullptr) { +tl_ptr.set_pointer(new __thread_struct); +} EricWF wrote: > bcraig wrote: > > The standard synopsis of notify_all_at_thread_exit

Re: [PATCH] D24159: Fix PR30202 - notify_all_at_thread_exit seg faults if run from a raw pthread context.

2016-09-02 Thread Ben Craig via cfe-commits
bcraig added inline comments. Comment at: src/condition_variable.cpp:86 @@ +85,3 @@ +if (tl_ptr.get() == nullptr) { +tl_ptr.set_pointer(new __thread_struct); +} The standard synopsis of notify_all_at_thread_exit in the standard doesn't have a

Re: [PATCH] D21803: [libcxxabi] Provide a fallback __cxa_thread_atexit() implementation

2016-09-01 Thread Ben Craig via cfe-commits
bcraig added a comment. The "@" will do a ping through phabricator, but a direct email is probably going to be your best bet at this point. https://reviews.llvm.org/D21803 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

Re: [PATCH] D21803: [libcxxabi] Provide a fallback __cxa_thread_atexit() implementation

2016-08-31 Thread Ben Craig via cfe-commits
bcraig added a comment. In https://reviews.llvm.org/D21803#530678, @tavianator wrote: > Ping? Well, I still think it's fine. Maybe a direct message to @mclow.lists or @EricWF? https://reviews.llvm.org/D21803 ___ cfe-commits mailing list

Re: [PATCH] D23279: clang-reorder-fields

2016-08-23 Thread Ben Craig via cfe-commits
bcraig added a comment. In https://reviews.llvm.org/D23279#523449, @alexshap wrote: > @djasper, @bcraig, @aaron.ballman - many thanks for the comments and > suggestions, > please, let me know if you are waiting for any other changes on my side (for > v0) or if there is anything else i can do

Re: [PATCH] D23279: clang-reorder-fields

2016-08-22 Thread Ben Craig via cfe-commits
bcraig added a comment. In https://reviews.llvm.org/D23279#522504, @djasper wrote: > In the meantime, I don't know whether we should check this in as a > standalone-tool and then merge into clang-refactor once ready. Ben, what do > you think? Well, I don't know how much you should count my

Re: [PATCH] D23719: [libc++] Use C11 for atomics check

2016-08-19 Thread Ben Craig via cfe-commits
bcraig added a comment. In https://reviews.llvm.org/D23719#520969, @rsmith wrote: > In https://reviews.llvm.org/D23719#520954, @bcraig wrote: > > > In https://reviews.llvm.org/D23719#520952, @rsmith wrote: > > > > > Are we really guaranteed that the C and C++ compiler behave the same way > > >

Re: [PATCH] D23719: [libc++] Use C11 for atomics check

2016-08-19 Thread Ben Craig via cfe-commits
bcraig added a comment. In https://reviews.llvm.org/D23719#520953, @smeenai wrote: > @bcraig: `__STDC_NO_ATOMICS__` wouldn't be defined for pre-C11 compilers > either, right? > > From what I understand of the original code sample, one of the purposes was > to check for 64-bit atomic support on

Re: [PATCH] D23719: [libc++] Use C11 for atomics check

2016-08-19 Thread Ben Craig via cfe-commits
bcraig added a comment. In https://reviews.llvm.org/D23719#520952, @rsmith wrote: > Are we really guaranteed that the C and C++ compiler behave the same way > here? I don't see why that would necessarily be the case. For libc++, std::atomic is implemented in terms of _Atomic. So as long as

Re: [PATCH] D23719: [libc++] Use C11 for atomics check

2016-08-19 Thread Ben Craig via cfe-commits
bcraig added a subscriber: bcraig. bcraig added a comment. I like the rationale here, but can we avoid pulling in headers at all? You could test _ _STDC_NO_ATOMICS_ _. You could also have some code like this... _Atomic int x; int main() { x += 1; return x; }

Re: [PATCH] D23387: [Analyzer] Report found fields order in PaddingChecker.

2016-08-15 Thread Ben Craig via cfe-commits
bcraig added a comment. LGTM Comment at: lib/StaticAnalyzer/Checkers/PaddingChecker.cpp:217 @@ +216,3 @@ +// then large field indices to small field indices +return std::make_tuple(Align, -Size, + Field ?

Re: [PATCH] D23387: [Analyzer] Report found fields order in PaddingChecker.

2016-08-12 Thread Ben Craig via cfe-commits
bcraig added a comment. LGTM. Thanks for the patch! Comment at: lib/StaticAnalyzer/Checkers/PaddingChecker.cpp:217 @@ +216,3 @@ +// then large field indices to small field indices +return std::make_tuple(Align, -Size, + Field ?

Re: [PATCH] D23387: [Analyzer] Report found fields order in PaddingChecker.

2016-08-11 Thread Ben Craig via cfe-commits
bcraig added a comment. I am kind of interested in what the warning message for clang::Sema in clang/include/clang/Sema/Sema.h looks like. When I last analyzed that file, there was a bunch of padding. It's a very long class though, so it can stress the usability of your messages.

Re: [PATCH] D23387: [Analyzer] Report found fields order in PaddingChecker.

2016-08-11 Thread Ben Craig via cfe-commits
bcraig added a comment. Note: In the future, try to provide more context lines in the patch. http://llvm.org/docs/Phabricator.html#requesting-a-review-via-the-web-interface I'm mostly fine with the algorithmic changes. I'm not thrilled with the formatting of the error message, but I'm not

Re: [PATCH] D23279: clang-reorder-fields

2016-08-10 Thread Ben Craig via cfe-commits
bcraig added a comment. In https://reviews.llvm.org/D23279#511187, @omtcyfz wrote: > In https://reviews.llvm.org/D23279#510234, @alexshap wrote: > > > > I do think that an automated tool will do a better job of changing field > > > orderings in a non-breaking way than most people would, mostly

Re: [PATCH] D23279: clang-reorder-fields

2016-08-09 Thread Ben Craig via cfe-commits
bcraig added a comment. In https://reviews.llvm.org/D23279#510178, @omtcyfz wrote: > In https://reviews.llvm.org/D23279#510167, @alexshap wrote: > > > my point is the following: this tool helps perform some operation (changing > > the order of fields), > > if smb decides to do it manually -

Re: [PATCH] D23279: clang-reorder-fields

2016-08-09 Thread Ben Craig via cfe-commits
bcraig added a subscriber: bcraig. bcraig added a comment. In https://reviews.llvm.org/D23279#509017, @Eugene.Zelenko wrote: > Do we really need standalone tool for this purpose? If I'm not mistaken, > Static Analyzer already has clang-analyzer-optin.performance.Padding check, > which is also

[libcxx] r277373 - Adding smart_ptr benchmark

2016-08-01 Thread Ben Craig via cfe-commits
Author: bcraig Date: Mon Aug 1 14:56:39 2016 New Revision: 277373 URL: http://llvm.org/viewvc/llvm-project?rev=277373=rev Log: Adding smart_ptr benchmark Initial draft here: https://reviews.llvm.org/D22470 ... though this is Eric Fiselier's rewrite to fit in with Google Benchmark. Added:

Re: [PATCH] D22470: [libcxx] Improve shared_ptr dtor performance

2016-08-01 Thread Ben Craig via cfe-commits
bcraig closed this revision. bcraig added a comment. committed https://reviews.llvm.org/rL277357: Improve shared_ptr dtor performance. https://reviews.llvm.org/D22470 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[libcxx] r277357 - Improve shared_ptr dtor performance

2016-08-01 Thread Ben Craig via cfe-commits
Author: bcraig Date: Mon Aug 1 12:51:26 2016 New Revision: 277357 URL: http://llvm.org/viewvc/llvm-project?rev=277357=rev Log: Improve shared_ptr dtor performance If the last destruction is uncontended, skip the atomic store on __shared_weak_owners_. This shifts some costs from normal

Re: [PATCH] D22470: [libcxx] Improve shared_ptr dtor performance

2016-08-01 Thread Ben Craig via cfe-commits
bcraig added a comment. I am going to submit the code changes and the tests independently. I'm having trouble getting cmake to use the right compiler for the libcxx-benchmarks target. https://reviews.llvm.org/D22470 ___ cfe-commits mailing list

Re: [PATCH] D22557: [libcxx] Diagnose invalid memory order arguments in . Fixes PR21179.

2016-07-21 Thread Ben Craig via cfe-commits
bcraig added inline comments. Comment at: include/atomic:569 @@ +568,3 @@ +__attribute__ ((__enable_if__(__m == memory_order_release \ + || __m == memory_order_acq_rel, ""))) \ +__attribute__ ((__unavailable__("memory

Re: [PATCH] D22557: [libcxx] Diagnose invalid memory order arguments in . Fixes PR21179.

2016-07-20 Thread Ben Craig via cfe-commits
bcraig added a subscriber: bcraig. Comment at: include/atomic:569 @@ +568,3 @@ +__attribute__ ((__enable_if__(__m == memory_order_release \ + || __m == memory_order_acq_rel, ""))) \ +__attribute__

Re: [PATCH] D22470: [libcxx] Improve shared_ptr dtor performance

2016-07-19 Thread Ben Craig via cfe-commits
bcraig updated the summary for this revision. bcraig updated this revision to Diff 64504. bcraig added a comment. Added weak_ptr benchmark, as that's where the cost shifted. https://reviews.llvm.org/D22470 Files: benchmarks/shared_ptr_create_destroy.cpp

Re: [PATCH] D22292: [libunwind] Fix unw_getcontext for ARMv6-m

2016-07-18 Thread Ben Craig via cfe-commits
bcraig added a subscriber: bcraig. Comment at: src/UnwindRegistersRestore.S:325 @@ -324,4 +324,3 @@ DEFINE_LIBUNWIND_PRIVATE_FUNCTION(_ZN9libunwind13Registers_arm20restoreCoreAndJumpToEv) -#if !defined(__ARM_ARCH_ISA_ARM) - ldr r2, [r0, #52] - ldr r3, [r0, #60] +#if

[PATCH] D22470: [libcxx] Improve shared_ptr dtor performance

2016-07-18 Thread Ben Craig via cfe-commits
bcraig created this revision. bcraig added reviewers: jfb, mclow.lists, EricWF. bcraig added a subscriber: cfe-commits. If the last destruction is uncontended, skip the atomic store on __shared_weak_owners_. For x86_64, this results in an 8% improvement in shared_ptr ctor+dtor performance. Old

Re: [PATCH] D21968: [libcxx] Externally threaded libc++ variant - Take 2

2016-07-06 Thread Ben Craig via cfe-commits
bcraig added a comment. LGTM. As usual, you'll want to get one of the code owner's sign off first though. http://reviews.llvm.org/D21968 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

Re: [PATCH] D21803: [libcxxabi] Provide a fallback __cxa_thread_atexit() implementation

2016-07-05 Thread Ben Craig via cfe-commits
bcraig added inline comments. Comment at: test/thread_local_destruction_order.pass.cpp:54 @@ +53,3 @@ + std::thread{thread_fn}.join(); + + thread_local OrderChecker fn_thread_local{2}; tavianator wrote: > bcraig wrote: > > In the places where you can, validate

Re: [PATCH] D21968: [libcxx] Externally threaded libc++ variant - Take 2

2016-07-05 Thread Ben Craig via cfe-commits
bcraig added inline comments. Comment at: CMakeLists.txt:131 @@ -130,2 +130,3 @@ option(LIBCXX_HAS_MUSL_LIBC "Build libc++ with support for the Musl C library" OFF) -option(LIBCXX_HAS_PTHREAD_API "Ignore auto-detection and force use of pthread API" OFF)

Re: [PATCH] D21803: [libcxxabi] Provide a fallback __cxa_thread_atexit() implementation

2016-07-05 Thread Ben Craig via cfe-commits
bcraig 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"); The loop

Re: [PATCH] D21991: [libunwind][ARM] Improve unwinder stack usage - Make WMMX support optional

2016-07-05 Thread Ben Craig via cfe-commits
bcraig added inline comments. Comment at: CMakeLists.txt:108 @@ -107,2 +107,3 @@ option(LIBUNWIND_ENABLE_CROSS_UNWINDING "Enable cross-platform unwinding support." OFF) +option(LIBUNWIND_ENABLE_ARM_WMMX "Enable unwinding support for ARM WMMX registers." ON)

Re: [PATCH] D21803: [libcxxabi] Provide a fallback __cxa_thread_atexit() implementation

2016-06-30 Thread Ben Craig via cfe-commits
bcraig added a comment. Also, can you add test cases for a lot of these things? I don't expect test cases for the DSO side of things, but a lot of the tricky atexit cases should be covered. http://reviews.llvm.org/D21803 ___ cfe-commits mailing

Re: [PATCH] D21803: [libcxxabi] Provide a fallback __cxa_thread_atexit() implementation

2016-06-30 Thread Ben Craig via cfe-commits
bcraig added inline comments. Comment at: src/cxa_thread_atexit.cpp:47 @@ +46,3 @@ + void run_dtors(void* ptr) { +if (pthread_setspecific(dtors, ptr) != 0) { + abort_message("pthread_setspecific() failed during thread_local destruction"); Why are we

Re: [PATCH] D21803: [libcxxabi] Provide a fallback __cxa_thread_atexit() implementation

2016-06-30 Thread Ben Craig via cfe-commits
bcraig added a comment. In http://reviews.llvm.org/D21803#470564, @joerg wrote: > On the topic of __cxa_thread_atexit, was it ever specified how it interacts > with things like thread cancellation? I don't think it's officially specified anywhere. C++ threads don't have a cancel method.

Re: [PATCH] D21803: [libcxxabi] Provide a fallback __cxa_thread_atexit() implementation

2016-06-29 Thread Ben Craig via cfe-commits
bcraig added a comment. > Hmm, maybe? If other global destructors run after ~DtorListHolder(), and > they cause a thread_local to be initialized for the first time, > __cxa_thread_atexit() might be called again. I was thinking that dtors would > get re-initialized in that case but it appears

Re: [PATCH] D21803: [libcxxabi] Provide a fallback __cxa_thread_atexit() implementation

2016-06-29 Thread Ben Craig via cfe-commits
bcraig added a comment. In http://reviews.llvm.org/D21803#470060, @tavianator wrote: > In http://reviews.llvm.org/D21803#469988, @bcraig wrote: > > > You should look at __thread_specific_ptr in libcxx's . It does a > > lot of these things in order to satisfy the requirements of > >

Re: [PATCH] D21803: [libcxxabi] Provide a fallback __cxa_thread_atexit() implementation

2016-06-29 Thread Ben Craig via cfe-commits
bcraig added subscribers: rmaprath, bcraig. bcraig added a comment. You should look at __thread_specific_ptr in libcxx's . It does a lot of these things in order to satisfy the requirements of notify_all_at_thread_exit, set_value_at_thread_exit, and make_ready_at_thread_exit. @rmaprath has

Re: [PATCH] D21229: [Analyzer] Don't cache report generation ExplodedNodes

2016-06-23 Thread Ben Craig via cfe-commits
bcraig closed this revision. bcraig added a comment. Old: 106,657,666 New: 105,346,818 Submitted in r273572 http://reviews.llvm.org/D21229 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

r273572 - [Analyzer] Don't cache report generation ExplodedNodes

2016-06-23 Thread Ben Craig via cfe-commits
Author: bcraig Date: Thu Jun 23 10:47:12 2016 New Revision: 273572 URL: http://llvm.org/viewvc/llvm-project?rev=273572=rev Log: [Analyzer] Don't cache report generation ExplodedNodes During the core analysis, ExplodedNodes are added to the ExplodedGraph, and those nodes are cached for

Re: [PATCH] D21637: [libcxx] Don't use pthread initializers in constexpr constructors

2016-06-23 Thread Ben Craig via cfe-commits
bcraig added subscribers: joerg, bcraig. bcraig added a comment. @joerg. I think this has been fixed in newer versions of NetBSD. Sure would be nice if someone could fix this on the MUSL side of things. See https://bugs.musicpd.org/view.php?id=4110 for fun conversations about mutex,

Re: [PATCH] D21345: [libcxx] [test] Avoid huge main() functions and huge arrays.

2016-06-14 Thread Ben Craig via cfe-commits
bcraig added a comment. LGTM. I don't see anything controversial here. If you want to wait for @EricWF or @mclow.lists though, that's fine. http://reviews.llvm.org/D21345 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

Re: [PATCH] D21345: [libcxx] [test] Avoid huge main() functions and huge arrays.

2016-06-14 Thread Ben Craig via cfe-commits
bcraig added a subscriber: bcraig. bcraig added a comment. > Even though they have artificial blocks in order to keep variables locally > scoped, this gives MSVC's /analyze a headache, because it views main() as > consuming an enormous amount of stack space (it doesn't reuse stack, at least >

[PATCH] D21229: [Analyzer] Don't cache report generation ExplodedNodes

2016-06-10 Thread Ben Craig via cfe-commits
bcraig created this revision. bcraig added reviewers: zaks.anna, dcoughlin, jordan_rose. bcraig added a subscriber: cfe-commits. During the core analysis, ExplodedNodes are added to the ExplodedGraph, and those nodes are cached for deduplication purposes. After core analysis, reports are

Re: [PATCH] D20933: Preallocate ExplodedNode hash table

2016-06-10 Thread Ben Craig via cfe-commits
bcraig closed this revision. bcraig added a comment. Completed: At revision: 272394 http://reviews.llvm.org/D20933 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

r272394 - Preallocate ExplodedNode hash table

2016-06-10 Thread Ben Craig via cfe-commits
Author: bcraig Date: Fri Jun 10 08:22:13 2016 New Revision: 272394 URL: http://llvm.org/viewvc/llvm-project?rev=272394=rev Log: Preallocate ExplodedNode hash table Rehashing the ExplodedNode table is very expensive. The hashing itself is expensive, and the general activity of iterating over the

Re: [PATCH] D20933: Preallocate ExplodedNode hash table

2016-06-09 Thread Ben Craig via cfe-commits
bcraig added a comment. I got better valgrind numbers (symbols are important). Before: 106,131,538 After: 106,657,666 Diff: 526,128 larger. Note that this is sampled peaks for heap usage. They may not be accurate. Regardless, the peak usage increased by less than .5%.

Re: [PATCH] D20933: Preallocate ExplodedNode hash table

2016-06-09 Thread Ben Craig via cfe-commits
bcraig updated this revision to Diff 60177. bcraig added a comment. Capping the pre-reserve space http://reviews.llvm.org/D20933 Files: include/clang/StaticAnalyzer/Core/PathSensitive/ExplodedGraph.h lib/StaticAnalyzer/Core/CoreEngine.cpp Index: lib/StaticAnalyzer/Core/CoreEngine.cpp

Re: [PATCH] D20933: Preallocate ExplodedNode hash table

2016-06-09 Thread Ben Craig via cfe-commits
bcraig added a comment. In http://reviews.llvm.org/D20933#452854, @dcoughlin wrote: > A 6% speed improvement could be a big win! Do we have a sense of what the > expected increased memory cost (as a % of average use over the lifetime of > the process) is? My guess is it would be relatively

Re: [PATCH] D20933: Preallocate ExplodedNode hash table

2016-06-08 Thread Ben Craig via cfe-commits
bcraig added a comment. In http://reviews.llvm.org/D20933#452352, @zaks.anna wrote: > > For the pile of LLVM projects that I am currently building (llvm, clang, > > libcxx, libcxxabi), 18.9% of all analyzed > > > functions hit the maximum step count. For the previously discussed large > > .C

Re: [PATCH] D20933: Preallocate ExplodedNode hash table

2016-06-07 Thread Ben Craig via cfe-commits
bcraig updated this revision to Diff 59950. bcraig added a comment. Uploading more context. http://reviews.llvm.org/D20933 Files: include/clang/StaticAnalyzer/Core/PathSensitive/ExplodedGraph.h lib/StaticAnalyzer/Core/CoreEngine.cpp Index: lib/StaticAnalyzer/Core/CoreEngine.cpp

Re: [PATCH] D20933: Preallocate ExplodedNode hash table

2016-06-07 Thread Ben Craig via cfe-commits
bcraig added a comment. tldr; I'm going to recommend we stick with the current algorithm, but I only have partial data to back that up. For the pile of LLVM projects that I am currently building (llvm, clang, libcxx, libcxxabi), 18.9% of all analyzed functions hit the maximum step count. For

Re: [PATCH] D20933: Preallocate ExplodedNode hash table

2016-06-06 Thread Ben Craig via cfe-commits
bcraig added a comment. I'll try to get those stats. My suspicion is that it is highly language / projects specific. Specifically, I suspect that C (and maybe ObjC?) won't hit the analysis limit often, but that C++ will hit the limit frequently because of the large number of inline

Re: [PATCH] D20933: Preallocate ExplodedNode hash table

2016-06-03 Thread Ben Craig via cfe-commits
bcraig added a comment. In http://reviews.llvm.org/D20933#447608, @zaks.anna wrote: > Does FoldingSet already have reserve? (I do not see it.) The reserve call would be new. I'm attempting to add it in this llvm review: http://reviews.llvm.org/D20930 > My understanding is that you propose

[PATCH] D20933: Preallocate ExplodedNode hash table

2016-06-02 Thread Ben Craig via cfe-commits
bcraig created this revision. bcraig added reviewers: zaks.anna, krememek, jordan_rose. bcraig added a subscriber: cfe-commits. This depends on http://reviews.llvm.org/D20930 Rehashing the ExplodedNode table is very expensive. The hashing itself is expensive, and the general activity of

Re: [PATCH] D20328: [libcxx] Externally threaded libc++ variant

2016-06-02 Thread Ben Craig via cfe-commits
bcraig added inline comments. Comment at: include/__threading_support:201 @@ +200,3 @@ +# if !defined(__has_include) || __has_include(<__static_threading>) +#include<__static_threading> +# else This #include deserves a comment. Some perplexed developer is

Re: [PATCH] D20874: [libcxx] Two more threading dependencies

2016-06-02 Thread Ben Craig via cfe-commits
bcraig added inline comments. Comment at: include/__threading_support:187 @@ +186,3 @@ +{ +std::terminate(); +} rmaprath wrote: > bcraig wrote: > > Why does this need to go in threading_support? Seems like this is mostly > > orthogonal to threading.

Re: [PATCH] D20874: [libcxx] Two more threading dependencies

2016-06-01 Thread Ben Craig via cfe-commits
bcraig added inline comments. Comment at: include/__threading_support:187 @@ +186,3 @@ +{ +std::terminate(); +} Why does this need to go in threading_support? Seems like this is mostly orthogonal to threading. libcxxabi seems like the better place to hold

Re: [PATCH] D16545: [libcxxabi] Enable testing for static libc++abi

2016-06-01 Thread Ben Craig via cfe-commits
bcraig added a comment. In http://reviews.llvm.org/D16545#444869, @bcraig wrote: > In http://reviews.llvm.org/D16545#444854, @rmaprath wrote: > > > Perhaps you forgot to commit that file? > > > Hrm, you're right, it didn't go in. I'll get it in tomorrow morning. Should be in now. r271388.

[libcxxabi] r271388 - Missed part of D16545 (static lib testing)

2016-06-01 Thread Ben Craig via cfe-commits
Author: bcraig Date: Wed Jun 1 07:50:30 2016 New Revision: 271388 URL: http://llvm.org/viewvc/llvm-project?rev=271388=rev Log: Missed part of D16545 (static lib testing) http://reviews.llvm.org/D16545 Modified: libcxxabi/trunk/CMakeLists.txt Modified: libcxxabi/trunk/CMakeLists.txt URL:

Re: [PATCH] D16545: [libcxxabi] Enable testing for static libc++abi

2016-05-31 Thread Ben Craig via cfe-commits
bcraig added a comment. In http://reviews.llvm.org/D16545#444854, @rmaprath wrote: > Hmmm, it looks like part of this patch didn't go in? I cannot see the changes > in the `libcxxabi/CMakeLists.txt` file in the repo. > > I was hoping this would enable me to run the `libc++abi` tests on in-tree

Re: [PATCH] D20730: [libcxx] Prefer UNSUPPORTED over conditional compilation for tests

2016-05-27 Thread Ben Craig via cfe-commits
bcraig added subscribers: mclow.lists, bcraig. bcraig added a comment. LGTM. Add @mclow.lists as a reviewer. http://reviews.llvm.org/D20730 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

Re: [PATCH] D20320: [libunwind] Improve unwinder stack usage - II

2016-05-27 Thread Ben Craig via cfe-commits
bcraig added a comment. In http://reviews.llvm.org/D20320#442246, @rmaprath wrote: > Do you know if there are clang / llvm passes that attempts to achieve some > sort of stack compaction? I couldn't find much. I'm not sure if it belongs to its own pass, or if it is part of another pass, but

Re: [PATCH] D20334: [libcxx] Fix a bug in strstreambuf::overflow

2016-05-27 Thread Ben Craig via cfe-commits
bcraig added a comment. LGTM. You may want to wait on Marshall's approval though. Thanks for being patient with me, and thanks for the patch! http://reviews.llvm.org/D20334 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

Re: [PATCH] D20677: Make it possible to build a -fno-exceptions libc++abi variant.

2016-05-26 Thread Ben Craig via cfe-commits
bcraig added a comment. LGTM. Probably want a "LGTM" from at least one other person though. In http://reviews.llvm.org/D20677#441085, @rmaprath wrote: > In http://reviews.llvm.org/D20677#441061, @jroelofs wrote: > > > This is the canonical reference for the Itanium ABI: > >

Re: [PATCH] D20677: Make it possible to build a -fno-exceptions libc++abi variant.

2016-05-26 Thread Ben Craig via cfe-commits
bcraig added inline comments. Comment at: src/cxa_aux_runtime.cpp:19-24 @@ -18,4 +18,8 @@ extern "C" { _LIBCXXABI_FUNC_VIS LIBCXXABI_NORETURN void __cxa_bad_cast(void) { +#ifndef _LIBCXXABI_NO_EXCEPTIONS throw std::bad_cast(); +#else + std::terminate(); +#endif }

Re: [PATCH] D20334: [libcxx] Fix a bug in strstreambuf::overflow

2016-05-26 Thread Ben Craig via cfe-commits
bcraig added a comment. So I definitely see a problem, and I think your fix is a reasonable solution to that problem. I'm still unclear on how that caused the ASAN error though. Here's my understanding of the current problem... The initial buffer for strstreambuf is often 4096 ( _strstream>

Re: [PATCH] D20677: Make it possible to build a -fno-exceptions libc++abi variant.

2016-05-26 Thread Ben Craig via cfe-commits
bcraig added inline comments. Comment at: CMakeLists.txt:359 @@ -350,2 +358,3 @@ else() - add_subdirectory(test) + # libc++abi tests are mostly exceptions related. The only reason we want to + # build libc++abi without exceptions is to support the -fno-exceptions libc++

Re: [PATCH] D20573: [libcxx] Allow explicit pthread opt-in

2016-05-25 Thread Ben Craig via cfe-commits
bcraig added a comment. r270735 http://reviews.llvm.org/D20573 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D20574: [libcxxabi] Allow explicit pthread opt-in

2016-05-25 Thread Ben Craig via cfe-commits
bcraig closed this revision. bcraig added a comment. r270732 http://reviews.llvm.org/D20574 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[libcxx] r270735 - [libcxx] Allow explicit pthread opt-in

2016-05-25 Thread Ben Craig via cfe-commits
Author: bcraig Date: Wed May 25 12:40:09 2016 New Revision: 270735 URL: http://llvm.org/viewvc/llvm-project?rev=270735=rev Log: [libcxx] Allow explicit pthread opt-in The existing pthread detection code in __config is pretty good for common operating systems. It doesn't allow cmake-time choices

[libcxxabi] r270732 - Allow explicit pthread opt-in

2016-05-25 Thread Ben Craig via cfe-commits
Author: bcraig Date: Wed May 25 12:37:38 2016 New Revision: 270732 URL: http://llvm.org/viewvc/llvm-project?rev=270732=rev Log: Allow explicit pthread opt-in When building libcxxabi in tree (i.e. in llvm/projects/libcxxabi, along with llvm/projects/libcxx), libcxx's config_site.in doesn't get

Re: [PATCH] D20320: [libunwind] Improve unwinder stack usage - II

2016-05-25 Thread Ben Craig via cfe-commits
bcraig added a comment. LGTM. You probably need to get an approval from someone else though, as I haven't really established myself in the libunwind code base. As a side note... one of these days clang / llvm stack compaction may actually work reasonably well, and tricks like this won't be

Re: [PATCH] D20334: [libcxx] Fix a bug in strstreambuf::overflow

2016-05-25 Thread Ben Craig via cfe-commits
bcraig added a comment. In http://reviews.llvm.org/D20334#438052, @ahatanak wrote: > My understanding is that typically std::ends is explicitly appended to > null-terminate the stream buffer. The test case in the example does that. > > http://en.cppreference.com/w/cpp/io/ostrstream/str Doh!

Re: [PATCH] D20328: [libcxx] Externally threaded libc++ variant

2016-05-25 Thread Ben Craig via cfe-commits
bcraig added inline comments. Comment at: include/__threading_support:201 @@ +200,3 @@ +// Mutex +#define _LIBCPP_MUTEX_INITIALIZER nullptr +struct __libcpp_platform_mutex_t; rmaprath wrote: > rmaprath wrote: > > bcraig wrote: > > > rmaprath wrote: > > > > bcraig

Re: [PATCH] D20573: [libcxx] Allow explicit pthread opt-in

2016-05-24 Thread Ben Craig via cfe-commits
bcraig marked 2 inline comments as done. bcraig added a comment. http://reviews.llvm.org/D20573 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D20573: [libcxx] Allow explicit pthread opt-in

2016-05-24 Thread Ben Craig via cfe-commits
bcraig updated this revision to Diff 58320. http://reviews.llvm.org/D20573 Files: CMakeLists.txt include/__config include/__config_site.in include/__threading_support Index: include/__threading_support === ---

Re: [PATCH] D20574: [libcxxabi] Allow explicit pthread opt-in

2016-05-24 Thread Ben Craig via cfe-commits
bcraig marked an inline comment as done. bcraig added a comment. http://reviews.llvm.org/D20574 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D20574: [libcxxabi] Allow explicit pthread opt-in

2016-05-24 Thread Ben Craig via cfe-commits
bcraig updated this revision to Diff 58317. http://reviews.llvm.org/D20574 Files: CMakeLists.txt Index: CMakeLists.txt === --- CMakeLists.txt +++ CMakeLists.txt @@ -113,6 +113,7 @@ option(LIBCXXABI_ENABLE_WERROR "Fail and stop

[PATCH] D20596: [libcxx] Refactor locale switching, creation, and destruction

2016-05-24 Thread Ben Craig via cfe-commits
bcraig created this revision. bcraig added reviewers: mclow.lists, EricWF, joerg, jroelofs. bcraig added a subscriber: cfe-commits. Herald added a subscriber: jfb. This patch cleans up libcxx's usage of newlocale, freelocale, uselocale, and locale_t. First, libcxx no longer defines the posix

Re: [PATCH] D20328: [libcxx] Externally threaded libc++ variant

2016-05-24 Thread Ben Craig via cfe-commits
bcraig added inline comments. Comment at: include/__threading_support:201 @@ +200,3 @@ +// Mutex +#define _LIBCPP_MUTEX_INITIALIZER nullptr +struct __libcpp_platform_mutex_t; rmaprath wrote: > bcraig wrote: > > I'm not sure I like taking the freedom to define

Re: [PATCH] D20334: [libcxx] Fix a bug in strstreambuf::overflow

2016-05-24 Thread Ben Craig via cfe-commits
bcraig added a subscriber: bcraig. bcraig added a comment. I don't believe this is a libcxx bug, but it is a bug in the test code. oss.str(); isn't required to return a null terminated string. std::cout << (char *) requires a null terminated string though. http://reviews.llvm.org/D20334

Re: [PATCH] D20328: [libcxx] Externally threaded libc++ variant

2016-05-24 Thread Ben Craig via cfe-commits
bcraig added a comment. Note: You'll want to look at http://reviews.llvm.org/D20573, as there will be confilicts for whoever submits second. Comment at: include/__threading_support:201 @@ +200,3 @@ +// Mutex +#define _LIBCPP_MUTEX_INITIALIZER nullptr +struct

[PATCH] D20574: [libcxxabi] Allow explicit pthread opt-in

2016-05-24 Thread Ben Craig via cfe-commits
bcraig created this revision. bcraig added reviewers: rmaprath, mclow.lists, EricWF. bcraig added a subscriber: cfe-commits. Companion to http://reviews.llvm.org/D20573 When building libcxxabi in tree (i.e. in llvm/projects/libcxxabi, along with llvm/projects/libcxx), libcxx's __config_site.in

[PATCH] D20573: [libcxx] Allow explicit pthread opt-in

2016-05-24 Thread Ben Craig via cfe-commits
bcraig created this revision. bcraig added reviewers: rmaprath, mclow.lists, EricWF. bcraig added a subscriber: cfe-commits. The existing pthread detection code in __config is pretty good for common operating systems. It doesn't allow cmake-time choices to be made for uncommon operating

Re: [PATCH] D17416: [libcxx] Reorganize locale extension fallbacks. NFCI

2016-05-20 Thread Ben Craig via cfe-commits
bcraig closed this revision. bcraig added a comment. r270213 http://reviews.llvm.org/D17416 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[libcxx] r270213 - Reorganize locale extension fallbacks. NFCI

2016-05-20 Thread Ben Craig via cfe-commits
Author: bcraig Date: Fri May 20 07:58:41 2016 New Revision: 270213 URL: http://llvm.org/viewvc/llvm-project?rev=270213=rev Log: Reorganize locale extension fallbacks. NFCI The various _l locale extension functions originate from very different places. Some come from POSIX, some are BSD

Re: [PATCH] D17416: [libcxx] Reorganize locale extension fallbacks. NFCI

2016-05-17 Thread Ben Craig via cfe-commits
bcraig added a comment. I'll be submitting this Friday morning, barring any objections (I'm out of the office the next couple of days). http://reviews.llvm.org/D17416 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

Re: [PATCH] D17416: [libcxx] Reorganize locale extension fallbacks. NFCI

2016-05-17 Thread Ben Craig via cfe-commits
bcraig updated this revision to Diff 57521. bcraig marked 2 inline comments as done. http://reviews.llvm.org/D17416 Files: include/support/android/locale_bionic.h include/support/newlib/xlocale.h include/support/xlocale/__posix_l_fallback.h include/support/xlocale/__strtonum_fallback.h

Re: [PATCH] D17416: [libcxx] Reorganize locale extension fallbacks. NFCI

2016-05-17 Thread Ben Craig via cfe-commits
bcraig updated this revision to Diff 57520. http://reviews.llvm.org/D17416 Files: include/support/android/locale_bionic.h include/support/newlib/xlocale.h include/support/xlocale/__posix_l_fallback.h include/support/xlocale/__strtonum_fallback.h include/support/xlocale/xlocale.h

Re: [PATCH] D19936: Adding omitted column to invalid loc diagnostic.

2016-05-06 Thread Ben Craig via cfe-commits
bcraig added a comment. r268732 http://reviews.llvm.org/D19936 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

r268732 - Adding omitted column to invalid loc diagnostic.

2016-05-06 Thread Ben Craig via cfe-commits
Author: bcraig Date: Fri May 6 08:29:46 2016 New Revision: 268732 URL: http://llvm.org/viewvc/llvm-project?rev=268732=rev Log: Adding omitted column to invalid loc diagnostic. note_fe_backend_invalid_loc expects three arguments (file, line, column), and will assert when only given two. The

[PATCH] D19936: Adding omitted column to invalid loc diagnostic.

2016-05-04 Thread Ben Craig via cfe-commits
bcraig created this revision. bcraig added reviewers: olista01, t.p.northover. bcraig added a subscriber: cfe-commits. note_fe_backend_invalid_loc expects three arguments (file, line, column), and will assert when only given two. The other two places in this file that use

  1   2   3   >