[libcxx-commits] [PATCH] D133661: [libc++] Improve binary size when using __transaction

[Nikolas Klauser via Phabricator via libcxx-commits]

philnik added a comment.

In D133661#4095014 <https://reviews.llvm.org/D133661#4095014>, @dblaikie wrote:

> In D133661#4094932 <https://reviews.llvm.org/D133661#4094932>, @philnik wrote:
>
>> In D133661#4094917 <https://reviews.llvm.org/D133661#4094917>, @dblaikie wrote:
>>
>>>> the definition of the `__exception_guard` class being different is somehow more problematic.
>>>
>>> That's correct - Clang/LLVM debug info under LTO does require structures with linkage to have consistent size at least. 
>>> The sort of errors are:
>>>
>>>   fragment is larger than or outside of variable
>>>     call void @llvm.dbg.declare(metadata ptr undef, metadata !4274, metadata !DIExpression(DW_OP_LLVM_fragment, 200, 56)), !dbg !4288
>>>   !4274 = !DILocalVariable(name: "__guard", scope: !4275, file: !2826, line: 550, type: !2906)
>>>
>>> Because LLVM LTO is validating that the debug info describing a variable makes sense given the type - but the type is deduplicated based on the linkage name of the type - so some debug info was emitted in some +exceptions code that is larger than the type definition taken from some -exceptions code.
>>>
>>> I think some possible fixes would include
>>>
>>> - make the structure the same size regardless of +/-exceptions
>>> - use a macro to choose between two differently-linkage-named types (yeah, this is still an ODR violaiton for the function definitions that use these different entities - but within the realms of things we do for +/-exception compatibility, and nothing validates that ODR function definitions are identical)
>>>
>>> I'll see if I can repro this with godbolt, but I think this ^ should be enough to justify a timely revert and/or fix. This patch is broken for mixed exceptions LTO builds.
>>
>> Please don't revert. Other patches depend on this and it should be a trivial fix. We can just add an ABI tag to the noexcept version of the class IIUC. Could you confirm that? If that is the case I can make a patch.
>
> Not quite sure what you mean - got a rough example of what an ABI tag would look like in this case? (looking through the libc++ sources notihng immediately stands out) - an extra template parameter (I see "Abi" template parameters in some places, for instance) or some other technique?

I mean `[[gnu::abi_tag("-fno-exceptions")]]`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133661