Call `gtEffectiveVal` in `gtWalkOp`.
authorPat Gavlin <pagavlin@microsoft.com>
Fri, 30 Sep 2016 20:01:37 +0000 (13:01 -0700)
committerPat Gavlin <pagavlin@microsoft.com>
Fri, 30 Sep 2016 20:03:09 +0000 (13:03 -0700)
This function was using an ad-hoc, open-coded version of
`gtEffectiveVal` that was missing the effective value of `GT_NOP` nodes.
This change replaces these implementations with calls to
`gtEffectiveVal` and makes `gtEffectiveVal` non-recursive.

Fixes #6920.

src/jit/gentree.cpp
src/jit/gentree.h

index f1272b4..fc417d7 100644 (file)
@@ -3673,17 +3673,8 @@ void Compiler::gtWalkOp(GenTree** op1WB, GenTree** op2WB, GenTree* adr, bool con
 {
     GenTreePtr op1 = *op1WB;
     GenTreePtr op2 = *op2WB;
-    GenTreePtr op1EffectiveVal;
 
-    if (op1->gtOper == GT_COMMA)
-    {
-        op1EffectiveVal = op1->gtEffectiveVal();
-        if ((op1EffectiveVal->gtOper == GT_ADD) && (!op1EffectiveVal->gtOverflow()) &&
-            (!constOnly || (op1EffectiveVal->gtOp.gtOp2->IsCnsIntOrI())))
-        {
-            op1 = op1EffectiveVal;
-        }
-    }
+    op1 = op1->gtEffectiveVal();
 
     // Now we look for op1's with non-overflow GT_ADDs [of constants]
     while ((op1->gtOper == GT_ADD) && (!op1->gtOverflow()) && (!constOnly || (op1->gtOp.gtOp2->IsCnsIntOrI())))
@@ -3708,20 +3699,12 @@ void Compiler::gtWalkOp(GenTree** op1WB, GenTree** op2WB, GenTree* adr, bool con
             op2 = tmp;
         }
 
-        if (op1->gtOper == GT_COMMA)
-        {
-            op1EffectiveVal = op1->gtEffectiveVal();
-            if ((op1EffectiveVal->gtOper == GT_ADD) && (!op1EffectiveVal->gtOverflow()) &&
-                (!constOnly || (op1EffectiveVal->gtOp.gtOp2->IsCnsIntOrI())))
-            {
-                op1 = op1EffectiveVal;
-            }
-        }
-
         if (!constOnly && ((op2 == adr) || (!op2->IsCnsIntOrI())))
         {
             break;
         }
+
+        op1 = op1->gtEffectiveVal();
     }
 
     *op1WB = op1;
@@ -3755,15 +3738,7 @@ GenTreePtr Compiler::gtWalkOpEffectiveVal(GenTreePtr op)
 {
     for (;;)
     {
-        if (op->gtOper == GT_COMMA)
-        {
-            GenTreePtr opEffectiveVal = op->gtEffectiveVal();
-            if ((opEffectiveVal->gtOper == GT_ADD) && (!opEffectiveVal->gtOverflow()) &&
-                (opEffectiveVal->gtOp.gtOp2->IsCnsIntOrI()))
-            {
-                op = opEffectiveVal;
-            }
-        }
+        op = op->gtEffectiveVal();
 
         if ((op->gtOper != GT_ADD) || op->gtOverflow() || !op->gtOp.gtOp2->IsCnsIntOrI())
         {
index 9be54c8..7c12f94 100644 (file)
@@ -5099,23 +5099,22 @@ inline GenTreePtr GenTree::gtGetOp2()
 
 inline GenTreePtr GenTree::gtEffectiveVal(bool commaOnly)
 {
-    switch (gtOper)
+    GenTree* effectiveVal = this;
+    for (;;)
     {
-        case GT_COMMA:
-            return gtOp.gtOp2->gtEffectiveVal(commaOnly);
-
-        case GT_NOP:
-            if (!commaOnly && gtOp.gtOp1 != nullptr)
-            {
-                return gtOp.gtOp1->gtEffectiveVal();
-            }
-            break;
-
-        default:
-            break;
+        if (effectiveVal->gtOper == GT_COMMA)
+        {
+            effectiveVal = effectiveVal->gtOp.gtOp2;
+        }
+        else if (!commaOnly && (effectiveVal->gtOper == GT_NOP) && (effectiveVal->gtOp.gtOp1 != nullptr))
+        {
+            effectiveVal = effectiveVal->gtOp.gtOp1;
+        }
+        else
+        {
+            return effectiveVal;
+        }
     }
-
-    return this;
 }
 
 inline GenTree* GenTree::gtSkipReloadOrCopy()