From 9667abeeabc847914933915a8a153ef27d49ec7c Mon Sep 17 00:00:00 2001
From: Pat Gavlin <pagavlin@microsoft.com>
Date: Wed, 14 Sep 2016 14:22:52 -0700
Subject: [PATCH] Address more PR feedback.

---
 src/jit/codegenxarch.cpp | 71 ++++++++++++++++++++++--------------------------
 src/jit/gentree.h        |  2 +-
 src/jit/rationalize.cpp  |  2 ++
 3 files changed, 36 insertions(+), 39 deletions(-)

diff --git a/src/jit/codegenxarch.cpp b/src/jit/codegenxarch.cpp
index 0538a66..75cc56d 100644
--- a/src/jit/codegenxarch.cpp
+++ b/src/jit/codegenxarch.cpp
@@ -2489,51 +2489,46 @@ void CodeGen::genCodeForTreeNode(GenTreePtr treeNode)
             assert(compiler->compCurBB->bbJumpKind == BBJ_COND);
 
 #if !defined(_TARGET_64BIT_)
-            // For long compares, we emit special logic
-            if (varTypeIsLong(cmp->gtGetOp1()))
-            {
-                unreached();
-            }
-            else
+            // Long-typed compares should have been handled by Lowering::LowerCompare.
+            assert(!varTypeIsLong(cmp->gtGetOp1()));
 #endif
-            {
-                // Get the "kind" and type of the comparison.  Note that whether it is an unsigned cmp
-                // is governed by a flag NOT by the inherent type of the node
-                // TODO-XArch-CQ: Check if we can use the currently set flags.
-                emitJumpKind jumpKind[2];
-                bool         branchToTrueLabel[2];
-                genJumpKindsForTree(cmp, jumpKind, branchToTrueLabel);
-
-                BasicBlock* skipLabel = nullptr;
-                if (jumpKind[0] != EJ_NONE)
-                {
-                    BasicBlock* jmpTarget;
-                    if (branchToTrueLabel[0])
-                    {
-                        jmpTarget = compiler->compCurBB->bbJumpDest;
-                    }
-                    else
-                    {
-                        // This case arises only for ordered GT_EQ right now
-                        assert((cmp->gtOper == GT_EQ) && ((cmp->gtFlags & GTF_RELOP_NAN_UN) == 0));
-                        skipLabel = genCreateTempLabel();
-                        jmpTarget = skipLabel;
-                    }
 
-                    inst_JMP(jumpKind[0], jmpTarget);
-                }
+            // Get the "kind" and type of the comparison.  Note that whether it is an unsigned cmp
+            // is governed by a flag NOT by the inherent type of the node
+            // TODO-XArch-CQ: Check if we can use the currently set flags.
+            emitJumpKind jumpKind[2];
+            bool         branchToTrueLabel[2];
+            genJumpKindsForTree(cmp, jumpKind, branchToTrueLabel);
 
-                if (jumpKind[1] != EJ_NONE)
+            BasicBlock* skipLabel = nullptr;
+            if (jumpKind[0] != EJ_NONE)
+            {
+                BasicBlock* jmpTarget;
+                if (branchToTrueLabel[0])
                 {
-                    // the second conditional branch always has to be to the true label
-                    assert(branchToTrueLabel[1]);
-                    inst_JMP(jumpKind[1], compiler->compCurBB->bbJumpDest);
+                    jmpTarget = compiler->compCurBB->bbJumpDest;
                 }
-
-                if (skipLabel != nullptr)
+                else
                 {
-                    genDefineTempLabel(skipLabel);
+                    // This case arises only for ordered GT_EQ right now
+                    assert((cmp->gtOper == GT_EQ) && ((cmp->gtFlags & GTF_RELOP_NAN_UN) == 0));
+                    skipLabel = genCreateTempLabel();
+                    jmpTarget = skipLabel;
                 }
+
+                inst_JMP(jumpKind[0], jmpTarget);
+            }
+
+            if (jumpKind[1] != EJ_NONE)
+            {
+                // the second conditional branch always has to be to the true label
+                assert(branchToTrueLabel[1]);
+                inst_JMP(jumpKind[1], compiler->compCurBB->bbJumpDest);
+            }
+
+            if (skipLabel != nullptr)
+            {
+                genDefineTempLabel(skipLabel);
             }
         }
         break;
diff --git a/src/jit/gentree.h b/src/jit/gentree.h
index 3ce0895..53aca89 100644
--- a/src/jit/gentree.h
+++ b/src/jit/gentree.h
@@ -1428,7 +1428,7 @@ public:
         return (gtOper == GT_LIST) && ((gtFlags & GTF_LIST_AGGREGATE) != 0);
     }
 
-    bool OperIsConditionalJump()
+    bool OperIsConditionalJump() const
     {
         return (gtOper == GT_JTRUE) || (gtOper == GT_JCC);
     }
diff --git a/src/jit/rationalize.cpp b/src/jit/rationalize.cpp
index 03e0c9a..4a204c2 100644
--- a/src/jit/rationalize.cpp
+++ b/src/jit/rationalize.cpp
@@ -931,6 +931,8 @@ Compiler::fgWalkResult Rationalizer::RewriteNode(GenTree** useEdge, ArrayStack<G
 #endif // FEATURE_SIMD
 
         default:
+            // JCC nodes should not be present in HIR.
+            assert(node->OperGet() != GT_JCC);
             break;
     }
 
-- 
2.7.4