<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"><br>
================<br>
Comment at: include/llvm/Transforms/<wbr>Scalar/GVNExpression.h:71-83<br>
+  bool operator==(const Expression &Other) const {<br>
+    if (Opcode != Other.Opcode)<br>
+      return false;<br>
+    if (Opcode == ~0U || Opcode == ~1U)<br>
+      return true;<br>
+    // Compare etype for anything but load and store.<br>
<span class="">+    if (getExpressionType() != ExpressionTypeLoad &&<br>
----------------<br>
</span>The behavior of this operator is quite nonintuitive. It's actually `isTrivallyCongruent()`, not is equal. However if this is the natural equality definition for this type, then it would be fine with a comment explaining that.<br>
<br></blockquote><div><br></div><div>This is deliberate, so yes.</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br>
================<br>
Comment at: include/llvm/Transforms/<wbr>Scalar/GVNExpression.h:87-89<br>
+  virtual hash_code getHashValue() const {<br>
+    return hash_combine(EType, Opcode);<br>
+  }<br>
----------------<br>
This can return a different `hash_code` for Expressions which compare equal.<br></blockquote><div><br></div><div>What example are you thinking of?</div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
================<br>
Comment at: lib/Transforms/Scalar/NewGVN.<wbr>cpp:145<br>
+        return false;<br>
+      return *LHS == *RHS;<br>
+    }<br>
----------------<br>
This seems like the wrong equality operator for the above hash. It will return true for things that don't hash to the same value.<br>
<br></blockquote><div>Again, example?</div><div><br></div></div></div></div>