[release/10.0-preview7] Don't do an invalid mask conversion optimization for conditional select nodes (#118167)

* Don't do an invalid mask conversion optimization for conditional select nodes

* Add test

---------

Co-authored-by: Tanner Gooding <tagoo@outlook.com>
Co-authored-by: EgorBo <egorbo@gmail.com>

Author: 3 people

src/coreclr/jit/gentree.cpp

@@ -28441,22 +28441,6 @@ bool GenTree::OperIsConvertVectorToMask() const
     return false;
 }
 
-//------------------------------------------------------------------------
-// OperIsVectorConditionalSelect: Is this a vector ConditionalSelect hwintrinsic
-//
-// Return Value:
-//    true if the node is a vector ConditionalSelect hwintrinsic
-//    otherwise; false
-//
-bool GenTree::OperIsVectorConditionalSelect() const
-{
-    if (OperIsHWIntrinsic())
-    {
-        return AsHWIntrinsic()->OperIsVectorConditionalSelect();
-    }
-    return false;
-}
-
 //------------------------------------------------------------------------
 // OperIsVectorFusedMultiplyOp: Is this a vector FusedMultiplyOp hwintrinsic
 //

src/coreclr/jit/gentree.h

@@ -1715,7 +1715,6 @@ struct GenTree
     bool OperIsHWIntrinsic(NamedIntrinsic intrinsicId) const;
     bool OperIsConvertMaskToVector() const;
     bool OperIsConvertVectorToMask() const;
-    bool OperIsVectorConditionalSelect() const;
     bool OperIsVectorFusedMultiplyOp() const;
 
     // This is here for cleaner GT_LONG #ifdefs.
@@ -6540,34 +6539,6 @@ struct GenTreeHWIntrinsic : public GenTreeJitIntrinsic
 #endif
     }
 
-    bool OperIsVectorConditionalSelect() const
-    {
-        switch (GetHWIntrinsicId())
-        {
-#if defined(TARGET_XARCH)
-            case NI_Vector128_ConditionalSelect:
-            case NI_Vector256_ConditionalSelect:
-            case NI_Vector512_ConditionalSelect:
-            {
-                return true;
-            }
-#endif // TARGET_XARCH
-
-#if defined(TARGET_ARM64)
-            case NI_AdvSimd_BitwiseSelect:
-            case NI_Sve_ConditionalSelect:
-            {
-                return true;
-            }
-#endif // TARGET_ARM64
-
-            default:
-            {
-                return false;
-            }
-        }
-    }
-
     bool OperIsVectorFusedMultiplyOp() const
     {
         switch (GetHWIntrinsicId())

src/coreclr/jit/optimizemaskconversions.cpp

@@ -175,37 +175,11 @@ class MaskConversionsCheckVisitor final : public GenTreeVisitor<MaskConversionsC
 
                 // Look for:
                 //      user: ConvertVectorToMask(use:LCL_VAR(x)))
-                // -or-
-                //      user: ConditionalSelect(use:LCL_VAR(x), y, z)
 
-                if ((user != nullptr) && user->OperIsHWIntrinsic())
+                if ((user != nullptr) && user->OperIsConvertVectorToMask())
                 {
-                    GenTreeHWIntrinsic* hwintrin = user->AsHWIntrinsic();
-                    NamedIntrinsic      ni       = hwintrin->GetHWIntrinsicId();
-
-                    if (hwintrin->OperIsConvertVectorToMask())
-                    {
-                        convertOp     = user->AsHWIntrinsic();
-                        hasConversion = true;
-                    }
-                    else if (hwintrin->OperIsVectorConditionalSelect())
-                    {
-                        // We don't actually have a convert here, but we do have a case where
-                        // the mask is being used in a ConditionalSelect and therefore can be
-                        // consumed directly as a mask. While the IR shows TYP_SIMD, it gets
-                        // handled in lowering as part of the general embedded-mask support.
-
-                        // We notably don't check that op2 supported embedded masking directly
-                        // because we can still consume the mask directly in such cases. We'll just
-                        // emit `vblendmps zmm1 {k1}, zmm2, zmm3` instead  of containing the CndSel
-                        // as part of something like `vaddps zmm1 {k1}, zmm2, zmm3`
-
-                        if (hwintrin->Op(1) == lclOp)
-                        {
-                            convertOp     = user->AsHWIntrinsic();
-                            hasConversion = true;
-                        }
-                    }
+                    convertOp     = user->AsHWIntrinsic();
+                    hasConversion = true;
                 }
                 break;
             }
@@ -402,7 +376,6 @@ class MaskConversionsUpdateVisitor final : public GenTreeVisitor<MaskConversions
 
             lclOp->Data() = lclOp->Data()->AsHWIntrinsic()->Op(1);
         }
-
         else if (isLocalStore && addConversion)
         {
             // Convert
@@ -416,7 +389,6 @@ class MaskConversionsUpdateVisitor final : public GenTreeVisitor<MaskConversions
             lclOp->Data() = m_compiler->gtNewSimdCvtVectorToMaskNode(TYP_MASK, lclOp->Data(), weight->simdBaseJitType,
                                                                      weight->simdSize);
         }
-
         else if (isLocalUse && removeConversion)
         {
             // Convert
@@ -426,7 +398,6 @@ class MaskConversionsUpdateVisitor final : public GenTreeVisitor<MaskConversions
 
             *use = lclOp;
         }
-
         else if (isLocalUse && addConversion)
         {
             // Convert
@@ -510,7 +481,6 @@ class MaskConversionsUpdateVisitor final : public GenTreeVisitor<MaskConversions
 PhaseStatus Compiler::fgOptimizeMaskConversions()
 {
 #if defined(FEATURE_MASKED_HW_INTRINSICS)
-
     if (opts.OptimizationDisabled())
     {
         JITDUMP("Skipping. Optimizations Disabled\n");

src/tests/JIT/Regression/JitBlue/Runtime_118143/Runtime_118143.cs

@@ -0,0 +1,36 @@
+// Licensed to the .NET Foundation under one or more agreements.
+// The .NET Foundation licenses this file to you under the MIT license.
+
+using System;
+using System.Buffers.Text;
+using System.Linq;
+using System.Runtime.CompilerServices;
+using System.Threading;
+using Xunit;
+
+public class Runtime_118143
+{
+    [Fact]
+    public static void TestEntryPoint()
+    {
+        for (int i = 0; i < 300; i++)
+        {
+            RunBase64Test();
+            Thread.Sleep(1);
+        }
+    }
+
+    [MethodImpl(MethodImplOptions.NoInlining)]
+    private static void RunBase64Test()
+    {
+        byte[] input = new byte[64];
+        byte[] output = new byte[Base64.GetMaxEncodedToUtf8Length(input.Length)];
+        byte[] expected = Convert.FromHexString(
+            "5957466859574668595746685957466859574668595746685957466859574668595746685957466859574668" +
+            "5957466859574668595746685957466859574668595746685957466859574668595746685957466859513D3D");
+        input.AsSpan().Fill((byte)'a');
+        Base64.EncodeToUtf8(input, output, out _, out _);
+        if (!output.SequenceEqual(expected))
+            throw new InvalidOperationException("Invalid Base64 output");
+    }
+}

src/tests/JIT/Regression/JitBlue/Runtime_118143/Runtime_118143.csproj

@@ -0,0 +1,13 @@
+<Project Sdk="Microsoft.NET.Sdk">
+  <PropertyGroup>
+    <Optimize>True</Optimize>
+    <!-- Needed for CLRTestEnvironmentVariable -->
+    <RequiresProcessIsolation>true</RequiresProcessIsolation>
+  </PropertyGroup>
+  <ItemGroup>
+    <Compile Include="$(MSBuildProjectName).cs" />
+    <!-- The issue reproduces only with tiered compilation and PGO enabled -->
+    <CLRTestEnvironmentVariable Include="DOTNET_TieredCompilation" Value="1" />
+    <CLRTestEnvironmentVariable Include="DOTNET_TieredPGO" Value="1" />
+  </ItemGroup>
+</Project>