[PATCH] D27551: [DebugInfo] Add regression test for __fp16, float, and double constants.

[David Gross via llvm-commits]

On Mon, Dec 12, 2016 at 4:15 PM, David Blaikie <dblaikie at gmail.com> wrote:

>
>
> On Mon, Dec 12, 2016 at 4:11 PM David Gross <dgross at google.com> wrote:
>
>> Per the Details:
>>
>> Prior to the change "DebugInfo: New metadata representation for global
>>
>> variables." (D20147 <https://reviews.llvm.org/D20147>, D20415
>> <https://reviews.llvm.org/D20415>), clang emitted debug metadata for
>> static
>>
>> const values of floating-point types, but llvm did not translate this
>> debug metadata into DWARF. After that change, clang did not emit debug
>> metadata for static const values of floating-point types, but llvm is
>> capable of translating it into DWARF if it is emitted.
>>
>>
>> My (perhaps overly cautious) concern is that given what has occurred in
>> the past (where llvm was not emitting bytes for floating-point types), it's
>> possible that there might be a change in the future which would cause the
>> emission of the bytes to become type-sensitive again, and I'd like to be
>> sure we do continue to emit the bytes for floating-point types.
>>
>
> Hmm - could you point me to the particular parts of those changes (or
> roughly describe what was going on in the old code, and what goes on in the
> new code) that caused this change in behavior?
>


Briefly, in the old version of clang/llvm, clang emits debug metadata
that's at a higher semantic level (that is, it's not emitting a
DIExpression), and that debug metadata is type-independent (or at least
treats integral types and floating-point types the same); but when llvm
translates that debug metadata into DWARF, it does not do so for
floating-point types.

(Looking back on what I wrote, I realize I just restated the Details from
this patch.)  You could look at the two changes cited in the Details where
the metadata representation changed.

Also, here is a patch applied to the old version of clang/llvm to make a
similar fix.  Whereas for current versions of clang/llvm, the fix is to
modify clang to emit debug metadata for floating-point types (llvm already
does the right thing), for the old version of clang/llvm shown in the
following patches, the fix is to modify llvm to emit DWARF for
floating-point types (clang already does the right thing):
https://android-review.googlesource.com/#/c/312566/1/lib/CodeGen/AsmPrinter/DwarfCompileUnit.cpp


>
> - Dave
>
>
>>
>>
>> On Mon, Dec 12, 2016 at 3:47 PM, David Blaikie <dblaikie at gmail.com>
>> wrote:
>>
>>
>>
>> On Mon, Dec 12, 2016 at 3:24 PM David Gross via Phabricator <
>> reviews at reviews.llvm.org> wrote:
>>
>> dgross added a comment.
>>
>> In https://reviews.llvm.org/D27551#620373, @dblaikie wrote:
>>
>> > Does this need the test coverage if it's just "hey, LLVM can splat
>> these bytes out into DWARF even if we (independently) say the type of those
>> bytes is a float"? The code that emits the bytes is presumably independent
>> of the code that emits the type description, so testing the combination
>> seems unnecessary to me.
>>
>>
>> Can you explain this further?  What parts of this test case are you
>> suggesting may be unnecessary?
>>
>>
>> All of it. (partly under the argument that no code changes were required
>> to provide this functionality, but to be more complete:
>>
>> * We already have test coverage for emitting the correct DW_AT_type value
>> here
>> * We already have test coverage that the arbitrary bytes in the constant
>> value are emitted into the right place in the DW_AT_const_value or whatnot
>>
>> So what's the particularly interesting thing about testing the
>> combination of those two (so far as I can see) orthogonal features?)
>>
>>
>>
>>
>> https://reviews.llvm.org/D27551
>>
>>
>>
>>
>>
>>
>> Normal
>>
>>
>>
>>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20161212/c6213736/attachment.html>