<div dir="ltr"><div class="gmail_quote"><div dir="ltr">On Thu, Jun 30, 2016 at 6:36 PM Daniel Berlin via llvm-commits <<a href="mailto:llvm-commits@lists.llvm.org">llvm-commits@lists.llvm.org</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><div><br></div><div>4. The patch introduces a new SCC formation algorithm (double layer with support for CG mutation on the fly), however design document on how this works is missing.  It needs to document </div><div>    * What exactly is expected (in terms of vistation order) when a) an edge is added; b) an edge is deleted; c) when a new node is introduced and connected. </div><div>    * How ref-edges are formed, how indirect callsites are handled etc.</div><div>    * If it is a modification of the classic SCC formation algorithm, describe the change and prove that the algorithm works as expected.</div><div><br></div></div></div></div></blockquote><div><br></div></div></div></div><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><div>FWIW: I agree that for algorithms this complex, where we have no other reference for how they are supposed to work, and no way to know what invariants make them correct or not (other than reading code), we should have some design doc.</div></div></div></div></blockquote><div><br></div><div>Please see my response to David, there is no disagreement here.</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><div><br></div><div>In this case, it should not be hard to prove that you can validly form both ref and non-ref SCC's at once:<br></div></div></div></div></blockquote><div><br></div><div>I'd really like to move this to a separate discussion of LCG. Maybe in response to a separate patch that adds the needed documentation.</div><div><br></div><div>I'm fine if that has to conclude before we review this patch, but this patch is pretty much exclusively using the LCG API to do updates.</div></div></div>