<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 May 6, 2016, at 1:23 PM, Duncan P. N. Exon Smith <<a href="mailto:dexonsmith@apple.com" class="">dexonsmith@apple.com</a>> wrote:</div><br class="Apple-interchange-newline"><div class=""><span style="font-family: Menlo-Regular; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px; float: none; display: inline !important;" class="">Sorry for the delay answering your questions above, but it looks like we reached consensus anyway :).</span><br style="font-family: Menlo-Regular; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""><br style="font-family: Menlo-Regular; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""><span style="font-family: Menlo-Regular; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px; float: none; display: inline !important;" class="">LGTM with one fixup.</span><br style="font-family: Menlo-Regular; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""><br style="font-family: Menlo-Regular; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""><blockquote type="cite" style="font-family: Menlo-Regular; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class="">On 2016-May-06, at 13:16, Adrian Prantl <<a href="mailto:aprantl@apple.com" class="">aprantl@apple.com</a>> wrote:<br class=""><br class="">aprantl updated this revision to Diff 56450.<br class="">aprantl added a comment.<br class=""><br class="">I get it. Independent flags don't really make sense in this context, and with the >2 check we can later add further bits *to the version number*.<br class=""><br class=""><br class=""><a href="http://reviews.llvm.org/D20004" class="">http://reviews.llvm.org/D20004</a><br class=""><br class="">Files:<br class="">lib/Bitcode/Reader/BitcodeReader.cpp<br class="">lib/Bitcode/Writer/BitcodeWriter.cpp<br class=""><br class="">Index: lib/Bitcode/Writer/BitcodeWriter.cpp<br class="">===================================================================<br class="">--- lib/Bitcode/Writer/BitcodeWriter.cpp<br class="">+++ lib/Bitcode/Writer/BitcodeWriter.cpp<br class="">@@ -1353,7 +1353,8 @@<br class="">void ModuleBitcodeWriter::writeDISubprogram(const DISubprogram *N,<br class=""> SmallVectorImpl<uint64_t> &Record,<br class=""> unsigned Abbrev) {<br class="">- Record.push_back(N->isDistinct());<br class="">+ uint64_t HasUnitFlag = 1 << 1;<br class="">+ Record.push_back(N->isDistinct() | HasUnitFlag);<br class=""> Record.push_back(VE.getMetadataOrNullID(N->getScope()));<br class=""> Record.push_back(VE.getMetadataOrNullID(N->getRawName()));<br class=""> Record.push_back(VE.getMetadataOrNullID(N->getRawLinkageName()));<br class="">Index: lib/Bitcode/Reader/BitcodeReader.cpp<br class="">===================================================================<br class="">--- lib/Bitcode/Reader/BitcodeReader.cpp<br class="">+++ lib/Bitcode/Reader/BitcodeReader.cpp<br class="">@@ -2472,28 +2472,30 @@<br class=""> return error("Invalid record");<br class=""><br class=""> IsDistinct =<br class="">- Record[0] || Record[8]; // All definitions should be distinct.<br class="">+ (Record[0] & 1) || Record[8]; // All definitions should be distinct.<br class=""> // Version 1 has a Function as Record[15].<br class=""> // Version 2 has removed Record[15].<br class=""> // Version 3 has the Unit as Record[15].<br class="">+ bool HasUnit = Record[0] > 2;<br class=""></blockquote><br style="font-family: Menlo-Regular; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""><span style="font-family: Menlo-Regular; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px; float: none; display: inline !important;" class="">Should this be Record[0] >= 2?</span><br style="font-family: Menlo-Regular; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""></div></blockquote><div><br class=""></div><div>I’ll change it for better readability, but it doesn’t really matter. A value of 2 means that the node is not distinct, which implies that it cannot have a unit field. Such a DISubprogram would not survive the Verifier.</div><div><br class=""></div><div>-- adrian</div><blockquote type="cite" class=""><div class=""><br style="font-family: Menlo-Regular; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""><blockquote type="cite" style="font-family: Menlo-Regular; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class="">+ if (HasUnit && Record.size() != 19)<br class="">+ return error("Invalid record");<br class=""> Metadata *CUorFn = getMDOrNull(Record[15]);<br class=""> unsigned Offset = Record.size() == 19 ? 1 : 0;<br class="">- bool HasFn = Offset && dyn_cast_or_null<ConstantAsMetadata>(CUorFn);<br class="">- bool HasCU = Offset && !HasFn;<br class="">+ bool HasFn = Offset && !HasUnit;<br class=""> DISubprogram *SP = GET_OR_DISTINCT(<br class=""> DISubprogram,<br class=""> (Context, getDITypeRefOrNull(Record[1]), getMDString(Record[2]),<br class=""> getMDString(Record[3]), getMDOrNull(Record[4]), Record[5],<br class=""> getMDOrNull(Record[6]), Record[7], Record[8], Record[9],<br class=""> getDITypeRefOrNull(Record[10]), Record[11], Record[12], Record[13],<br class="">- Record[14], HasCU ? CUorFn : nullptr,<br class="">+ Record[14], HasUnit ? CUorFn : nullptr,<br class=""> getMDOrNull(Record[15 + Offset]), getMDOrNull(Record[16 + Offset]),<br class=""> getMDOrNull(Record[17 + Offset])));<br class=""> MetadataList.assignValue(SP, NextMetadataNo++);<br class=""><br class=""> // Upgrade sp->function mapping to function->sp mapping.<br class=""> if (HasFn) {<br class="">- if (auto *CMD = dyn_cast<ConstantAsMetadata>(CUorFn))<br class="">+ if (auto *CMD = dyn_cast_or_null<ConstantAsMetadata>(CUorFn))<br class=""> if (auto *F = dyn_cast<Function>(CMD->getValue())) {<br class=""> if (F->isMaterializable())<br class=""> // Defer until materialized; unmaterialized functions may not have<br class=""><br class=""><br class=""><D20004.56450.patch></blockquote></div></blockquote></div><br class=""></body></html>