From d2d23f5b0465a543bef13f1251ae9776ee019ef8 Mon Sep 17 00:00:00 2001
From: Bruce Forstall <brucefo@microsoft.com>
Date: Mon, 23 Jan 2017 17:19:30 -0800
Subject: [PATCH] Fix GenTree hash function for commutative operators

gtHashValue() computes a hash value for a tree that, at least now,
is only used for determining whether to display the tree in the
JitDump after morphing. The hash function for commutative binary
operators was designed to return the same value if the operands
were switched (possibly this was used for CSE previously?). However,
this means that in some cases, nearly symmetric operands before and
after morphing would cause the "after morph" tree not to be displayed,
which is very confusing, since it implies that morphing had no
effect on the tree.

Since it appears we have no need for this special handling of
commutative binary operators, remove the special hashing done for them.


Commit migrated from https://github.com/dotnet/coreclr/commit/a12b7af9cf3fd9bfa9e57fb5722d81641ac0d053
---
 src/coreclr/src/jit/compiler.h  |  4 ++--
 src/coreclr/src/jit/gentree.cpp | 21 ++++-----------------
 2 files changed, 6 insertions(+), 19 deletions(-)

diff --git a/src/coreclr/src/jit/compiler.h b/src/coreclr/src/jit/compiler.h
index 54079b9..cce4dfb 100644
--- a/src/coreclr/src/jit/compiler.h
+++ b/src/coreclr/src/jit/compiler.h
@@ -2070,13 +2070,13 @@ public:
 
     bool gtHasLocalsWithAddrOp(GenTreePtr tree);
 
-    unsigned gtHashValue(GenTree* tree);
-
     unsigned gtSetListOrder(GenTree* list, bool regs, bool isListCallArgs);
 
     void gtWalkOp(GenTree** op1, GenTree** op2, GenTree* adr, bool constOnly);
 
 #ifdef DEBUG
+    unsigned gtHashValue(GenTree* tree);
+
     GenTreePtr gtWalkOpEffectiveVal(GenTreePtr op);
 #endif
 
diff --git a/src/coreclr/src/jit/gentree.cpp b/src/coreclr/src/jit/gentree.cpp
index 7ed7be4..a9de460 100644
--- a/src/coreclr/src/jit/gentree.cpp
+++ b/src/coreclr/src/jit/gentree.cpp
@@ -2686,6 +2686,8 @@ bool Compiler::gtHasLocalsWithAddrOp(GenTreePtr tree)
     return desc.hasAddrTakenLcl;
 }
 
+#ifdef DEBUG
+
 /*****************************************************************************
  *
  *  Helper used to compute hash values for trees.
@@ -2701,11 +2703,6 @@ inline unsigned genTreeHashAdd(unsigned old, void* add)
     return genTreeHashAdd(old, (unsigned)(size_t)add);
 }
 
-inline unsigned genTreeHashAdd(unsigned old, unsigned add1, unsigned add2)
-{
-    return (old + old / 2) ^ add1 ^ add2;
-}
-
 /*****************************************************************************
  *
  *  Given an arbitrary expression tree, compute a hash value for it.
@@ -2900,18 +2897,6 @@ AGAIN:
 
         unsigned hsh1 = gtHashValue(op1);
 
-        /* Special case: addition of two values */
-
-        if (GenTree::OperIsCommutative(oper))
-        {
-            unsigned hsh2 = gtHashValue(op2);
-
-            /* Produce a hash that allows swapping the operands */
-
-            hash = genTreeHashAdd(hash, hsh1, hsh2);
-            goto DONE;
-        }
-
         /* Add op1's hash to the running value and continue with op2 */
 
         hash = genTreeHashAdd(hash, hsh1);
@@ -3027,6 +3012,8 @@ DONE:
     return hash;
 }
 
+#endif // DEBUG
+
 /*****************************************************************************
  *
  *  Given an arbitrary expression tree, attempts to find the set of all local variables
-- 
2.7.4