Arm64 SVE: Fix conditionalselect with constant arguments

src/coreclr/jit/compiler.h

@@ -3205,7 +3205,7 @@ class Compiler
                                     unsigned    simdSize);
 
 #if defined(FEATURE_MASKED_HW_INTRINSICS)
-    GenTreeHWIntrinsic* gtNewSimdCvtVectorToMaskNode(var_types type, GenTree* op1, CorInfoType simdBaseJitType, unsigned simdSize);
+    GenTree* gtNewSimdCvtVectorToMaskNode(var_types type, GenTree* op1, CorInfoType simdBaseJitType, unsigned simdSize);
 #endif // FEATURE_MASKED_HW_INTRINSICS
 
     GenTree* gtNewSimdCreateBroadcastNode(

src/coreclr/jit/gentree.cpp

@@ -18608,30 +18608,6 @@ void GenTreeVecCon::EvaluateBinaryInPlace(genTreeOps oper, bool scalar, var_type
     }
 }
 
-#if defined(TARGET_ARM64)
-//------------------------------------------------------------------------
-// GenTreeVecCon::EvaluateUnaryInPlace: Evaluates this constant using the given operation, when the other
-//                                      operand is a constant mask
-//
-// Arguments:
-//    oper     - the operation to use in the evaluation
-//    scalar   - true if this is a scalar operation; otherwise, false
-//    baseType - the base type of the constant being checked
-//    other    - the mask constant to use in the evaluation
-//
-void GenTreeVecCon::EvaluateBinaryInPlace(genTreeOps oper, bool scalar, var_types baseType, GenTreeMskCon* other)
-{
-    assert(gtType == TYP_SIMD16);
-
-    simd16_t otherSimdVal;
-    EvaluateSimdCvtMaskToVector<simd16_t>(baseType, &otherSimdVal, other->gtSimdMaskVal);
-
-    simd16_t result = {};
-    EvaluateBinarySimd<simd16_t>(oper, scalar, baseType, &result, gtSimd16Val, otherSimdVal);
-    gtSimd16Val = result;
-}
-#endif // TARGET_ARM64
-
 //------------------------------------------------------------------------
 // GenTreeVecCon::EvaluateBroadcastInPlace: Evaluates this constant using a broadcast
 //
@@ -21971,10 +21947,10 @@ GenTree* Compiler::gtNewSimdCvtNativeNode(var_types   type,
 // Return Value:
 //    The node converted to the a mask type
 //
-GenTreeHWIntrinsic* Compiler::gtNewSimdCvtVectorToMaskNode(var_types   type,
-                                                           GenTree*    op1,
-                                                           CorInfoType simdBaseJitType,
-                                                           unsigned    simdSize)
+GenTree* Compiler::gtNewSimdCvtVectorToMaskNode(var_types   type,
+                                                GenTree*    op1,
+                                                CorInfoType simdBaseJitType,
+                                                unsigned    simdSize)
 {
     assert(varTypeIsMask(type));
     assert(varTypeIsSIMD(op1));
@@ -32820,28 +32796,65 @@ GenTree* Compiler::gtFoldExprHWIntrinsic(GenTreeHWIntrinsic* tree)
             case NI_Vector512_ConditionalSelect:
 #elif defined(TARGET_ARM64)
             case NI_AdvSimd_BitwiseSelect:
-            case NI_Sve_ConditionalSelect:
 #endif
             {
                 assert(!varTypeIsMask(retType));
+                assert(!varTypeIsMask(op1));
 
                 if (cnsNode != op1)
                 {
                     break;
                 }
 
-#if defined(TARGET_ARM64)
-                if (ni == NI_Sve_ConditionalSelect)
+                if (op1->IsVectorAllBitsSet())
                 {
-                    assert(!op1->IsVectorAllBitsSet() && !op1->IsVectorZero());
+                    if ((op3->gtFlags & GTF_SIDE_EFFECT) != 0)
+                    {
+                        // op3 has side effects, this would require us to append a new statement
+                        // to ensure that it isn't lost, which isn't safe to do from the general
+                        // purpose handler here. We'll recognize this and mark it in VN instead
+                        break;
+                    }
+
+                    // op3 has no side effects, so we can return op2 directly
+                    return op2;
                 }
-                else
+
+                if (op1->IsVectorZero())
+                {
+                    return gtWrapWithSideEffects(op3, op2, GTF_ALL_EFFECT);
+                }
+
+                if (op2->IsCnsVec() && op3->IsCnsVec())
                 {
-                    assert(!op1->IsTrueMask(simdBaseType) && !op1->IsFalseMask());
+                    // op2 = op2 & op1
+                    op2->AsVecCon()->EvaluateBinaryInPlace(GT_AND, false, simdBaseType, op1->AsVecCon());
+
+                    // op3 = op2 & ~op1
+                    op3->AsVecCon()->EvaluateBinaryInPlace(GT_AND_NOT, false, simdBaseType, op1->AsVecCon());
+
+                    // op2 = op2 | op3
+                    op2->AsVecCon()->EvaluateBinaryInPlace(GT_OR, false, simdBaseType, op3->AsVecCon());
+
+                    resultNode = op2;
                 }
-#endif
+                break;
+            }
+
+#if defined(TARGET_ARM64)
+            case NI_Sve_ConditionalSelect:
+            {
+                assert(!varTypeIsMask(retType));
+                assert(varTypeIsMask(op1));
+
+                if (cnsNode != op1)
+                {
+                    break;
+                }
+
+                assert(!op1->IsVectorAllBitsSet() && !op1->IsVectorZero());
 
-                if (op1->IsVectorAllBitsSet() || op1->IsTrueMask(simdBaseType))
+                if (op1->IsTrueMask(simdBaseType))
                 {
                     if ((op3->gtFlags & GTF_SIDE_EFFECT) != 0)
                     {
@@ -32855,40 +32868,39 @@ GenTree* Compiler::gtFoldExprHWIntrinsic(GenTreeHWIntrinsic* tree)
                     return op2;
                 }
 
-                if (op1->IsVectorZero() || op1->IsFalseMask())
+                if (op1->IsFalseMask())
                 {
                     return gtWrapWithSideEffects(op3, op2, GTF_ALL_EFFECT);
                 }
 
                 if (op2->IsCnsVec() && op3->IsCnsVec())
                 {
-                    if (op1->IsCnsVec())
-                    {
-                        // op2 = op2 & op1
-                        op2->AsVecCon()->EvaluateBinaryInPlace(GT_AND, false, simdBaseType, op1->AsVecCon());
+                    assert(op2->gtType == TYP_SIMD16);
+                    assert(op3->gtType == TYP_SIMD16);
 
-                        // op3 = op2 & ~op1
-                        op3->AsVecCon()->EvaluateBinaryInPlace(GT_AND_NOT, false, simdBaseType, op1->AsVecCon());
-                    }
-#if defined(TARGET_ARM64)
-                    else if (op1->IsCnsMsk())
-                    {
-                        assert(ni == NI_Sve_ConditionalSelect);
+                    simd16_t op1SimdVal;
+                    EvaluateSimdCvtMaskToVector<simd16_t>(simdBaseType, &op1SimdVal, op1->AsMskCon()->gtSimdMaskVal);
 
-                        // op2 = op2 & op1
-                        op2->AsVecCon()->EvaluateBinaryInPlace(GT_AND, false, simdBaseType, op1->AsMskCon());
+                    // op2 = op2 & op1
+                    simd16_t result = {};
+                    EvaluateBinarySimd<simd16_t>(GT_AND, false, simdBaseType, &result, op2->AsVecCon()->gtSimd16Val,
+                                                 op1SimdVal);
+                    op2->AsVecCon()->gtSimd16Val = result;
+
+                    // op3 = op2 & ~op1
+                    result = {};
+                    EvaluateBinarySimd<simd16_t>(GT_AND_NOT, false, simdBaseType, &result, op3->AsVecCon()->gtSimd16Val,
+                                                 op1SimdVal);
+                    op3->AsVecCon()->gtSimd16Val = result;
 
-                        // op3 = op2 & ~op1
-                        op3->AsVecCon()->EvaluateBinaryInPlace(GT_AND_NOT, false, simdBaseType, op1->AsMskCon());
-                    }
-#endif
                     // op2 = op2 | op3
                     op2->AsVecCon()->EvaluateBinaryInPlace(GT_OR, false, simdBaseType, op3->AsVecCon());
 
                     resultNode = op2;
                 }
                 break;
             }
+#endif // TARGET_ARM64
 
             default:
             {

src/coreclr/jit/gentree.h

@@ -6851,9 +6851,6 @@ struct GenTreeVecCon : public GenTree
 
     void EvaluateUnaryInPlace(genTreeOps oper, bool scalar, var_types baseType);
     void EvaluateBinaryInPlace(genTreeOps oper, bool scalar, var_types baseType, GenTreeVecCon* other);
-#if defined(TARGET_ARM64)
-    void EvaluateBinaryInPlace(genTreeOps oper, bool scalar, var_types baseType, GenTreeMskCon* other);
-#endif
 
     template <typename TBase>
     void EvaluateBroadcastInPlace(TBase scalar)

src/coreclr/jit/lowerarmarch.cpp

@@ -1193,9 +1193,8 @@ GenTree* Lowering::LowerCnsMask(GenTreeMskCon* mask)
     EvaluateSimdCvtMaskToVector<simd16_t>(parentBaseType, &vecCon->gtSimdVal, mask->gtSimdMaskVal);
     BlockRange().InsertBefore(mask, vecCon);
 
-    GenTreeHWIntrinsic* convertedVec =
-        comp->gtNewSimdCvtVectorToMaskNode(TYP_MASK, vecCon, parentSimdBaseJitType, parentSimdSize);
-    BlockRange().InsertBefore(mask, convertedVec->Op(1));
+    GenTree* convertedVec = comp->gtNewSimdCvtVectorToMaskNode(TYP_MASK, vecCon, parentSimdBaseJitType, parentSimdSize);
+    BlockRange().InsertBefore(mask, convertedVec->AsHWIntrinsic()->Op(1));
     BlockRange().InsertBefore(mask, convertedVec);
 
     use.ReplaceWith(convertedVec);