<div dir="ltr">Aaron, I've followed both links you point to to answer the questions being asked, and to me they don't seem to contain obvious answers. I understand that repeatedly answering the same question over and over again is tedious and we are all busy, but please remind us why both DIFlagTrivial and DIFlagNonTrivial are necessary. IIRC last time I looked at this the DIFlagTrivial flag didn't exist, and it was added since we first discussed this change.</div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Wed, Mar 13, 2019 at 1:04 PM Aaron Smith <<a href="mailto:aaron.smith@microsoft.com" target="_blank">aaron.smith@microsoft.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<div lang="EN-US">
<div class="gmail-m_-2161518181928222128gmail-m_3337286684833778821WordSection1">
<p class="MsoNormal"><span style="font-size:11pt;font-family:Calibri,sans-serif">Hi Paul,<u></u><u></u></span></p>
<p class="MsoNormal"><span style="font-size:11pt;font-family:Calibri,sans-serif"><u></u> <u></u></span></p>
<p class="MsoNormal"><span style="font-size:11pt;font-family:Calibri,sans-serif">There are additional tests here that may answer your questions,<u></u><u></u></span></p>
<p class="MsoNormal"><span style="font-size:11pt;font-family:Calibri,sans-serif"><a href="https://reviews.llvm.org/rGe8475f78e2634d5d348d7ad746efc1e6526e72f5" target="_blank">https://reviews.llvm.org/rGe8475f78e2634d5d348d7ad746efc1e6526e72f5</a><u></u><u></u></span></p>
<p class="MsoNormal"><span style="font-size:11pt;font-family:Calibri,sans-serif"><u></u> <u></u></span></p>
<p class="MsoNormal"><span style="font-size:11pt;font-family:Calibri,sans-serif">Aaron<u></u><u></u></span></p>
<p class="MsoNormal"><span style="font-size:11pt;font-family:Calibri,sans-serif"><u></u> <u></u></span></p>
<div style="border-right:none;border-bottom:none;border-left:none;border-top:1pt solid rgb(181,196,223);padding:3pt 0in 0in">
<p class="MsoNormal"><b><span style="color:black">From: </span></b><span style="color:black">"<a href="mailto:paul.robinson@sony.com" target="_blank">paul.robinson@sony.com</a>" <<a href="mailto:paul.robinson@sony.com" target="_blank">paul.robinson@sony.com</a>><br>
<b>Date: </b>Wednesday, March 13, 2019 at 12:49 PM<br>
<b>To: </b>Aaron Smith <<a href="mailto:aaron.smith@microsoft.com" target="_blank">aaron.smith@microsoft.com</a>>, "<a href="mailto:dblaikie@gmail.com" target="_blank">dblaikie@gmail.com</a>" <<a href="mailto:dblaikie@gmail.com" target="_blank">dblaikie@gmail.com</a>>, "<a href="mailto:rnk@google.com" target="_blank">rnk@google.com</a>" <<a href="mailto:rnk@google.com" target="_blank">rnk@google.com</a>>, "<a href="mailto:aprantl@apple.com" target="_blank">aprantl@apple.com</a>" <<a href="mailto:aprantl@apple.com" target="_blank">aprantl@apple.com</a>>, "<a href="mailto:jdevlieghere@apple.com" target="_blank">jdevlieghere@apple.com</a>" <<a href="mailto:jdevlieghere@apple.com" target="_blank">jdevlieghere@apple.com</a>><br>
<b>Cc: </b>"<a href="mailto:cfe-commits@lists.llvm.org" target="_blank">cfe-commits@lists.llvm.org</a>" <<a href="mailto:cfe-commits@lists.llvm.org" target="_blank">cfe-commits@lists.llvm.org</a>><br>
<b>Subject: </b>RE: r354843 - [CGDebugInfo] Set NonTrivial DIFlag to a c++ record if it's not trivial<u></u><u></u></span></p>
</div>
<div>
<p class="MsoNormal"><span style="font-size:11pt"><u></u> <u></u></span></p>
</div>
<p class="MsoNormal"><span style="font-size:11pt;font-family:Calibri,sans-serif;color:rgb(31,73,125)">Hi Aaron,</span><u></u><u></u></p>
<p class="MsoNormal"><span style="font-size:11pt;font-family:Calibri,sans-serif;color:rgb(31,73,125)">I think I am less clever than David, and it's not clear what the difference is between the two flags. In the review, Zach asked the question but I did not see
a straight answer. What do these flags actually mean? What is the third state, with both flags off, implying not Trivial and not NonTrivial? (At the very least it seems that the names could be improved.)</span><u></u><u></u></p>
<p class="MsoNormal"><span style="font-size:11pt;font-family:Calibri,sans-serif;color:rgb(31,73,125)">Thanks,</span><u></u><u></u></p>
<p class="MsoNormal"><span style="font-size:11pt;font-family:Calibri,sans-serif;color:rgb(31,73,125)">--paulr</span><u></u><u></u></p>
<p class="MsoNormal"><span style="font-size:11pt;font-family:Calibri,sans-serif;color:rgb(31,73,125)"> </span><u></u><u></u></p>
<div style="border-top:none;border-right:none;border-bottom:none;border-left:1.5pt solid blue;padding:0in 0in 0in 4pt">
<div>
<div style="border-right:none;border-bottom:none;border-left:none;border-top:1pt solid rgb(181,196,223);padding:3pt 0in 0in">
<p class="MsoNormal"><b><span style="font-size:10pt;font-family:Tahoma,sans-serif">From:</span></b><span style="font-size:10pt;font-family:Tahoma,sans-serif"> cfe-commits [mailto:<a href="mailto:cfe-commits-bounces@lists.llvm.org" target="_blank">cfe-commits-bounces@lists.llvm.org</a>]
<b>On Behalf Of </b>Aaron Smith via cfe-commits<br>
<b>Sent:</b> Monday, March 04, 2019 6:22 PM<br>
<b>To:</b> David Blaikie; Reid Kleckner; Adrian Prantl; Jonas Devlieghere<br>
<b>Cc:</b> <a href="mailto:cfe-commits@lists.llvm.org" target="_blank">cfe-commits@lists.llvm.org</a><br>
<b>Subject:</b> Re: r354843 - [CGDebugInfo] Set NonTrivial DIFlag to a c++ record if it's not trivial</span><u></u><u></u></p>
</div>
</div>
<p class="MsoNormal"> <u></u><u></u></p>
<div>
<div>
<div>
<p class="MsoNormal">Hi David, I can take a look at adding another test. Please read the code review which answers your question. A new flag is required. Thanks.<u></u><u></u></p>
</div>
</div>
<div>
<p class="MsoNormal"> <u></u><u></u></p>
</div>
</div>
<div class="MsoNormal" align="center" style="text-align:center">
<hr size="0" width="100%" align="center">
</div>
<div id="gmail-m_-2161518181928222128gmail-m_3337286684833778821divRplyFwdMsg">
<p class="MsoNormal"><b><span style="font-size:11pt;font-family:Calibri,sans-serif;color:black">From:</span></b><span style="font-size:11pt;font-family:Calibri,sans-serif;color:black"> David Blaikie <<a href="mailto:dblaikie@gmail.com" target="_blank">dblaikie@gmail.com</a>><br>
<b>Sent:</b> Tuesday, March 5, 2019 12:51:27 AM<br>
<b>To:</b> Aaron Smith; Reid Kleckner; Adrian Prantl; Jonas Devlieghere<br>
<b>Cc:</b> <a href="mailto:cfe-commits@lists.llvm.org" target="_blank">cfe-commits@lists.llvm.org</a><br>
<b>Subject:</b> Re: r354843 - [CGDebugInfo] Set NonTrivial DIFlag to a c++ record if it's not trivial</span>
<u></u><u></u></p>
<div>
<p class="MsoNormal"> <u></u><u></u></p>
</div>
</div>
<div>
<div>
<p class="MsoNormal" style="margin-bottom:12pt">Hi Aaron,<br>
<br>
I don't see any mention of this in D44406 - so it might have been good to have a separate review for this (or included this in the review of D44406, which I think is possible with the monorepo).<br>
<br>
Specifically - this change is missing test coverage (there should be a clang test that goes from C++ source code to LLVM IR & demonstrates the flag being emitted into the IR, etc).<br>
<br>
Also - what's the reason the non-triviality can't be implied by the absence of the trivial flag? (or the other way around) - the flags seem redundant with one another.<u></u><u></u></p>
<div>
<div>
<p class="MsoNormal">On Mon, Feb 25, 2019 at 8:02 PM Aaron Smith via cfe-commits <<a href="mailto:cfe-commits@lists.llvm.org" target="_blank">cfe-commits@lists.llvm.org</a>> wrote:<u></u><u></u></p>
</div>
<blockquote style="border-top:none;border-right:none;border-bottom:none;border-left:1pt solid rgb(204,204,204);padding:0in 0in 0in 6pt;margin:5pt 0in 5pt 4.8pt">
<p class="MsoNormal">Author: asmith<br>
Date: Mon Feb 25 19:49:05 2019<br>
New Revision: 354843<br>
<br>
URL: <a href="https://nam06.safelinks.protection.outlook.com/?url=http%3A%2F%2Fllvm.org%2Fviewvc%2Fllvm-project%3Frev%3D354843%26view%3Drev&data=02%7C01%7Caaron.smith%40microsoft.com%7Ca81fbd6197e04e329a7708d6a7ecf4ce%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C636881033496912406&sdata=lSb5rDLYvfn3Fx25%2FISjPJ1z6sUyvNHnlPBIUFM22aQ%3D&reserved=0" target="_blank">
http://llvm.org/viewvc/llvm-project?rev=354843&view=rev</a><br>
Log:<br>
[CGDebugInfo] Set NonTrivial DIFlag to a c++ record if it's not trivial<br>
<br>
This goes with <a href="https://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Freviews.llvm.org%2FD44406&data=02%7C01%7Caaron.smith%40microsoft.com%7Ca81fbd6197e04e329a7708d6a7ecf4ce%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C636881033496912406&sdata=mJz7kzeqyH%2B5mAld8TA%2BELQ4ouBkVyr2opJyICfEM60%3D&reserved=0" target="_blank">
https://reviews.llvm.org/D44406</a><br>
<br>
<br>
Modified:<br>
cfe/trunk/lib/CodeGen/CGDebugInfo.cpp<br>
<br>
Modified: cfe/trunk/lib/CodeGen/CGDebugInfo.cpp<br>
URL: <a href="https://nam06.safelinks.protection.outlook.com/?url=http%3A%2F%2Fllvm.org%2Fviewvc%2Fllvm-project%2Fcfe%2Ftrunk%2Flib%2FCodeGen%2FCGDebugInfo.cpp%3Frev%3D354843%26r1%3D354842%26r2%3D354843%26view%3Ddiff&data=02%7C01%7Caaron.smith%40microsoft.com%7Ca81fbd6197e04e329a7708d6a7ecf4ce%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C636881033496922411&sdata=Y%2BVdpDTdlUcyZzPpcs4TMB3DQT7eAK%2F5ASLmWM%2Fg73A%3D&reserved=0" target="_blank">
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGDebugInfo.cpp?rev=354843&r1=354842&r2=354843&view=diff</a><br>
==============================================================================<br>
--- cfe/trunk/lib/CodeGen/CGDebugInfo.cpp (original)<br>
+++ cfe/trunk/lib/CodeGen/CGDebugInfo.cpp Mon Feb 25 19:49:05 2019<br>
@@ -3031,6 +3031,8 @@ llvm::DICompositeType *CGDebugInfo::Crea<br>
// Record if a C++ record is trivial type.<br>
if (CXXRD->isTrivial())<br>
Flags |= llvm::DINode::FlagTrivial;<br>
+ else<br>
+ Flags |= llvm::DINode::FlagNonTrivial;<br>
}<br>
<br>
llvm::DICompositeType *RealDecl = DBuilder.createReplaceableCompositeType(<br>
<br>
<br>
_______________________________________________<br>
cfe-commits mailing list<br>
<a href="mailto:cfe-commits@lists.llvm.org" target="_blank">cfe-commits@lists.llvm.org</a><br>
<a href="https://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Flists.llvm.org%2Fcgi-bin%2Fmailman%2Flistinfo%2Fcfe-commits&data=02%7C01%7Caaron.smith%40microsoft.com%7Ca81fbd6197e04e329a7708d6a7ecf4ce%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C636881033496932416&sdata=X%2B6QGG3ySr1r0lyyP7tx2rzxIGfl4giQllnoX8pG6kM%3D&reserved=0" target="_blank">https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits</a><u></u><u></u></p>
</blockquote>
</div>
</div>
</div>
</div>
</div>
</div>
</blockquote></div>