<div dir="ltr">Sure, which is why you define the semantics to something that makes sense for exposing the issue?<div><br></div><div>How much state you reset/don't reset to accomplish that semantic is up to you :)</div><div><br></div><div>But it's not like it's impossible to have skip have the semantic of "don't do any actual changes for the first x iterations", it just may not be trivial given the structure of instcombine ATM.  That never changes until someone changes it :P</div><div>(and imho, i'd argue that's one reason these bugs creep in.  If it was split up to be more easily testable, people may actually test it more)</div><div>  <br></div><div><div><div><div class="gmail_extra"><br><div class="gmail_quote">On Tue, Mar 21, 2017 at 5:20 PM, Craig Topper <span dir="ltr"><<a href="mailto:craig.topper@gmail.com" target="_blank">craig.topper@gmail.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr">But the contents of the second iteration of the worklist could be very different based on what was skipped and what other changes happened in the first iteration.</div><div class="gmail_extra"><span class="HOEnZb"><font color="#888888"><br clear="all"><div><div class="m_3577439787533725458gmail_signature" data-smartmail="gmail_signature">~Craig</div></div></font></span><div><div class="h5">
<br><div class="gmail_quote">On Tue, Mar 21, 2017 at 5:11 PM, Daniel Berlin <span dir="ltr"><<a href="mailto:dberlin@dberlin.org" target="_blank">dberlin@dberlin.org</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote"><span>On Tue, Mar 21, 2017 at 4:52 PM, Craig Topper via Phabricator via llvm-commits <span dir="ltr"><<a href="mailto:llvm-commits@lists.llvm.org" target="_blank">llvm-commits@lists.llvm.org</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">craig.topper added a comment.<br>
<br>
Yeah I saw that. Should we add a debug counter around the top level worklist loop? The semantics of the "skip" part of the counter wouldn't make sense though.<br>
<div class="m_3577439787533725458m_-583643192982593670HOEnZb"><div class="m_3577439787533725458m_-583643192982593670h5"><br></div></div></blockquote></span><div>That depends on what semantics you want out of it :)</div><div><br></div><div>FWIW: In NewGVN, we want the semantic of the value numbering counter to to be "choose which  instructions are processed on each iteration of the worklist".</div><div><br></div><div>There are two ways to accomplish that:</div><div>1. Record which were skipped, skip them again (some cost)</div><div>or</div><div>2. use the isCounterSet/etc to reset the counter at the start of  each iteration of the worklist to the same values. (no cost if debug is off, though i see i forgot to make this constant when #ifdef NDEBUG)</div><div><br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div class="m_3577439787533725458m_-583643192982593670HOEnZb"><div class="m_3577439787533725458m_-583643192982593670h5">
<br>
<a href="https://reviews.llvm.org/D31120" rel="noreferrer" target="_blank">https://reviews.llvm.org/D3112<wbr>0</a><br>
<br>
<br>
<br>
______________________________<wbr>_________________<br>
llvm-commits mailing list<br>
<a href="mailto:llvm-commits@lists.llvm.org" target="_blank">llvm-commits@lists.llvm.org</a><br>
<a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits" rel="noreferrer" target="_blank">http://lists.llvm.org/cgi-bin/<wbr>mailman/listinfo/llvm-commits</a><br>
</div></div></blockquote></div><br></div></div>
</blockquote></div><br></div></div></div>
</blockquote></div><br></div></div></div></div></div>