<div dir="ltr"><br><div><br></div><div class="gmail_extra"><br><br><div class="gmail_quote">On Mon, Aug 26, 2013 at 4:28 PM, David Blaikie <span dir="ltr"><<a href="mailto:dblaikie@gmail.com" target="_blank">dblaikie@gmail.com</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><div class="im">On Mon, Aug 26, 2013 at 4:18 PM, Manman Ren <<a href="mailto:manman.ren@gmail.com">manman.ren@gmail.com</a>> wrote:<br>

><br>
><br>
><br>
> On Mon, Aug 26, 2013 at 4:00 PM, David Blaikie <<a href="mailto:dblaikie@gmail.com">dblaikie@gmail.com</a>> wrote:<br>
>><br>
>> On Mon, Aug 26, 2013 at 3:46 PM, Manman Ren <<a href="mailto:manman.ren@gmail.com">manman.ren@gmail.com</a>> wrote:<br>
>> > In r189282 and r189283.<br>
>> ><br>
>> > I will work on clang front-end to generate the unique identifier for<br>
>> > DICompositeType.<br>
>><br>
>> For each field that uses a DIType that may refer to a DICompositeType,<br>
>> we'll see two patches - one for the backend support (to do the<br>
>> necessary indirect access through a map lookup) and frontend support<br>
>> to emit a name.<br>
><br>
><br>
> By frontend support, are you referring to the change to DIBuilder?<br>
<br>
</div>I was thinking of the actual bit where Clang passes in an identifier,<br>
but yes - that'll be fairly mechanical (lots of test cases to update)<br>
& a no-op until DIBuilder actually propagates that to type references.<br>
<div class="im"><br>
> DIBuilder<br>
> will<br>
> inspect the DIType reference, and if it is a DICompositeType and the unique<br>
> identifier is non-null, DIBuilder will use the identifier instead of the<br>
> MDNode itself.<br>
><br>
> Per our discussion via IRC, I thought clang will first generate unique<br>
> identifier for<br>
> external C++ DICompositeTypes. The next step is to update the usage of a<br>
> DIType<br>
> that may refer to a DICompositeType (with one use as a start).<br>
<br>
</div>You're right, this could happen in either order & either will be<br>
testable but have no ultimate effect until both frontend and backend<br>
have been modified.<br>
<br>
So I suppose to get my "one simple case" working, we'd be talking<br>
about having DIBuilder do the "is this a DICompositeType, if so use<br>
the identifier instead of the MDNode" check on exactly one field &<br>
implement the backend support for that too.<br>
<div class="im"><br>
><br>
>><br>
>> The backend patch can be committed prior to the<br>
>> frontend patch (though the frontend patch would be a nice way to<br>
>> generate the backend test to ensure it's what we intend to<br>
>> support/works end-to-end).<br>
>><br>
>> It'd be good to see exactly one (a small one, where possible) use of<br>
>> this first so we can checkin the map creation & lookup plumbing with a<br>
>> single simple field to motivate/demonstrate/test it, then<br>
>> incrementally commit support for each field after that.<br>
><br>
> Agreed.<br>
<br>
</div>OK, so:<br>
<br>
1) add identifier from Clang (DIBuilder support, plus passing in the<br>
identifier from Clang), updating lots of Clang test cases<br></blockquote><div><br></div><div>Attaching patches to implement the above item:</div><div>One patch on llvm side to update DIBuilder interface:</div><div>createClassType, createStructType, createUnionType, createEnumerationType,</div>
<div>and createForwardDecl will take an optional StringRef for clang to pass in the unique</div><div>identifier.</div><div><br></div><div>One patch on clang side to actually generate the unique identifier and pass it</div>
<div>to DIBuilder:</div><div>1> add getUniqueTagTypeName to use CXX mangler to generate a unique identifier for</div><div>external CXX TagType.</div><div>2> call getUniqueTagTypeName and add the type to RetainedTypes in a few places, before</div>
<div>creating a ClassType, StructType, UnionType, EnumerationType, or ForwardDecl.</div><div>3> Input for getOrCreateRecordFwdDecl is modified from RecordDecl to RecordType to enable</div><div>CXX mangler, same applies for CreateEnumType.</div>
<div>4> It is possible that the same Type is pushed to RetainedTypes multiple times, so a SmallPtrSet</div><div>is used locally in finalize() to remove duplication.</div><div><br></div><div>The 3rd patch to update testing cases on clang side:</div>
<div><br></div><div>There is one remaining issue with the current implementation of using CXX mangler</div><div>to generate the unique identifier: CodeGenCXX/debug-info-template.cpp fails with error:</div><div>cannot yet mangle expression type CXXUuidoofExpr.</div>
<div>We can definitely fix the above error in ItaniumMangle.cpp, but I want to see whether the approach I am</div><div>taking is on the right track.</div><div><br></div><div>Thanks,</div><div>Manman</div><div><br></div><div>
 </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">
2) add LLVM support for mapping one particular type field, if it's<br>
string instead of an MDNode, add explicit test cases, possibly<br>
generated by Clang with (2.1)<br>
2.1) (can be separate from (2), probably should be for reviewability -<br>
that does leave (2) semi-untested - a manual test case or test case<br>
generated with (2.1) but committed with (2) might be OK) change<br>
DIBuilder to do the string-instead-of-mdnode thing for that one field.<br>
<br>
(2.1 can't come before 2 or Clang would be broken in the interim -<br>
generating identifier references when LLVM was incapable of doing the<br>
mapping)<br>
<br>
Repeat 2 and 2.1 for one field at a time until done.<br>
</blockquote></div><br></div></div>