<html><head><meta http-equiv="Content-Type" content="text/html charset=us-ascii"></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 2016-Aug-02, at 10:15, David Blaikie <<a href="mailto:dblaikie@gmail.com" class="">dblaikie@gmail.com</a>> wrote:</div><br class="Apple-interchange-newline"><div class=""><div dir="ltr" style="font-family: Helvetica; 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="">+Duncan in case he has any concerns/ideas/verification for any of this.<br class=""><br class=""><div class="gmail_quote"><div dir="ltr" class="">On Mon, Aug 1, 2016 at 5:11 PM Artem Belevich <<a href="mailto:tra@google.com" class="">tra@google.com</a>> wrote:<br class=""></div><blockquote 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;" class="gmail_quote">tra added a comment.<br class=""><br class="">In<span class="Apple-converted-space"> </span><a href="https://reviews.llvm.org/D22837#502775" rel="noreferrer" target="_blank" class="">https://reviews.llvm.org/D22837#502775</a>, @dblaikie wrote:<br class=""><br class="">> "As far as I can tell, explicitly updating named metadata is not necessary -- updating metadata nodes for variables we've changed appears to maintain metadata consistency. I didn't manage to find a good way to verify it, though. If anyone has suggestions how to reference variable from named metadata, I can add it to the new test."<br class="">><br class="">> Not sure I understand what you were and were not able to verify/test. Could you explain that more/differently?<br class=""><br class=""><br class="">I've verified that DI metadata is consistent after the pass. I.e. llvm.dbg.cu->DICompileUnit:globals->!3->DIGlobalVariable->correct reference to @static_var. Implicitly IR verification pass also verified that there's no leftover DICompileUnits hanging around which was the case before the patch.<br class=""></blockquote><div class=""><br class=""></div><div class="">But the reason that happened was because the Subprogram referenced the original CU, right? So I'd expect to just test that chain: Function->subprogram->CU-> cu named metadata list.<br class=""><br class="">Could you just test that?</div></div></div></div></blockquote><div><br class=""></div><div>Makes sense to me too.</div><br class=""><blockquote type="cite" class=""><div class=""><div dir="ltr" style="font-family: Helvetica; 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 class="gmail_quote"><div class=""> </div><blockquote 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;" class="gmail_quote"><br class="">What I'm not sure about is whether named metadata will always be handled correctly.<br class="">As far as I can tell, named metadata is pretty much read-only. You can only replace it with a new temporary instance.<br class="">Removed code was attempting to manually update named metadata if it happened to reference (directly or not) any IR that the pass changed.<br class=""><br class="">Currently all named metadata is few references away from IR that gets changed. Regular metadata nodes can be modified (all?, some?), so changing a leaf node does not necessarily require replacement of the metadata node that references it. Updating single metadata node appears to be sufficient and does not require to propagate changes further.<br class=""><br class="">However, if there's a way for a global node to refer to IR, that may need explicit replacement of such node.<br class=""><br class="">> The test case looks fairly reasonable to me - but I also wouldn't mind an explanation of all the features (why the artificial/named metadata node? I would expect to just match the CU/Globals/global metadata nodes, and check that the reference (as you have) uses whatever is necessary (the addrspace casts, etc) to still reference that global)<br class=""><br class=""><br class="">Artificial named node is a placeholder for a named metadata node that would have something that is modified by the pass and thus may need to be replaced. So far I've failed to create such metadata.<br class=""></blockquote><div class=""><br class=""></div><div class="">Hopefully Duncan can back (or refute) me up here - but I'd say just remove that as unneeded/not interesting to the reproduction. It seems no different than the actual debug info metadata - a named node that indirectly references the global & gets updated as expected.</div></div></div></div></blockquote><div><br class=""></div><div>Agreed.</div><div><br class=""></div><blockquote type="cite" class=""><div class=""><div dir="ltr" style="font-family: Helvetica; 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 class="gmail_quote"><div class=""> </div></div></div></div></blockquote><blockquote type="cite" class=""><div class=""><div dir="ltr" style="font-family: Helvetica; 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 class="gmail_quote"><blockquote 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;" class="gmail_quote">LLVM language reference says that named metadata can only have node operands, which guarantees at least one level of indirection, so named node can't refer to IR directly (i.e. it can't have DIGlobalVariable operands.). This simplifies things somehow.<br class=""><br class="">What I need to know is -- when a regular metadata node is modified, am I guaranteed that:<br class="">a) either modification happend in-place, without replacing the node,<br class="">b) or, if the node is replaced, all metadata nodes referencing it (including named metadata nodes) are updated appropriately.<br class=""><br class="">If that is true, then we're good. It appears to be the case, but I can't tell if I'm just missing a test case that would prove me wrong.<br class=""></blockquote></div></div></div></blockquote><div><br class=""></div><div>As long as you're using RAUW on the global variables, you shouldn't have anything to worry about. The MDNodes that reference them will be updated to point at the new global variables (through bitcasts, like other IR).</div><div><br class=""></div><div>Note that the MDNodes themselves do not have use lists in general, so they do not know who references them. Their operands will change without changing their address.</div><br class=""><blockquote type="cite" class=""><div class=""><div dir="ltr" style="font-family: Helvetica; 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 class="gmail_quote"><blockquote 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;" class="gmail_quote"><br class="">I guess I can try to see what happens when I apply ReplaceAllUsesWith() to a node used as an operand of named metadata. If that works correctly (i.e it would update named metadata operand to point to the new node), then I'm in the clear. If it does not, then I'll need to figure out how such case should be handled.<br class=""><br class=""><br class=""><a href="https://reviews.llvm.org/D22837" rel="noreferrer" target="_blank" class="">https://reviews.llvm.org/D22837</a></blockquote></div></div></div></blockquote></div><br class=""></body></html>