[PATCH] D106585: Fix clang debug info irgen of i128 enums
Reid Kleckner via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Mon Jul 26 15:57:16 PDT 2021
rnk added a comment.
In D106585#2905663 <https://reviews.llvm.org/D106585#2905663>, @dblaikie wrote:
> In D106585#2902588 <https://reviews.llvm.org/D106585#2902588>, @dblaikie wrote:
>
>> Preserving existing behavior sounds OK - maybe some comment about that it might be good to remove so the next person who looks at it knows there's something not-quite-fully-reasoned here (& the comment about GCC's representation choice still seems off - GCC does use the signedness of the enumerators, not only the enumeration).
>
> @rnk - any thoughts on the comments? (the existing comment about GCC's emission seems incorrect to me (or subtly correct, but probably easy to misunderstand) - GCC does use different DWARF encodings for negative enumerators, depending on the enumerator) And seems like a comment about how this is maybe confusing/in need of some more touch-up might be handy for the next reader? (specifically that the current DWARF backend ignores the "isUnsigned" flag entirely, and relies on the signedness of the enumerator's underlying type instead - so maybe we could remove the isUnsigned flag?)
I wasn't aware that the backend ignored this info. If that's the case, then I think we can carry the signedness over from the enumerator, update the enum2 IRgen test, and someone can update the DWARF backend later if they want to match GCC more closely. I think C and GCC's behavior is driven by a desire to make enumerators behave as close as possible to an integer literal macro definition, which could explain the per-enumerator signedness, similar to how certain ways of spelling a literal affect its type.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D106585/new/
https://reviews.llvm.org/D106585
More information about the llvm-commits
mailing list