<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 Jan 7, 2015, at 4:04 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: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: 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="">Actually, no -- see below:</span><br style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: 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: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: 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: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: 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 2015-Jan-07, at 16:02, Pete Cooper <<a href="mailto:peter_cooper@apple.com" class="">peter_cooper@apple.com</a>> wrote:<br class=""><br class="">Hi Duncan<br class=""><br class="">I assume that prior to these commits clang is already using MDNode::getDistinct? Otherwise I can’t see how your clang patch would work here.<br class=""><br class="">Otherwise LGTM.<br class=""><br class="">Thanks,<br class="">Pete<br class=""><blockquote type="cite" class="">On Jan 7, 2015, at 3:27 PM, Duncan P. N. Exon Smith <<a href="mailto:dexonsmith@apple.com" class="">dexonsmith@apple.com</a>> wrote:<br class=""><br class="">In order to "defeat" `MDNode` uniquing, metadata schemas resort to<br class="">using self-references to prevent merging.<br class=""><br class="">Introduce a `distinct` designator which explicitly requests this<br class="">behaviour, without the need for a self-reference. These nodes are<br class="">still stored in the `LLVMContext`, but are not uniqued based on<br class="">their operands (well, not uniqued at all).<br class=""><br class="">- This concept *already exists*. Before the metadata/value split,<br class=""> it was used whenever operands went to null (to prevent teardown<br class=""> madness, but it happened more frequently). It's still in use<br class=""> for self-referencing `MDNode`s, as well as when<br class=""> `MDNode::replaceOperandWith()` causes a uniquing collision (see<br class=""> module flags behaviour before r225397 for an example).<br class=""></blockquote></blockquote><br style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: 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: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: 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="">^ Distinct `MDNode`s are implicitly created for self-references and on</span><br style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: 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: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: 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="">uniquing collisions.</span><br style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: 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>Cool. Still LGTM then :)</div><div><blockquote type="cite" class=""><div class=""><br style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: 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: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: 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" class=""><br class=""> Some recent commits have already exposed `getDistinct()` and<br class=""> `isDistinct()` as API (see r225401 and r225406).<br class=""><br class="">- The first patch adds assembly/bitcode support. I chose the<br class=""> 'distinct' keyword. It has an accompanying clang patch to<br class=""> update testcases.<br class=""><br class="">- The second patch adds support to `MapMetadata()` to maintain the<br class=""> 'distinct'-ness.<br class=""><br class="">- The third patch just clears out the TODO.<br class=""><br class="">I haven't updated any metadata schemas here; I figure the owners of<br class="">the schemas can update those in their own time.<br class=""><br class="">My main concern here was about how to deal with `MapMetadata()` --<br class="">it feels wrong to just duplicate everything. However, I looked<br class="">through all the callers. In *most* cases RF_NoModuleLevelChanges<br class="">is passed, and in the others this naive solution seems correct (or<br class="">at least harmless).<br class=""><br class="">If this becomes an issue (i.e., there's a place with module-level<br class="">changes where distinct metadata should only be duplicated when<br class="">operands change), we can add another flag to decide whether to<br class="">duplicate it. For now, this just matches the behaviour we already<br class="">get out of self-references.<br class=""><br class=""><0001-IR-Add-distinct-MDNodes-to-bitcode-and-assembly.patch><clang.patch><0002-Utils-Keep-distinct-MDNodes-distinct-in-MapMetadata.patch><0003-IR-Drop-TODO-now-that-PR22111-is-finished.patch>_______________________________________________<br class="">llvm-commits mailing list<br class=""><a href="mailto:llvm-commits@cs.uiuc.edu" class="">llvm-commits@cs.uiuc.edu</a><br class="">http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits</blockquote></blockquote></div></blockquote></div><br class=""></body></html>