[PATCH] D106585: Fix clang debug info irgen of i128 enums

Reid Kleckner via Phabricator via cfe-commits cfe-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 cfe-commits mailing list