Update fgMorphHWIntrinsic to more closely follow fgMorphSmpOp

Author: tannergooding

src/coreclr/jit/compiler.h

@@ -6691,6 +6691,8 @@ class Compiler
     GenTree* fgOptimizeRelationalComparisonWithFullRangeConst(GenTreeOp* cmp);
 #if defined(FEATURE_HW_INTRINSICS)
     GenTree* fgMorphHWIntrinsic(GenTreeHWIntrinsic* tree);
+    GenTree* fgMorphHWIntrinsicRequired(GenTreeHWIntrinsic* tree);
+    GenTree* fgMorphHWIntrinsicOptional(GenTreeHWIntrinsic* tree);
     GenTree* fgOptimizeHWIntrinsic(GenTreeHWIntrinsic* node);
     GenTree* fgOptimizeHWIntrinsicAssociative(GenTreeHWIntrinsic* node);
 #if defined(FEATURE_MASKED_HW_INTRINSICS)

src/coreclr/jit/morph.cpp

@@ -10938,6 +10938,20 @@ GenTree* Compiler::fgMorphSmpOpOptional(GenTreeOp* tree, bool* optAssertionPropD
 //
 GenTree* Compiler::fgMorphHWIntrinsic(GenTreeHWIntrinsic* tree)
 {
+    // It is important that this follows the general flow of fgMorphSmpOp
+    // * Perform required preorder processing
+    // * Process the operands, in order, if any
+    // * Perform required postorder morphing
+    // * Perform optional postorder morphing if optimizing
+    //
+    // It is also important that similar checks be done where relevant, so
+    // if fgMorphSmpOp does a check for fgGlobalMorph or OptimizationEnabled
+    // so should this method.
+
+    // ------------------------------------------------------------------------
+    // First do any PRE-ORDER processing
+    //
+
     bool allArgsAreConst            = true;
     bool canBenefitFromConstantProp = false;
     bool hasImmediateOperand        = false;
@@ -10955,6 +10969,18 @@ GenTree* Compiler::fgMorphHWIntrinsic(GenTreeHWIntrinsic* tree)
         hasImmediateOperand = true;
     }
 
+#ifdef TARGET_XARCH
+    if (intrinsicId == NI_Vector128_op_Division || intrinsicId == NI_Vector256_op_Division)
+    {
+        fgAddCodeRef(compCurBB, SCK_DIV_BY_ZERO);
+        fgAddCodeRef(compCurBB, SCK_OVERFLOW);
+    }
+#endif // TARGET_XARCH
+
+    // ------------------------------------------------------------------------
+    // Process the operands, if any
+    //
+
     for (GenTree** use : tree->UseEdges())
     {
         *use             = fgMorphTree(*use);
@@ -11004,102 +11030,143 @@ GenTree* Compiler::fgMorphHWIntrinsic(GenTreeHWIntrinsic* tree)
         tree->AddAllEffectsFlags(operand);
     }
 
-#ifdef TARGET_XARCH
-    if (intrinsicId == NI_Vector128_op_Division || intrinsicId == NI_Vector256_op_Division)
+    // ------------------------------------------------------------------------
+    // Now do POST-ORDER processing
+    //
+
+    var_types   retType         = tree->TypeGet();
+    CorInfoType simdBaseJitType = tree->GetSimdBaseJitType();
+    var_types   simdBaseType    = tree->GetSimdBaseType();
+    unsigned    simdSize        = tree->GetSimdSize();
+
+    // Try to fold it, maybe we get lucky,
+    GenTree* morphedTree = gtFoldExpr(tree);
+
+    if ((morphedTree != tree) || !morphedTree->OperIsHWIntrinsic())
     {
-        fgAddCodeRef(compCurBB, SCK_DIV_BY_ZERO);
-        fgAddCodeRef(compCurBB, SCK_OVERFLOW);
+        assert(!fgIsCommaThrow(morphedTree));
+        morphedTree->SetMorphed(this);
     }
-#endif // TARGET_XARCH
-
-    if (opts.OptimizationEnabled())
+    else
     {
-        var_types   retType         = tree->TypeGet();
-        CorInfoType simdBaseJitType = tree->GetSimdBaseJitType();
-        var_types   simdBaseType    = tree->GetSimdBaseType();
-        unsigned    simdSize        = tree->GetSimdSize();
-
-        if (tree->isCommutativeHWIntrinsic())
+        if (allArgsAreConst && tree->IsVectorCreate())
         {
-            assert(tree->GetOperandCount() == 2);
-            GenTree*& op1 = tree->Op(1);
+            // Avoid unexpected CSE for constant arguments for Vector_.Create
+            // but only if all arguments are constants.
 
-            if (op1->IsCnsVec())
+            for (GenTree* arg : tree->Operands())
             {
-                // Move constant vectors from op1 to op2 for commutative operations
-                std::swap(op1, tree->Op(2));
+                arg->SetDoNotCSE();
             }
         }
-        else
-        {
-            bool       isScalar = false;
-            genTreeOps oper     = tree->GetOperForHWIntrinsicId(&isScalar);
 
-            // We can't handle scalar operations since they can copy upper bits from op1
-            if (GenTree::OperIsCompare(oper) && !isScalar)
-            {
-                assert(tree->GetOperandCount() == 2);
+        // ------------------------------------------------------------------------
+        // Perform the required oper-specific postorder morphing
+        //
 
-                GenTree* op1 = tree->Op(1);
-                GenTree* op2 = tree->Op(2);
+        morphedTree = fgMorphHWIntrinsicRequired(tree);
 
-                if (op1->IsCnsVec())
-                {
-                    // Move constant vectors from op1 to op2 for comparison operations
+        // ------------------------------------------------------------------------
+        // Optional morphing is done if tree transformations is permitted
+        //
 
-                    genTreeOps newOper = GenTree::SwapRelop(oper);
-                    var_types  lookupType =
-                        GenTreeHWIntrinsic::GetLookupTypeForCmpOp(this, newOper, retType, simdBaseType, simdSize);
-                    NamedIntrinsic newId =
-                        GenTreeHWIntrinsic::GetHWIntrinsicIdForCmpOp(this, newOper, retType, op2, op1, simdBaseType,
-                                                                     simdSize, false);
+        if ((opts.compFlags & CLFLG_TREETRANS) != 0)
+        {
+            morphedTree = fgMorphHWIntrinsicOptional(morphedTree->AsHWIntrinsic());
+        }
+    }
 
-                    if (newId != NI_Illegal)
-                    {
-                        tree->ResetHWIntrinsicId(newId, op2, op1);
+    if (retType != morphedTree->TypeGet())
+    {
+        assert(varTypeIsMask(morphedTree));
+        morphedTree = gtNewSimdCvtMaskToVectorNode(retType, morphedTree, simdBaseJitType, simdSize);
+        morphedTree = gtFoldExpr(morphedTree);
+    }
+    return morphedTree;
+}
 
-                        if (lookupType != retType)
-                        {
-                            assert(varTypeIsMask(lookupType));
-                            tree->gtType = lookupType;
-                        }
-                    }
-                }
-            }
-        }
+//------------------------------------------------------------------------
+// fgMorphHWIntrinsicRequired: Perform required postorder morphing of a GenTreeHWIntrinsic tree.
+//
+// Arguments:
+//    tree - The tree to morph
+//
+// Return Value:
+//    The morphed tree.
+//
+GenTree* Compiler::fgMorphHWIntrinsicRequired(GenTreeHWIntrinsic* tree)
+{
+    var_types   retType         = tree->TypeGet();
+    CorInfoType simdBaseJitType = tree->GetSimdBaseJitType();
+    var_types   simdBaseType    = tree->GetSimdBaseType();
+    unsigned    simdSize        = tree->GetSimdSize();
 
-        // Try to fold it, maybe we get lucky,
-        GenTree* morphedTree = gtFoldExpr(tree);
+    if (tree->isCommutativeHWIntrinsic())
+    {
+        assert(tree->GetOperandCount() == 2);
+        GenTree*& op1 = tree->Op(1);
 
-        if ((morphedTree != tree) || !morphedTree->OperIsHWIntrinsic())
+        if (op1->OperIsConst())
         {
-            morphedTree->SetMorphed(this);
+            // Move constants from op1 to op2 for commutative operations
+            std::swap(op1, tree->Op(2));
         }
-        else
+    }
+    else
+    {
+        bool       isScalar = false;
+        genTreeOps oper     = tree->GetOperForHWIntrinsicId(&isScalar);
+
+        // We can't handle scalar operations since they can copy upper bits from op1
+        if (GenTree::OperIsCompare(oper) && !isScalar)
         {
-            if (allArgsAreConst && tree->IsVectorCreate())
+            assert(tree->GetOperandCount() == 2);
+
+            GenTree* op1 = tree->Op(1);
+            GenTree* op2 = tree->Op(2);
+
+            if (op1->IsCnsVec())
             {
-                // Avoid unexpected CSE for constant arguments for Vector_.Create
-                // but only if all arguments are constants.
+                // Move constant vectors from op1 to op2 for comparison operations
+
+                genTreeOps newOper = GenTree::SwapRelop(oper);
+                var_types  lookupType =
+                    GenTreeHWIntrinsic::GetLookupTypeForCmpOp(this, newOper, retType, simdBaseType, simdSize);
+                NamedIntrinsic newId = GenTreeHWIntrinsic::GetHWIntrinsicIdForCmpOp(this, newOper, retType, op2, op1,
+                                                                                    simdBaseType, simdSize, false);
 
-                for (GenTree* arg : tree->Operands())
+                if (newId != NI_Illegal)
                 {
-                    arg->SetDoNotCSE();
+                    tree->ResetHWIntrinsicId(newId, op2, op1);
+
+                    if (lookupType != retType)
+                    {
+                        assert(varTypeIsMask(lookupType));
+                        tree->gtType = lookupType;
+                    }
                 }
             }
-
-            morphedTree = fgOptimizeHWIntrinsic(tree);
         }
+    }
 
-        if (retType != morphedTree->TypeGet())
-        {
-            assert(varTypeIsMask(morphedTree));
-            morphedTree = gtNewSimdCvtMaskToVectorNode(retType, morphedTree, simdBaseJitType, simdSize);
-            morphedTree = gtFoldExpr(morphedTree);
-        }
-        return morphedTree;
+    if (opts.OptimizationEnabled())
+    {
+        return fgOptimizeHWIntrinsic(tree);
     }
+    return tree;
+}
 
+//------------------------------------------------------------------------
+// fgMorphHWIntrinsicOptional: Perform optional postorder morphing of a GenTreeHWIntrinsic tree.
+//
+// Arguments:
+//    tree                 - The tree to morph
+//
+// Return Value:
+//    The morphed tree.
+//
+GenTree* Compiler::fgMorphHWIntrinsicOptional(GenTreeHWIntrinsic* tree)
+{
     return tree;
 }
 #endif // FEATURE_HW_INTRINSICS