mclow.lists added inline comments.

================
Comment at: include/vector:318
+    }
+}
+
----------------
Quuxplusone wrote:
> Marshall writes:
> > Instead, we should be writing simple loops that the compiler can optimize 
> > into a call to memcpy if it chooses. Having calls to memcpy in the code 
> > paths makes it impossible to "constexp-ify" this code.
> 
> Well, I have three thoughts on that.
> 
> (A), "removing the calls to memcpy" sounds like you want to just call the 
> *actual* move-constructor in a loop, and then later call the actual 
> destructor in a loop. Which is to say, you don't want libc++ to have a 
> codepath for this speed optimization at all. That's just leaving a ton of 
> performance on the table, and I strongly disagree with that idea.
> 
> (B), regardless, couldn't you achieve that goal simply by taking this patch 
> almost exactly as it is except removing the overloads that take `true_type`? 
> If you want constexpr-friendliness badly enough that you're willing to call 
> the move-constructor and destructor even of trivially copyable types, then 
> you can still use this framework; you just have to remove the overloads that 
> call memcpy. That wouldn't be a major refactoring.
> 
> (C), surely if you want the best of both worlds, you should be pushing 
> someone to invent a constexpr memcpy and/or a way to [detect 
> constexpr-context at compile 
> time](https://quuxplusone.github.io/blog/2018/06/12/perennial-impossibilities/#detect-the-constexprness-of-the-current-context)?
>  I don't think it makes sense to pessimize existing (non-constexpr) users in 
> C++03-through-C++17 just because someone hypothetically might in 
> C++2a-or-later want to mutate a std::vector in a constexpr context.
> Which is to say, you don't want libc++ to have a codepath for this speed 
> optimization at all. 

You're completely correct. I don't want libc++ to have such a code path.
I want clang to generate a `memcpy` from the code w/o ever mentioning `memcpy` 
in the source.




Repository:
  rCXX libc++

https://reviews.llvm.org/D49317



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to