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

Tue Jan 31 15:04:26 PST 2023

philnik added a comment.

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.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133661