<html><head><meta http-equiv="Content-Type" content="text/html charset=utf-8"></head><body style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;" class=""><br class=""><div><blockquote type="cite" class=""><div class="">On Dec 23, 2014, at 4:04 PM, David Blaikie <<a href="mailto:dblaikie@gmail.com" class="">dblaikie@gmail.com</a>> wrote:</div><br class="Apple-interchange-newline"><div class=""><div dir="ltr" class=""><br class=""><div class="gmail_extra"><br class=""><div class="gmail_quote">On Tue, Dec 23, 2014 at 3:55 PM, Adrian Prantl <span dir="ltr" class=""><<a href="mailto:aprantl@apple.com" target="_blank" class="">aprantl@apple.com</a>></span> wrote:<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div style="word-wrap:break-word" class=""><br class=""><div class=""><span class=""><blockquote type="cite" class=""><div class="">On Dec 23, 2014, at 3:42 PM, David Blaikie <<a href="mailto:dblaikie@gmail.com" target="_blank" class="">dblaikie@gmail.com</a>> wrote:</div><br class=""><div class=""><div dir="ltr" class=""><br class=""><div class="gmail_extra"><br class=""><div class="gmail_quote">On Tue, Dec 23, 2014 at 11:11 AM, Adrian Prantl <span dir="ltr" class=""><<a href="mailto:aprantl@apple.com" target="_blank" class="">aprantl@apple.com</a>></span> wrote:<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Author: adrian<br class="">
Date: Tue Dec 23 13:11:47 2014<br class="">
New Revision: 224780<br class="">
<br class="">
URL: <a href="http://llvm.org/viewvc/llvm-project?rev=224780&view=rev" target="_blank" class="">http://llvm.org/viewvc/llvm-project?rev=224780&view=rev</a><br class="">
Log:<br class="">
DIBuilder: Similar to createPointerType, make createMemberPointerType take<br class="">
a size and alignment. Several assertions in DwarfDebug rely on all variable<br class="">
types to report back a size, or to be derived from a type with a size.<br class=""></blockquote><div class=""><br class="">Are these reasonable assertions?<br class=""></div></div></div></div></div></blockquote><div class=""><br class=""></div></span><div class="">In my opinion, yes. I added them to sanity-check DW_OP_piece expressions.</div></div></div></blockquote><div class=""><br class="">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))<br class=""></div></div></div></div></div></blockquote><div><br class=""></div><div>I see this mostly as a way to sanity-check what the optimization passes (sroa, legalize, ..., hopefully more!) do to the debug info. Unless there is a specific frontend that needs the flexibility of not specifying type sizes, I’d rather use the information that we have to make the backend more robust and able to verify the integrity of the metadata. And if there is a frontend that needs to be able to omit type sizes, we could still address that by making those checks optional, while still having the improved safety for clang.</div><div><br class=""></div><div>-- adrian</div><blockquote type="cite" class=""><div class=""><div dir="ltr" class=""><div class="gmail_extra"><div class="gmail_quote"><div class=""> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div style="word-wrap:break-word" class=""><div class=""><span class=""><br class=""><blockquote type="cite" class=""><div class=""><div dir="ltr" class=""><div class="gmail_extra"><div class="gmail_quote"><div class="">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)<br class=""></div></div></div></div></div></blockquote><div class=""><br class=""></div></span><div class="">.. 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.</div><div class=""><br class=""></div><div class=""><div class="">/// constructTypeDIE - Construct derived type die from DIDerivedType.</div><div class="">void DwarfUnit::constructTypeDIE(DIE &Buffer, DIDerivedType DTy) {</div></div><div class=""> ...</div><div class=""> // Add size if non-zero (derived types might be zero-sized.)</div><div class=""> if (Size && Tag != dwarf::DW_TAG_pointer_type)</div><div class=""> addUInt(Buffer, dwarf::DW_AT_byte_size, None, Size);</div></div></div></blockquote><div class=""><br class="">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)<br class=""> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div style="word-wrap:break-word" class=""><div class="">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. </div></div></blockquote><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div style="word-wrap:break-word" class=""><div class=""><span class="HOEnZb"><font color="#888888" class=""><div class=""><br class=""></div><div class="">-- adrian</div></font></span><div class=""><div class="h5"><div class=""><br class=""></div><blockquote type="cite" class=""><div class=""><div dir="ltr" class=""><div class="gmail_extra"><div class="gmail_quote"><div class=""> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br class="">
Tested in CFE.<br class="">
<br class="">
Modified:<br class="">
llvm/trunk/include/llvm/IR/DIBuilder.h<br class="">
llvm/trunk/lib/IR/DIBuilder.cpp<br class="">
<br class="">
Modified: llvm/trunk/include/llvm/IR/DIBuilder.h<br class="">
URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/IR/DIBuilder.h?rev=224780&r1=224779&r2=224780&view=diff" target="_blank" class="">http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/IR/DIBuilder.h?rev=224780&r1=224779&r2=224780&view=diff</a><br class="">
==============================================================================<br class="">
--- llvm/trunk/include/llvm/IR/DIBuilder.h (original)<br class="">
+++ llvm/trunk/include/llvm/IR/DIBuilder.h Tue Dec 23 13:11:47 2014<br class="">
@@ -172,8 +172,12 @@ namespace llvm {<br class="">
<br class="">
/// \brief Create debugging information entry for a pointer to member.<br class="">
/// @param PointeeTy Type pointed to by this pointer.<br class="">
+ /// @param SizeInBits Size.<br class="">
+ /// @param AlignInBits Alignment. (optional)<br class="">
/// @param Class Type for which this pointer points to members of.<br class="">
- DIDerivedType createMemberPointerType(DIType PointeeTy, DIType Class);<br class="">
+ DIDerivedType createMemberPointerType(DIType PointeeTy, DIType Class,<br class="">
+ uint64_t SizeInBits,<br class="">
+ uint64_t AlignInBits = 0);<br class="">
<br class="">
/// createReferenceType - Create debugging information entry for a c++<br class="">
/// style reference or rvalue reference type.<br class="">
<br class="">
Modified: llvm/trunk/lib/IR/DIBuilder.cpp<br class="">
URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/IR/DIBuilder.cpp?rev=224780&r1=224779&r2=224780&view=diff" target="_blank" class="">http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/IR/DIBuilder.cpp?rev=224780&r1=224779&r2=224780&view=diff</a><br class="">
==============================================================================<br class="">
--- llvm/trunk/lib/IR/DIBuilder.cpp (original)<br class="">
+++ llvm/trunk/lib/IR/DIBuilder.cpp Tue Dec 23 13:11:47 2014<br class="">
@@ -328,14 +328,15 @@ DIBuilder::createPointerType(DIType Poin<br class="">
return DIDerivedType(MDNode::get(VMContext, Elts));<br class="">
}<br class="">
<br class="">
-DIDerivedType DIBuilder::createMemberPointerType(DIType PointeeTy,<br class="">
- DIType Base) {<br class="">
+DIDerivedType<br class="">
+DIBuilder::createMemberPointerType(DIType PointeeTy, DIType Base,<br class="">
+ uint64_t SizeInBits, uint64_t AlignInBits) {<br class="">
// Pointer types are encoded in DIDerivedType format.<br class="">
Metadata *Elts[] = {HeaderBuilder::get(dwarf::DW_TAG_ptr_to_member_type)<br class="">
.concat(StringRef())<br class="">
.concat(0) // Line<br class="">
- .concat(0) // Size<br class="">
- .concat(0) // Align<br class="">
+ .concat(SizeInBits) // Size<br class="">
+ .concat(AlignInBits) // Align<br class="">
.concat(0) // Offset<br class="">
.concat(0) // Flags<br class="">
.get(VMContext),<br class="">
<br class="">
<br class="">
_______________________________________________<br class="">
llvm-commits mailing list<br class="">
<a href="mailto:llvm-commits@cs.uiuc.edu" target="_blank" class="">llvm-commits@cs.uiuc.edu</a><br class="">
<a href="http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits" target="_blank" class="">http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits</a><br class="">
</blockquote></div></div></div>
</div></blockquote></div></div></div><br class=""></div></blockquote></div></div></div>
</div></blockquote></div><br class=""></body></html>