<html xmlns:v="urn:schemas-microsoft-com:vml" xmlns:o="urn:schemas-microsoft-com:office:office" xmlns:w="urn:schemas-microsoft-com:office:word" xmlns:m="http://schemas.microsoft.com/office/2004/12/omml" xmlns="http://www.w3.org/TR/REC-html40">
<head>
<meta http-equiv="Content-Type" content="text/html; charset=utf-8">
<meta name="Generator" content="Microsoft Word 14 (filtered medium)">
<style><!--
/* Font Definitions */
@font-face
{font-family:Calibri;
panose-1:2 15 5 2 2 2 4 3 2 4;}
@font-face
{font-family:Tahoma;
panose-1:2 11 6 4 3 5 4 4 2 4;}
/* Style Definitions */
p.MsoNormal, li.MsoNormal, div.MsoNormal
{margin:0in;
margin-bottom:.0001pt;
font-size:12.0pt;
font-family:"Times New Roman","serif";}
a:link, span.MsoHyperlink
{mso-style-priority:99;
color:blue;
text-decoration:underline;}
a:visited, span.MsoHyperlinkFollowed
{mso-style-priority:99;
color:purple;
text-decoration:underline;}
span.EmailStyle17
{mso-style-type:personal-reply;
font-family:"Calibri","sans-serif";
color:#1F497D;}
.MsoChpDefault
{mso-style-type:export-only;
font-family:"Calibri","sans-serif";}
@page WordSection1
{size:8.5in 11.0in;
margin:1.0in 1.0in 1.0in 1.0in;}
div.WordSection1
{page:WordSection1;}
--></style><!--[if gte mso 9]><xml>
<o:shapedefaults v:ext="edit" spidmax="1026" />
</xml><![endif]--><!--[if gte mso 9]><xml>
<o:shapelayout v:ext="edit">
<o:idmap v:ext="edit" data="1" />
</o:shapelayout></xml><![endif]-->
</head>
<body lang="EN-US" link="blue" vlink="purple">
<div class="WordSection1">
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1F497D">(This time correctly cc'ing llvm-commits instead of llvm-dev…)<o:p></o:p></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1F497D">Honestly, once you know which attribute/form combination has a problem, it's really not too hard to track down where it gets emitted. Grep is your friend.
Once I realized this version would require a virtual method I was not happy, but carried through to see where it would lead.<o:p></o:p></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1F497D">Going back to the original location (during abbrev emission) is cheap and easy, and with the debugging output it will be sufficient.<o:p></o:p></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1F497D">--paulr<o:p></o:p></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1F497D"><o:p> </o:p></span></p>
<p class="MsoNormal"><a name="_MailEndCompose"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1F497D"><o:p> </o:p></span></a></p>
<div style="border:none;border-left:solid blue 1.5pt;padding:0in 0in 0in 4.0pt">
<div>
<div style="border:none;border-top:solid #B5C4DF 1.0pt;padding:3.0pt 0in 0in 0in">
<p class="MsoNormal"><b><span style="font-size:10.0pt;font-family:"Tahoma","sans-serif"">From:</span></b><span style="font-size:10.0pt;font-family:"Tahoma","sans-serif""> llvm-commits [mailto:llvm-commits-bounces@lists.llvm.org]
<b>On Behalf Of </b>David Blaikie via llvm-commits<br>
<b>Sent:</b> Wednesday, April 12, 2017 11:40 AM<br>
<b>To:</b> reviews+D30785+public+750a0f0570653e04@reviews.llvm.org; aprantl@apple.com; clayborg@gmail.com<br>
<b>Cc:</b> llvm-commits@lists.llvm.org<br>
<b>Subject:</b> Re: [PATCH] D30785: [DWARF] Versioning for DWARF constants; verify FORMs<o:p></o:p></span></p>
</div>
</div>
<p class="MsoNormal"><o:p> </o:p></p>
<div>
<p class="MsoNormal" style="margin-bottom:12.0pt">Adding a vtable to DIE's probably not OK - memory usage there is, I think, a bit of a concern.<br>
<br>
Adrian - thoughts on where this check might be best placed? It'd seem best to make sure it fails at the time the attribute is added to the DIE, rather than during a later walk - but sure, no big deal, probably doesn't come up all that often/not /too/ painful
to debug (look at the attribute type, the DIE memory address, set a conditional breakpoint on DIEValueList::addValue & run again). <br>
<br>
There's a relatively small handful of functions that benefit from this generality, and it might be easy enough to split them (might not be, too) - given that the generality is over blocks and locations that don't have attribute kinds - they're not very much
the same thing.... dunno though.<o:p></o:p></p>
<div>
<div>
<p class="MsoNormal">On Tue, Apr 11, 2017 at 4:22 PM Paul Robinson via Phabricator <<a href="mailto:reviews@reviews.llvm.org">reviews@reviews.llvm.org</a>> wrote:<o:p></o:p></p>
</div>
<blockquote style="border:none;border-left:solid #CCCCCC 1.0pt;padding:0in 0in 0in 6.0pt;margin-left:4.8pt;margin-right:0in">
<p class="MsoNormal" style="margin-bottom:12.0pt">probinson updated this revision to Diff 94905.<br>
probinson added a comment.<br>
<br>
Move the version check to DIE::addValue(). This required making the base-class method virtual, because the base class doesn't have a way to find the DWARF version; only a DIE can do that.<br>
Other review comments addressed as well.<br>
<br>
<br>
<a href="https://reviews.llvm.org/D30785" target="_blank">https://reviews.llvm.org/D30785</a><br>
<br>
Files:<br>
include/llvm/CodeGen/DIE.h<br>
include/llvm/ObjectYAML/DWARFYAML.h<br>
include/llvm/Support/Dwarf.def<br>
include/llvm/Support/Dwarf.h<br>
lib/CodeGen/AsmPrinter/DIE.cpp<br>
lib/Support/Dwarf.cpp<br>
test/DebugInfo/AMDGPU/pointer-address-space-dwarf-v1.ll<br>
test/DebugInfo/AMDGPU/variable-locations-dwarf-v1.ll<br>
tools/dsymutil/DwarfLinker.cpp<br>
unittests/DebugInfo/DWARF/DWARFDebugInfoTest.cpp<o:p></o:p></p>
</blockquote>
</div>
</div>
</div>
</div>
</body>
</html>