<div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote">2017-07-10 11:40 GMT-07:00 Teresa Johnson <span dir="ltr"><<a href="mailto:tejohnson@google.com" target="_blank">tejohnson@google.com</a>></span>:<br><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-style:solid;border-left-color:rgb(204,204,204);padding-left:1ex"><div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote"><span class="gmail-">On Mon, Jul 10, 2017 at 11:36 AM, Dehao Chen via Phabricator <span dir="ltr"><<a href="mailto:reviews@reviews.llvm.org" target="_blank">reviews@reviews.llvm.org</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-style:solid;border-left-color:rgb(204,204,204);padding-left:1ex">danielcdh added a comment.<br>
<br>
How should we proceed with this patch?<br>
<br>
There are indeed some code that iterates through this type, e.g.:<br></blockquote><div><br></div></span><div>Right, that's what I was referring to here: "The only place I see an iteration is when we are computing imports for a module, so I suppose that could affect the debug output ordering, but does that matter (if iteration order is even non-deterministic for DenseMap, and from what I can tell from the LLVM doc linked above it isn't)."</div><div><br></div><div>I don't think this is an issue, and from Mehdi's response it sounds like he agrees.</div><div><br></div><div>I think the patch is good to go. Mehdi - do you concur?</div></div></div></div></blockquote><div><br></div><div>I'm not totally sure: for example we won't call `<span style="font-size:12.800000190734863px">computeImportForFunction` in the same order.</span></div><div><span style="font-size:12.800000190734863px">Right now I don't see how it should matter though.</span></div><div><span style="font-size:12.800000190734863px"><br></span></div><div><span style="font-size:12.800000190734863px">However, as long as the iteration order is deterministic, it does not matter if it is undefined: we still have full reproducibility.</span></div><div><span style="font-size:12.800000190734863px">It would be an issue if we have multiple-threads that lock and insert to this map though, as it would make the iteration order non reproducible. If it is not case I think we're good!</span></div><div><span style="font-size:12.800000190734863px"><br></span></div><div><span style="font-size:12.800000190734863px">Hope it makes sense.</span></div><div><span style="font-size:12.800000190734863px"><br></span></div><div><span style="font-size:12.800000190734863px">-- </span></div><div><span style="font-size:12.800000190734863px">Mehdi</span></div><div><span style="font-size:12.800000190734863px"><br></span></div><div><br></div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-style:solid;border-left-color:rgb(204,204,204);padding-left:1ex"><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><span class="gmail-"><div><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-style:solid;border-left-color:rgb(204,204,204);padding-left:1ex">
<br>
  // Populate the worklist with the import for the functions in the current<br>
  // module<br>
  for (auto &GVSummary : DefinedGVSummaries) {<br>
    if (!Index.isGlobalValueLive(GVSu<wbr>mmary.second)) {<br>
      DEBUG(dbgs() << "Ignores Dead GUID: " << GVSummary.first << "\n");<br>
      continue;<br>
    }<br>
    auto *Summary = GVSummary.second;<br>
    if (auto *AS = dyn_cast<AliasSummary>(Summary<wbr>))<br>
      Summary = &AS->getAliasee();<br>
    auto *FuncSummary = dyn_cast<FunctionSummary>(Summ<wbr>ary);<br>
    if (!FuncSummary)<br>
      // Skip import for global variables<br>
      continue;<br>
    DEBUG(dbgs() << "Initalize import for " << GVSummary.first << "\n");<br>
    computeImportForFunction(*Func<wbr>Summary, Index, ImportInstrLimit,<br>
                             DefinedGVSummaries, Worklist, ImportList,<br>
                             ExportLists);<br>
  }<br>
<br>
<br>
<a href="https://reviews.llvm.org/D35148" rel="noreferrer" target="_blank">https://reviews.llvm.org/D3514<wbr>8</a><br>
<br>
<br>
<br>
</blockquote></span></div><br><br clear="all"><span class="gmail-"><div><br></div>-- <br><div class="gmail-m_-6632033315920490700gmail_signature"><span style="font-family:Times;font-size:medium"><table cellspacing="0" cellpadding="0"><tbody><tr style="color:rgb(85,85,85);font-family:sans-serif;font-size:small"><td nowrap style="border-top-width:2px;border-top-style:solid;border-top-color:rgb(213,15,37)">Teresa Johnson |</td><td nowrap style="border-top-width:2px;border-top-style:solid;border-top-color:rgb(51,105,232)"> Software Engineer |</td><td nowrap style="border-top-width:2px;border-top-style:solid;border-top-color:rgb(0,153,57)"> <a href="mailto:tejohnson@google.com" target="_blank">tejohnson@google.com</a> |</td><td nowrap style="border-top-width:2px;border-top-style:solid;border-top-color:rgb(238,178,17)"> <a href="tel:(408)%20460-2413" value="+14084602413" target="_blank">408-460-2413</a></td></tr></tbody></table></span></div>
</span></div></div>
</blockquote></div><br></div></div>