<div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><br>
<br>
================<br>
Comment at: lib/Transforms/Scalar/NewGVN.<wbr>cpp:1455<br>
+<br>
+    if (DFSIn < other.DFSIn)<br>
+      return true;<br>
----------------<br>
This is just implementing a lexicographical comparison, right?<br></blockquote><div>Yes </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<br>
If so, I would do:<br>
<br>
```<br>
return std::tie(DFSIn, DFSOut, ...) < std::tie(other.DFSIn, other.DFSOut, ...);<br></blockquote><div><br></div><div><br></div><div>When i started this code, we didn't have tuple (it's that old!), but yeah, this is what should be done now.</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
```<br>
<br>
<br>
================<br>
Comment at: lib/Transforms/Scalar/NewGVN.<wbr>cpp:1464<br>
+        else if (LocalNum == other.LocalNum) {<br>
+          if (Val < other.Val)<br>
+            return true;<br>
----------------<br>
This seems super sketchy, comparing < on pointers. What's up with that? </blockquote><div> It's really just here for completeness,but agree it looks sketchy.<br></div><div> <br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">Won't that make the result nondeterministic?</blockquote><div>No, actually, though i admit it's not obvious to see this.</div><div> It's not possible to push or pop a value for elimination differently  in a way that matters based on the .val (and you should be able to prove this).   Each LLVM instruction only produces one value, and thus  the lowest-level differentiator that really matters for the stack (and what we use as as a replacement) is the local dfs number.</div><div>Everything else in the structure is instruction level, and only affects the order in which we will replace operands of a given instruction.</div><div><br></div><div>For a given instruction (IE things with equal dfsin, dfsout, localnum), the order of replacement of uses does not matter (and no other pass guarantees it, AFAIK)</div><div><br></div><div>IE given,</div><div><br></div><div>a = 5</div><div>b = a + a</div><div><br></div><div>When you hit b, you will have two valuedfs with the same dfsin, out, and localnum.</div><div>The .val will be the same as well.</div><div>The .u's will be different.</div><div><br>You will replace both, and it does not matter what order you replace them in (IE whether you replace operand 2, then operand 1, or operand 1, then operand 2). </div><div><br></div><div><br></div><div>Similarly for the case of same dfsin, dfsout, localnum, but different .val's </div><div><br></div><div>a = 5</div><div>b  = 6</div><div>c = a + b</div><div><br></div><div>in c, we will a valuedfs for a, and one for b,with everything the same but .val  and .u.</div><div>It does not matter what order we replace these operands in.</div><div><br></div><div>You will always end up with the same IR, and this is guaranteed.</div><div><br></div><div><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<br>
<br>
<a href="https://reviews.llvm.org/D26224" rel="noreferrer" target="_blank">https://reviews.llvm.org/<wbr>D26224</a><br>
<br>
<br>
<br>
</blockquote></div><br></div></div>