[libcxx-commits] [PATCH] D66177: Update shared_ptr's constructor

[Zoe Carver via Phabricator via libcxx-commits]

zoecarver marked an inline comment as done.
zoecarver added inline comments.


================
Comment at: libcxx/include/memory:3979
+    : __ptr_(__p),
+      __cntrl_(__allocate_shared(__p))
 {
----------------
zoecarver wrote:
> ldionne wrote:
> > Before, we would create the control block with `new`, and with this patch we're using the allocator. Is this correct? Why?
> > 
> > Also, we're now unconditionally using a try-catch block -- I'd be curious to see what the impact on code generation is.
> Yes, it's correct. The allocator will call `__builtin_operator_new` (or `operator new`), which makes it functionally equivalent. When the control block is deallocated, it calls `__a.deallocate`. I think it's better that the control block is also allocated with the allocator (even if they're functionally equivalent) both for cognitive load and in case we introduced an optimization (for example) that broke this type of deallocation down the road. 
> 
> I'll take a look at the codegen. 
Yes, the try-catch-block really hurts the codegen. [[ https://godbolt.org/z/VxvLdA | With ]] and [[ https://godbolt.org/z/YThM7e | without ]] (18 vs 2 lines of assembly).


Repository:
  rCXX libc++

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D66177/new/

https://reviews.llvm.org/D66177