JIT: Non-void ThrowHelpers

src/coreclr/jit/flowgraph.cpp

@@ -684,31 +684,16 @@ PhaseStatus Compiler::fgImport()
 
 bool Compiler::fgIsThrow(GenTree* tree)
 {
-    if ((tree->gtOper != GT_CALL) || (tree->AsCall()->gtCallType != CT_HELPER))
+    if (!tree->IsCall())
     {
         return false;
     }
-
-    // TODO-Throughput: Replace all these calls to eeFindHelper() with a table based lookup
-
-    if ((tree->AsCall()->gtCallMethHnd == eeFindHelper(CORINFO_HELP_OVERFLOW)) ||
-        (tree->AsCall()->gtCallMethHnd == eeFindHelper(CORINFO_HELP_VERIFICATION)) ||
-        (tree->AsCall()->gtCallMethHnd == eeFindHelper(CORINFO_HELP_RNGCHKFAIL)) ||
-        (tree->AsCall()->gtCallMethHnd == eeFindHelper(CORINFO_HELP_THROWDIVZERO)) ||
-        (tree->AsCall()->gtCallMethHnd == eeFindHelper(CORINFO_HELP_THROWNULLREF)) ||
-        (tree->AsCall()->gtCallMethHnd == eeFindHelper(CORINFO_HELP_THROW)) ||
-        (tree->AsCall()->gtCallMethHnd == eeFindHelper(CORINFO_HELP_RETHROW)) ||
-        (tree->AsCall()->gtCallMethHnd == eeFindHelper(CORINFO_HELP_THROW_TYPE_NOT_SUPPORTED)) ||
-        (tree->AsCall()->gtCallMethHnd == eeFindHelper(CORINFO_HELP_THROW_PLATFORM_NOT_SUPPORTED)))
+    GenTreeCall* call = tree->AsCall();
+    if ((call->gtCallType == CT_HELPER) && s_helperCallProperties.AlwaysThrow(eeGetHelperNum(call->gtCallMethHnd)))
     {
-        noway_assert(tree->gtFlags & GTF_CALL);
-        noway_assert(tree->gtFlags & GTF_EXCEPT);
+        noway_assert(call->gtFlags & GTF_EXCEPT);
         return true;
     }
-
-    // TODO-CQ: there are a bunch of managed methods in System.ThrowHelper
-    // that would be nice to recognize.
-
     return false;
 }
 

src/coreclr/jit/morph.cpp

@@ -9429,15 +9429,8 @@ GenTree* Compiler::fgMorphCall(GenTreeCall* call)
         // BBJ_THROW would result in the tail call being dropped as the epilog is generated
         // only for BBJ_RETURN blocks.
         //
-        // Currently this doesn't work for non-void callees. Some of the code that handles
-        // fgRemoveRestOfBlock expects the tree to have GTF_EXCEPT flag set but call nodes
-        // do not have this flag by default. We could add the flag here but the proper solution
-        // would be to replace the return expression with a local var node during inlining
-        // so the rest of the call tree stays in a separate statement. That statement can then
-        // be removed by fgRemoveRestOfBlock without needing to add GTF_EXCEPT anywhere.
-        //
 
-        if (!call->IsTailCall() && call->TypeGet() == TYP_VOID)
+        if (!call->IsTailCall())
         {
             fgRemoveRestOfBlock = true;
         }
@@ -14916,7 +14909,6 @@ GenTree* Compiler::fgMorphSmpOp(GenTree* tree, MorphAddrContext* mac)
                 if (fgIsCommaThrow(op1, true))
                 {
                     GenTree* throwNode = op1->AsOp()->gtOp1;
-                    noway_assert(throwNode->gtType == TYP_VOID);
 
                     JITDUMP("Removing [%06d] GT_JTRUE as the block now unconditionally throws an exception.\n",
                             dspTreeID(tree));
@@ -14969,7 +14961,6 @@ GenTree* Compiler::fgMorphSmpOp(GenTree* tree, MorphAddrContext* mac)
             }
 
             GenTree* throwNode = op1->AsOp()->gtOp1;
-            noway_assert(throwNode->gtType == TYP_VOID);
 
             if (oper == GT_COMMA)
             {

src/coreclr/jit/utils.cpp

@@ -1217,6 +1217,7 @@ void HelperCallProperties::init()
         //
         bool isPure        = false; // true if the result only depends upon input args and not any global state
         bool noThrow       = false; // true if the helper will never throw
+        bool alwaysThrow   = false; // true if the helper will always throw
         bool nonNullReturn = false; // true if the result will never be null or zero
         bool isAllocator   = false; // true if the result is usually a newly created heap item, or may throw OutOfMemory
         bool mutatesHeap   = false; // true if any previous heap objects [are|can be] modified
@@ -1458,7 +1459,12 @@ void HelperCallProperties::init()
             case CORINFO_HELP_THROW_NOT_IMPLEMENTED:
             case CORINFO_HELP_THROW_PLATFORM_NOT_SUPPORTED:
             case CORINFO_HELP_THROW_TYPE_NOT_SUPPORTED:
+            case CORINFO_HELP_FAIL_FAST:
+            case CORINFO_HELP_METHOD_ACCESS_EXCEPTION:
+            case CORINFO_HELP_FIELD_ACCESS_EXCEPTION:
+            case CORINFO_HELP_CLASS_ACCESS_EXCEPTION:
 
+                alwaysThrow = true;
                 break;
 
             // These helper calls may throw an exception
@@ -1498,6 +1504,7 @@ void HelperCallProperties::init()
 
         m_isPure[helper]        = isPure;
         m_noThrow[helper]       = noThrow;
+        m_alwaysThrow[helper]   = alwaysThrow;
         m_nonNullReturn[helper] = nonNullReturn;
         m_isAllocator[helper]   = isAllocator;
         m_mutatesHeap[helper]   = mutatesHeap;

src/coreclr/jit/utils.h

@@ -451,6 +451,7 @@ class HelperCallProperties
 private:
     bool m_isPure[CORINFO_HELP_COUNT];
     bool m_noThrow[CORINFO_HELP_COUNT];
+    bool m_alwaysThrow[CORINFO_HELP_COUNT];
     bool m_nonNullReturn[CORINFO_HELP_COUNT];
     bool m_isAllocator[CORINFO_HELP_COUNT];
     bool m_mutatesHeap[CORINFO_HELP_COUNT];
@@ -478,6 +479,13 @@ class HelperCallProperties
         return m_noThrow[helperId];
     }
 
+    bool AlwaysThrow(CorInfoHelpFunc helperId)
+    {
+        assert(helperId > CORINFO_HELP_UNDEF);
+        assert(helperId < CORINFO_HELP_COUNT);
+        return m_alwaysThrow[helperId];
+    }
+
     bool NonNullReturn(CorInfoHelpFunc helperId)
     {
         assert(helperId > CORINFO_HELP_UNDEF);

src/tests/JIT/opt/ThrowHelper/NonVoidThrowHelper.cs

@@ -0,0 +1,180 @@
+// 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.Linq;
+using System.Reflection;
+using System.Runtime.CompilerServices;
+
+public class ProgramException : Exception {}
+
+public sealed class ProgramSubclass : Program
+{
+    public static readonly object s_Obj = new object();
+}
+
+public unsafe class Program
+{
+    private static int s_ReturnCode = 100;
+
+    private Guid field;
+
+    private static Program s_Instance = new ();
+
+    private static Program GetClass() => throw new ProgramException();
+
+    private static Guid GetGuid() => throw new ProgramException();
+
+    private static IntPtr GetIntPtr() => throw new ProgramException();
+
+    private static int* GetPtr() => throw new ProgramException();
+
+    private static Span<byte> GetSpan() => throw new ProgramException();
+
+    private static int GetInt(object obj) => throw new ProgramException();
+
+
+    [MethodImpl(MethodImplOptions.NoInlining)]
+    private static void DoWork() => s_ReturnCode++;
+
+    private static void TestCond0()
+    {
+        if (GetClass() == default)
+            DoWork();
+    }
+
+    private static void TestCond1()
+    {
+        if (GetClass() is ProgramSubclass)
+            DoWork();
+    }
+
+    private static void TestCond2()
+    {
+        if (GetInt(ProgramSubclass.s_Obj) != 42)
+            DoWork();
+    }
+
+    private static void TestCond3()
+    {
+        if (GetClass() == s_Instance)
+            DoWork();
+    }
+
+    private static void TestCond4()
+    {
+        if (GetClass().field == Guid.NewGuid())
+            DoWork();
+    }
+
+    private static void TestCond5()
+    {
+        if (GetGuid() == default)
+            DoWork();
+    }
+
+    private static void TestCond6()
+    {
+        if (GetIntPtr() == (IntPtr)42)
+            DoWork();
+    }
+
+    private static void TestCond7()
+    {
+        if (*GetPtr() == 42)
+            DoWork();
+    }
+
+    private static void TestCond8()
+    {
+        if (GetSpan()[4] == 42)
+            DoWork();
+    }
+
+    private static bool TestRet1()
+    {
+        return GetClass() == default;
+    }
+
+    private static bool TestRet2()
+    {
+        return GetClass() == s_Instance;
+    }
+
+    private static bool TestRet3()
+    {
+        return GetClass() is ProgramSubclass;
+    }
+
+    private static bool TestRet4()
+    {
+        return GetInt(ProgramSubclass.s_Obj) == 42;
+    }
+
+    private static bool TestRet5()
+    {
+        return GetClass().field == Guid.NewGuid();
+    }
+
+    private static bool TestRet6()
+    {
+        return GetGuid() == default;
+    }
+
+    private static bool TestRet7()
+    {
+        return GetIntPtr() == (IntPtr)42;
+    }
+
+    private static bool TestRet8()
+    {
+        return *GetPtr() == 42;
+    }
+
+    private static bool TestRet9()
+    {
+        return GetSpan()[100] == 42;
+    }
+
+    private static Program TestTailCall1()
+    {
+        return GetClass();
+    }
+
+    private static Guid TestTailCall2()
+    {
+        return GetGuid();
+    }
+
+    private static IntPtr TestTailCall3()
+    {
+        return GetIntPtr();
+    }
+
+    private static int* TestTailCall4()
+    {
+        return GetPtr();
+    }
+
+    public static int Main()
+    {
+        foreach (var method in typeof(Program)
+            .GetMethods(BindingFlags.Static | BindingFlags.NonPublic)
+            .Where(m => m.Name.StartsWith("Test")))
+        {
+            try
+            {
+                method.Invoke(null, null);
+            }
+            catch (TargetInvocationException tie)
+            {
+                if (tie.InnerException is ProgramException)
+                {
+                    continue;
+                }
+            }
+
+            s_ReturnCode++;
+        }
+        return s_ReturnCode;
+    }
+}

src/tests/JIT/opt/ThrowHelper/NonVoidThrowHelper.csproj

@@ -0,0 +1,10 @@
+<Project Sdk="Microsoft.NET.Sdk">
+  <PropertyGroup>
+    <OutputType>Exe</OutputType>
+    <Optimize>True</Optimize>
+    <AllowUnsafeBlocks>true</AllowUnsafeBlocks>
+  </PropertyGroup>
+  <ItemGroup>
+    <Compile Include="NonVoidThrowHelper.cs" />
+  </ItemGroup>
+</Project>

src/tests/issues.targets

@@ -2667,6 +2667,9 @@
 
 
     <ItemGroup Condition=" '$(TargetArchitecture)' == 'wasm' " >
+        <ExcludeList Include = "$(XunitTestBinBase)/JIT/opt/ThrowHelper/NonVoidThrowHelper/**">
+            <Issue>https://github.com/dotnet/runtime/issues/48819</Issue>
+        </ExcludeList>
         <ExcludeList Include = "$(XunitTestBinBase)/baseservices/threading/paramthreadstart/ThreadStartBool/**">
             <Issue>https://github.com/dotnet/runtime/issues/41193</Issue>
         </ExcludeList>