[llvm] r224780 - DIBuilder: Similar to createPointerType, make createMemberPointerType take

[David Blaikie]

On Tue, Dec 23, 2014 at 3:55 PM, Adrian Prantl <aprantl at apple.com> wrote:
>
>
> On Dec 23, 2014, at 3:42 PM, David Blaikie <dblaikie at gmail.com> wrote:
>
>
>
> On Tue, Dec 23, 2014 at 11:11 AM, Adrian Prantl <aprantl at apple.com> wrote:
>>
>> Author: adrian
>> Date: Tue Dec 23 13:11:47 2014
>> New Revision: 224780
>>
>> URL: http://llvm.org/viewvc/llvm-project?rev=224780&view=rev
>> Log:
>> DIBuilder: Similar to createPointerType, make createMemberPointerType take
>> a size and alignment. Several assertions in DwarfDebug rely on all
>> variable
>> types to report back a size, or to be derived from a type with a size.
>>
>
> Are these reasonable assertions?
>
>
> In my opinion, yes. I added them to sanity-check DW_OP_piece expressions.
>

Perhaps we don't need to sanity check these? I can sort of see it, but on
the other hand we could jsut treat the size in the metadata as being an
opaque value used to emit DWARF, not something we should necessarily rely
on/care about in any semantic way. (that way the absence of the value in
the metadata reflects the absence of the value in the emitted DWARF, etc -
simpler model, allows greater flexibility for clients (some LLVM clients
might have different languages they want to generate debug info for with
different types that they want to include/omit the size from, etc))


>
> Does this change modify the resulting DWARF? (should we be putting a
> DW_AT_size on such things? I assume not, but haven't looked/compared/etc)
>
>
> .. looking.. Actually it does, but that was unintentional. If want to keep
> the old behavior, we should extend this check to also include member
> function pointers.
>
> /// constructTypeDIE - Construct derived type die from DIDerivedType.
> void DwarfUnit::constructTypeDIE(DIE &Buffer, DIDerivedType DTy) {
>   ...
>  // Add size if non-zero (derived types might be zero-sized.)
>   if (Size && Tag != dwarf::DW_TAG_pointer_type)
>     addUInt(Buffer, dwarf::DW_AT_byte_size, None, Size);
>

I don't hugely mind adding a special case for member pointers the same as
pointer_type that's already there - but I do wonder if it's the right
approach (given my reasoning above)


> I’ll do that — just to be consistent with the past. Specifying the size of
> a pointer in DWARF seems redundant as it should be specified by the target;
> assuming no target we want to support has more than one pointer size.
>

> -- adrian
>
>
>
>>
>> Tested in CFE.
>>
>> Modified:
>>     llvm/trunk/include/llvm/IR/DIBuilder.h
>>     llvm/trunk/lib/IR/DIBuilder.cpp
>>
>> Modified: llvm/trunk/include/llvm/IR/DIBuilder.h
>> URL:
>> http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/IR/DIBuilder.h?rev=224780&r1=224779&r2=224780&view=diff
>>
>> ==============================================================================
>> --- llvm/trunk/include/llvm/IR/DIBuilder.h (original)
>> +++ llvm/trunk/include/llvm/IR/DIBuilder.h Tue Dec 23 13:11:47 2014
>> @@ -172,8 +172,12 @@ namespace llvm {
>>
>>      /// \brief Create debugging information entry for a pointer to
>> member.
>>      /// @param PointeeTy Type pointed to by this pointer.
>> +    /// @param SizeInBits  Size.
>> +    /// @param AlignInBits Alignment. (optional)
>>      /// @param Class Type for which this pointer points to members of.
>> -    DIDerivedType createMemberPointerType(DIType PointeeTy, DIType
>> Class);
>> +    DIDerivedType createMemberPointerType(DIType PointeeTy, DIType Class,
>> +                                          uint64_t SizeInBits,
>> +                                          uint64_t AlignInBits = 0);
>>
>>      /// createReferenceType - Create debugging information entry for a
>> c++
>>      /// style reference or rvalue reference type.
>>
>> Modified: llvm/trunk/lib/IR/DIBuilder.cpp
>> URL:
>> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/IR/DIBuilder.cpp?rev=224780&r1=224779&r2=224780&view=diff
>>
>> ==============================================================================
>> --- llvm/trunk/lib/IR/DIBuilder.cpp (original)
>> +++ llvm/trunk/lib/IR/DIBuilder.cpp Tue Dec 23 13:11:47 2014
>> @@ -328,14 +328,15 @@ DIBuilder::createPointerType(DIType Poin
>>    return DIDerivedType(MDNode::get(VMContext, Elts));
>>  }
>>
>> -DIDerivedType DIBuilder::createMemberPointerType(DIType PointeeTy,
>> -                                                 DIType Base) {
>> +DIDerivedType
>> +DIBuilder::createMemberPointerType(DIType PointeeTy, DIType Base,
>> +                                   uint64_t SizeInBits, uint64_t
>> AlignInBits) {
>>    // Pointer types are encoded in DIDerivedType format.
>>    Metadata *Elts[] =
>> {HeaderBuilder::get(dwarf::DW_TAG_ptr_to_member_type)
>>                            .concat(StringRef())
>>                            .concat(0) // Line
>> -                          .concat(0) // Size
>> -                          .concat(0) // Align
>> +                          .concat(SizeInBits) // Size
>> +                          .concat(AlignInBits) // Align
>>                            .concat(0) // Offset
>>                            .concat(0) // Flags
>>                            .get(VMContext),
>>
>>
>> _______________________________________________
>> llvm-commits mailing list
>> llvm-commits at cs.uiuc.edu
>> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20141223/1c92a99c/attachment.html>