diff --git a/src/coreclr/jit/flowgraph.cpp b/src/coreclr/jit/flowgraph.cpp index 8b89c2e5945a55..9fb409929044df 100644 --- a/src/coreclr/jit/flowgraph.cpp +++ b/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; } diff --git a/src/coreclr/jit/morph.cpp b/src/coreclr/jit/morph.cpp index 648ddc49661f23..6b31152a9978a5 100644 --- a/src/coreclr/jit/morph.cpp +++ b/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) { diff --git a/src/coreclr/jit/utils.cpp b/src/coreclr/jit/utils.cpp index c973f6f63c876d..dafa816193cb77 100644 --- a/src/coreclr/jit/utils.cpp +++ b/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; diff --git a/src/coreclr/jit/utils.h b/src/coreclr/jit/utils.h index 2a50680229efd3..3f08f3e25618d7 100644 --- a/src/coreclr/jit/utils.h +++ b/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); diff --git a/src/tests/JIT/opt/ThrowHelper/NonVoidThrowHelper.cs b/src/tests/JIT/opt/ThrowHelper/NonVoidThrowHelper.cs new file mode 100644 index 00000000000000..c6b0e9e527f14b --- /dev/null +++ b/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 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; + } +} diff --git a/src/tests/JIT/opt/ThrowHelper/NonVoidThrowHelper.csproj b/src/tests/JIT/opt/ThrowHelper/NonVoidThrowHelper.csproj new file mode 100644 index 00000000000000..3430940509932f --- /dev/null +++ b/src/tests/JIT/opt/ThrowHelper/NonVoidThrowHelper.csproj @@ -0,0 +1,10 @@ + + + Exe + True + true + + + + + diff --git a/src/tests/issues.targets b/src/tests/issues.targets index 8cbaa5e5cacbd7..004c1fc48070ed 100644 --- a/src/tests/issues.targets +++ b/src/tests/issues.targets @@ -2667,6 +2667,9 @@ + + https://github.com/dotnet/runtime/issues/48819 + https://github.com/dotnet/runtime/issues/41193