[PATCH] D48342: [libcxx] Optimize vectors construction of trivial types from an iterator range with const-ness mismatch.

vsapsai added inline comments. Comment at: libcxx/test/std/containers/sequences/vector/vector.cons/construct_iter_iter.pass.cpp:161 + { +// Though types are different, initialization can be done with `memcpy`. +int32_t array[1] = {-1}; Quuxplusone

[PATCH] D48342: [libcxx] Optimize vectors construction of trivial types from an iterator range with const-ness mismatch.

This revision was not accepted when it landed; it landed in state "Needs Review". This revision was automatically updated to reflect the committed changes. Closed by commit rL350583: [libcxx] Optimize vectors construction of trivial types from an iterator range… (authored by vsapsai, committed

[PATCH] D48342: [libcxx] Optimize vectors construction of trivial types from an iterator range with const-ness mismatch.

Quuxplusone added inline comments. Comment at: libcxx/test/std/containers/sequences/vector/vector.cons/construct_iter_iter.pass.cpp:161 + { +// Though types are different, initialization can be done with `memcpy`. +int32_t array[1] = {-1}; vsapsai

[PATCH] D48342: [libcxx] Optimize vectors construction of trivial types from an iterator range with const-ness mismatch.

vsapsai updated this revision to Diff 180577. vsapsai added a comment. - Test initializing vector of pointers with array of pointers of convertible type. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D48342/new/ https://reviews.llvm.org/D48342 Files: libcxx/include/memory

[PATCH] D48342: [libcxx] Optimize vectors construction of trivial types from an iterator range with const-ness mismatch.

vsapsai added inline comments. Comment at: libcxx/test/std/containers/sequences/vector/vector.cons/construct_iter_iter.pass.cpp:161 + { +// Though types are different, initialization can be done with `memcpy`. +int32_t array[1] = {-1}; Quuxplusone

[PATCH] D48342: [libcxx] Optimize vectors construction of trivial types from an iterator range with const-ness mismatch.

vsapsai added inline comments. Comment at: libcxx/test/std/containers/sequences/vector/vector.cons/construct_iter_iter.pass.cpp:196 test_ctor_under_alloc(); + test_ctor_with_different_value_type(); } Quuxplusone wrote: > vsapsai wrote: > > Quuxplusone

[PATCH] D48342: [libcxx] Optimize vectors construction of trivial types from an iterator range with const-ness mismatch.

Quuxplusone added inline comments. Comment at: libcxx/test/std/containers/sequences/vector/vector.cons/construct_iter_iter.pass.cpp:161 + { +// Though types are different, initialization can be done with `memcpy`. +int32_t array[1] = {-1}; I might add

[PATCH] D48342: [libcxx] Optimize vectors construction of trivial types from an iterator range with const-ness mismatch.

vsapsai updated this revision to Diff 180556. vsapsai added a comment. - Test initializing vector of `unsigned int` with array of `int`. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D48342/new/ https://reviews.llvm.org/D48342 Files: libcxx/include/memory

[PATCH] D48342: [libcxx] Optimize vectors construction of trivial types from an iterator range with const-ness mismatch.

ldionne accepted this revision. ldionne added a comment. LGTM, but I suggest you address @Quuxplusone 's concerns regarding the tests. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D48342/new/ https://reviews.llvm.org/D48342 ___ cfe-commits

[PATCH] D48342: [libcxx] Optimize vectors construction of trivial types from an iterator range with const-ness mismatch.

Quuxplusone added inline comments. Comment at: libcxx/test/std/containers/sequences/vector/vector.cons/construct_iter_iter.pass.cpp:196 test_ctor_under_alloc(); + test_ctor_with_different_value_type(); } vsapsai wrote: > Quuxplusone wrote: > > I suggest

[PATCH] D48342: [libcxx] Optimize vectors construction of trivial types from an iterator range with const-ness mismatch.

vsapsai added inline comments. Comment at: libcxx/test/std/containers/sequences/vector/vector.cons/construct_iter_iter.pass.cpp:196 test_ctor_under_alloc(); + test_ctor_with_different_value_type(); } Quuxplusone wrote: > I suggest that interesting test

[PATCH] D48342: [libcxx] Optimize vectors construction of trivial types from an iterator range with const-ness mismatch.

vsapsai updated this revision to Diff 180316. vsapsai added a comment. - Tweak the test. - Use for `__construct_range_forward` approach recommended by @ldionne (it works with C++03). - Use `__is_default_allocator` for `__construct_forward` and `__construct_backward`. Didn't introduce

[PATCH] D48342: [libcxx] Optimize vectors construction of trivial types from an iterator range with const-ness mismatch.

ldionne added inline comments. Comment at: libcxx/include/memory:1658 +|| !__has_construct::value) && + is_trivially_move_constructible<_DestTp>::value, void Quuxplusone wrote: > Shouldn't this be something like

[PATCH] D48342: [libcxx] Optimize vectors construction of trivial types from an iterator range with const-ness mismatch.

Quuxplusone added inline comments. Comment at: libcxx/include/memory:1645 -template +template _LIBCPP_INLINE_VISIBILITY ldionne wrote: > Quuxplusone wrote: > > ldionne wrote: > > > Coming at it from a slightly different angle, I would think

[PATCH] D48342: [libcxx] Optimize vectors construction of trivial types from an iterator range with const-ness mismatch.

ldionne added inline comments. Comment at: libcxx/include/memory:1645 -template +template _LIBCPP_INLINE_VISIBILITY Quuxplusone wrote: > ldionne wrote: > > Coming at it from a slightly different angle, I would think this is what we > >

[PATCH] D48342: [libcxx] Optimize vectors construction of trivial types from an iterator range with const-ness mismatch.

Quuxplusone added inline comments. Comment at: libcxx/include/memory:1645 -template +template _LIBCPP_INLINE_VISIBILITY ldionne wrote: > Coming at it from a slightly different angle, I would think this is what we > want: > > ``` > template

[PATCH] D48342: [libcxx] Optimize vectors construction of trivial types from an iterator range with const-ness mismatch.

scanon added inline comments. Comment at: libcxx/test/std/containers/sequences/vector/vector.cons/construct_iter_iter.pass.cpp:187 + assert(std::fabs(v[0] - 0.0f) < FLT_EPSILON); + assert(std::fabs(v[1] - 1.0f) < FLT_EPSILON); + assert(std::fabs(v[2] - 2.0f) < FLT_EPSILON);

[PATCH] D48342: [libcxx] Optimize vectors construction of trivial types from an iterator range with const-ness mismatch.

ldionne added a comment. If you use some of my suggestions, please make sure it compiles in C++03 mode. I might be using some C++11 features (not sure whether default argument for template parameters is C++03). Comment at:

[PATCH] D48342: [libcxx] Optimize vectors construction of trivial types from an iterator range with const-ness mismatch.

vsapsai added a comment. Thanks for the feedback. Answered one simple question, the rest needs more time. Comment at: libcxx/test/std/containers/sequences/vector/vector.cons/construct_iter_iter.pass.cpp:186 + std::vector v(array, array + 3); + assert(std::fabs(v[0] - 0.0f)

[PATCH] D48342: [libcxx] Optimize vectors construction of trivial types from an iterator range with const-ness mismatch.

ldionne requested changes to this revision. ldionne added a comment. This revision now requires changes to proceed. Herald added a subscriber: jkorous. I believe this patch fixes an important QOI bug: see http://llvm.org/PR37574. I wholeheartedly agree with Eric that allocators-over-const are an

[PATCH] D48342: [libcxx] Optimize vectors construction of trivial types from an iterator range with const-ness mismatch.

vsapsai updated this revision to Diff 154327. vsapsai added a comment. - Clean up tests according to review. We don't need a new test for custom allocators, parent patch covers that. https://reviews.llvm.org/D48342 Files: libcxx/include/memory

[PATCH] D48342: [libcxx] Optimize vectors construction of trivial types from an iterator range with const-ness mismatch.

vsapsai added inline comments. Comment at: libcxx/include/memory:1479 +struct __has_construct_missing +: false_type +{ Quuxplusone wrote: > vsapsai wrote: > > Quuxplusone wrote: > > > vsapsai wrote: > > > > vsapsai wrote: > > > > > vsapsai wrote: > > > > > >

[PATCH] D48342: [libcxx] Optimize vectors construction of trivial types from an iterator range with const-ness mismatch.

vsapsai added a comment. In https://reviews.llvm.org/D48342#1152063, @EricWF wrote: > Are there any tests which actually exercise the new behavior? Added tests only verify we don't use memcpy erroneously. And existing tests make sure there are no functionality regressions. But there is

[PATCH] D48342: [libcxx] Optimize vectors construction of trivial types from an iterator range with const-ness mismatch.

EricWF added a comment. Are there any tests which actually exercise the new behavior? Comment at: libcxx/include/memory:1665 +(is_same::type> >::value +|| is_same >::value +|| !__has_construct::value) && I'm not sure

[PATCH] D48342: [libcxx] Optimize vectors construction of trivial types from an iterator range with const-ness mismatch.

Quuxplusone added inline comments. Comment at: libcxx/include/memory:1479 +struct __has_construct_missing +: false_type +{ vsapsai wrote: > Quuxplusone wrote: > > vsapsai wrote: > > > vsapsai wrote: > > > > vsapsai wrote: > > > > > erik.pilkington wrote: > >

[PATCH] D48342: [libcxx] Optimize vectors construction of trivial types from an iterator range with const-ness mismatch.

vsapsai added inline comments. Comment at: libcxx/include/memory:1479 +struct __has_construct_missing +: false_type +{ Quuxplusone wrote: > vsapsai wrote: > > vsapsai wrote: > > > vsapsai wrote: > > > > erik.pilkington wrote: > > > > > vsapsai wrote: > > > >

[PATCH] D48342: [libcxx] Optimize vectors construction of trivial types from an iterator range with const-ness mismatch.

vsapsai updated this revision to Diff 154021. vsapsai added a comment. - Proper support for custom allocators without `construct`. https://reviews.llvm.org/D48342 Files: libcxx/include/memory libcxx/test/std/containers/sequences/vector/vector.cons/construct_iter_iter_alloc.pass.cpp

[PATCH] D48342: [libcxx] Optimize vectors construction of trivial types from an iterator range with const-ness mismatch.

Quuxplusone added inline comments. Comment at: libcxx/include/memory:1479 +struct __has_construct_missing +: false_type +{ vsapsai wrote: > vsapsai wrote: > > vsapsai wrote: > > > erik.pilkington wrote: > > > > vsapsai wrote: > > > > > erik.pilkington wrote:

[PATCH] D48342: [libcxx] Optimize vectors construction of trivial types from an iterator range with const-ness mismatch.

vsapsai planned changes to this revision. vsapsai added inline comments. Comment at: libcxx/include/memory:1479 +struct __has_construct_missing +: false_type +{ vsapsai wrote: > vsapsai wrote: > > erik.pilkington wrote: > > > vsapsai wrote: > > > >

[PATCH] D48342: [libcxx] Optimize vectors construction of trivial types from an iterator range with const-ness mismatch.

vsapsai added a comment. In https://reviews.llvm.org/D48342#1148751, @mclow.lists wrote: > I want to point out that this code (not -necessarily- this patch, but where > it lives) needs to be rewritten. > > There is no prohibition on users specializing `allocator_traits` for their > allocators,

[PATCH] D48342: [libcxx] Optimize vectors construction of trivial types from an iterator range with const-ness mismatch.

mclow.lists added a comment. I want to point out that this code (not -necessarily- this patch, but where it lives) needs to be rewritten. There is no prohibition on users specializing `allocator_traits` for their allocators, and yet libc++'s vector depends on the existence of

[PATCH] D48342: [libcxx] Optimize vectors construction of trivial types from an iterator range with const-ness mismatch.

vsapsai added inline comments. Comment at: libcxx/include/memory:1479 +struct __has_construct_missing +: false_type +{ vsapsai wrote: > erik.pilkington wrote: > > vsapsai wrote: > > > erik.pilkington wrote: > > > > Shouldn't this be true_type? > > > I see

[PATCH] D48342: [libcxx] Optimize vectors construction of trivial types from an iterator range with const-ness mismatch.

vsapsai updated this revision to Diff 153431. vsapsai added a comment. Try to exclude changes available in parent review from this review. https://reviews.llvm.org/D48342 Files: libcxx/include/memory

[PATCH] D48342: [libcxx] Optimize vectors construction of trivial types from an iterator range with const-ness mismatch.

vsapsai updated this revision to Diff 153430. vsapsai added a comment. Herald added a subscriber: dexonsmith. - Don't check `!__has_construct` for `__construct_range_forward`. Incompatible types will cause a lack of `construct` but it doesn't mean we should use memcpy instead. And missing

[PATCH] D48342: [libcxx] Optimize vectors construction of trivial types from an iterator range with const-ness mismatch.

vsapsai added inline comments. Comment at: libcxx/include/memory:1479 +struct __has_construct_missing +: false_type +{ erik.pilkington wrote: > vsapsai wrote: > > erik.pilkington wrote: > > > Shouldn't this be true_type? > > I see this is confusing and I'm

[PATCH] D48342: [libcxx] Optimize vectors construction of trivial types from an iterator range with const-ness mismatch.

erik.pilkington added inline comments. Comment at: libcxx/include/memory:1479 +struct __has_construct_missing +: false_type +{ vsapsai wrote: > erik.pilkington wrote: > > Shouldn't this be true_type? > I see this is confusing and I'm still struggling how to

[PATCH] D48342: [libcxx] Optimize vectors construction of trivial types from an iterator range with const-ness mismatch.

vsapsai updated this revision to Diff 152990. vsapsai added a comment. - Don't use memcpy specialization with custom allocators. Not entirely satisfied with comparing allocator to non-const and const, don't know if there is a more elegant way to do the same. https://reviews.llvm.org/D48342

[PATCH] D48342: [libcxx] Optimize vectors construction of trivial types from an iterator range with const-ness mismatch.

vsapsai planned changes to this revision. vsapsai added inline comments. Comment at: libcxx/include/memory:1479 +struct __has_construct_missing +: false_type +{ erik.pilkington wrote: > Shouldn't this be true_type? I see this is confusing and I'm still

[PATCH] D48342: [libcxx] Optimize vectors construction of trivial types from an iterator range with const-ness mismatch.

erik.pilkington added a comment. Hi Volodymyr, thanks for working on this! Comment at: libcxx/include/memory:1479 +struct __has_construct_missing +: false_type +{ Shouldn't this be true_type? Comment at: libcxx/include/memory:1673-1677 +

[PATCH] D48342: [libcxx] Optimize vectors construction of trivial types from an iterator range with const-ness mismatch.

vsapsai added a comment. Ping. https://reviews.llvm.org/D48342 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D48342: [libcxx] Optimize vectors construction of trivial types from an iterator range with const-ness mismatch.

vsapsai added a comment. Related change is https://reviews.llvm.org/D8109 For me the performance improvement was a twice faster execution on a dirty benchmark that doesn't exclude set up and tear down. https://reviews.llvm.org/D48342 ___

[PATCH] D48342: [libcxx] Optimize vectors construction of trivial types from an iterator range with const-ness mismatch.

vsapsai created this revision. vsapsai added reviewers: mclow.lists, EricWF. Herald added a subscriber: christof. We already have a specialization that will use memcpy for construction of trivial types from an iterator range like std::vector(int *, int *); But if we have const-ness mismatch