Skip to content

Commit beb14c1

Browse files
EgorBoEgorBo
authored
JIT: Non-void ThrowHelpers (#48589)
1 parent 3eddf4a commit beb14c1

File tree

7 files changed

+213
-29
lines changed

7 files changed

+213
-29
lines changed

‎src/coreclr/jit/flowgraph.cpp

Lines changed: 4 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -684,31 +684,16 @@ PhaseStatus Compiler::fgImport()
684684

685685
bool Compiler::fgIsThrow(GenTree* tree)
686686
{
687-
if ((tree->gtOper != GT_CALL) || (tree->AsCall()->gtCallType != CT_HELPER))
687+
if (!tree->IsCall())
688688
{
689689
return false;
690690
}
691-
692-
// TODO-Throughput: Replace all these calls to eeFindHelper() with a table based lookup
693-
694-
if ((tree->AsCall()->gtCallMethHnd == eeFindHelper(CORINFO_HELP_OVERFLOW)) ||
695-
(tree->AsCall()->gtCallMethHnd == eeFindHelper(CORINFO_HELP_VERIFICATION)) ||
696-
(tree->AsCall()->gtCallMethHnd == eeFindHelper(CORINFO_HELP_RNGCHKFAIL)) ||
697-
(tree->AsCall()->gtCallMethHnd == eeFindHelper(CORINFO_HELP_THROWDIVZERO)) ||
698-
(tree->AsCall()->gtCallMethHnd == eeFindHelper(CORINFO_HELP_THROWNULLREF)) ||
699-
(tree->AsCall()->gtCallMethHnd == eeFindHelper(CORINFO_HELP_THROW)) ||
700-
(tree->AsCall()->gtCallMethHnd == eeFindHelper(CORINFO_HELP_RETHROW)) ||
701-
(tree->AsCall()->gtCallMethHnd == eeFindHelper(CORINFO_HELP_THROW_TYPE_NOT_SUPPORTED)) ||
702-
(tree->AsCall()->gtCallMethHnd == eeFindHelper(CORINFO_HELP_THROW_PLATFORM_NOT_SUPPORTED)))
691+
GenTreeCall* call = tree->AsCall();
692+
if ((call->gtCallType == CT_HELPER) && s_helperCallProperties.AlwaysThrow(eeGetHelperNum(call->gtCallMethHnd)))
703693
{
704-
noway_assert(tree->gtFlags & GTF_CALL);
705-
noway_assert(tree->gtFlags & GTF_EXCEPT);
694+
noway_assert(call->gtFlags & GTF_EXCEPT);
706695
return true;
707696
}
708-
709-
// TODO-CQ: there are a bunch of managed methods in System.ThrowHelper
710-
// that would be nice to recognize.
711-
712697
return false;
713698
}
714699

‎src/coreclr/jit/morph.cpp

Lines changed: 1 addition & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -9429,15 +9429,8 @@ GenTree* Compiler::fgMorphCall(GenTreeCall* call)
94299429
// BBJ_THROW would result in the tail call being dropped as the epilog is generated
94309430
// only for BBJ_RETURN blocks.
94319431
//
9432-
// Currently this doesn't work for non-void callees. Some of the code that handles
9433-
// fgRemoveRestOfBlock expects the tree to have GTF_EXCEPT flag set but call nodes
9434-
// do not have this flag by default. We could add the flag here but the proper solution
9435-
// would be to replace the return expression with a local var node during inlining
9436-
// so the rest of the call tree stays in a separate statement. That statement can then
9437-
// be removed by fgRemoveRestOfBlock without needing to add GTF_EXCEPT anywhere.
9438-
//
94399432

9440-
if (!call->IsTailCall() && call->TypeGet() == TYP_VOID)
9433+
if (!call->IsTailCall())
94419434
{
94429435
fgRemoveRestOfBlock = true;
94439436
}
@@ -14916,7 +14909,6 @@ GenTree* Compiler::fgMorphSmpOp(GenTree* tree, MorphAddrContext* mac)
1491614909
if (fgIsCommaThrow(op1, true))
1491714910
{
1491814911
GenTree* throwNode = op1->AsOp()->gtOp1;
14919-
noway_assert(throwNode->gtType == TYP_VOID);
1492014912

1492114913
JITDUMP("Removing [%06d] GT_JTRUE as the block now unconditionally throws an exception.\n",
1492214914
dspTreeID(tree));
@@ -14969,7 +14961,6 @@ GenTree* Compiler::fgMorphSmpOp(GenTree* tree, MorphAddrContext* mac)
1496914961
}
1497014962

1497114963
GenTree* throwNode = op1->AsOp()->gtOp1;
14972-
noway_assert(throwNode->gtType == TYP_VOID);
1497314964

1497414965
if (oper == GT_COMMA)
1497514966
{

‎src/coreclr/jit/utils.cpp

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1217,6 +1217,7 @@ void HelperCallProperties::init()
12171217
//
12181218
bool isPure = false; // true if the result only depends upon input args and not any global state
12191219
bool noThrow = false; // true if the helper will never throw
1220+
bool alwaysThrow = false; // true if the helper will always throw
12201221
bool nonNullReturn = false; // true if the result will never be null or zero
12211222
bool isAllocator = false; // true if the result is usually a newly created heap item, or may throw OutOfMemory
12221223
bool mutatesHeap = false; // true if any previous heap objects [are|can be] modified
@@ -1458,7 +1459,12 @@ void HelperCallProperties::init()
14581459
case CORINFO_HELP_THROW_NOT_IMPLEMENTED:
14591460
case CORINFO_HELP_THROW_PLATFORM_NOT_SUPPORTED:
14601461
case CORINFO_HELP_THROW_TYPE_NOT_SUPPORTED:
1462+
case CORINFO_HELP_FAIL_FAST:
1463+
case CORINFO_HELP_METHOD_ACCESS_EXCEPTION:
1464+
case CORINFO_HELP_FIELD_ACCESS_EXCEPTION:
1465+
case CORINFO_HELP_CLASS_ACCESS_EXCEPTION:
14611466

1467+
alwaysThrow = true;
14621468
break;
14631469

14641470
// These helper calls may throw an exception
@@ -1498,6 +1504,7 @@ void HelperCallProperties::init()
14981504

14991505
m_isPure[helper] = isPure;
15001506
m_noThrow[helper] = noThrow;
1507+
m_alwaysThrow[helper] = alwaysThrow;
15011508
m_nonNullReturn[helper] = nonNullReturn;
15021509
m_isAllocator[helper] = isAllocator;
15031510
m_mutatesHeap[helper] = mutatesHeap;

‎src/coreclr/jit/utils.h

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -451,6 +451,7 @@ class HelperCallProperties
451451
private:
452452
bool m_isPure[CORINFO_HELP_COUNT];
453453
bool m_noThrow[CORINFO_HELP_COUNT];
454+
bool m_alwaysThrow[CORINFO_HELP_COUNT];
454455
bool m_nonNullReturn[CORINFO_HELP_COUNT];
455456
bool m_isAllocator[CORINFO_HELP_COUNT];
456457
bool m_mutatesHeap[CORINFO_HELP_COUNT];
@@ -478,6 +479,13 @@ class HelperCallProperties
478479
return m_noThrow[helperId];
479480
}
480481

482+
bool AlwaysThrow(CorInfoHelpFunc helperId)
483+
{
484+
assert(helperId > CORINFO_HELP_UNDEF);
485+
assert(helperId < CORINFO_HELP_COUNT);
486+
return m_alwaysThrow[helperId];
487+
}
488+
481489
bool NonNullReturn(CorInfoHelpFunc helperId)
482490
{
483491
assert(helperId > CORINFO_HELP_UNDEF);

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

Lines changed: 180 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,180 @@
1+
// Licensed to the .NET Foundation under one or more agreements.
2+
// The .NET Foundation licenses this file to you under the MIT license.
3+
using System;
4+
using System.Linq;
5+
using System.Reflection;
6+
using System.Runtime.CompilerServices;
7+
8+
public class ProgramException : Exception {}
9+
10+
public sealed class ProgramSubclass : Program
11+
{
12+
public static readonly object s_Obj = new object();
13+
}
14+
15+
public unsafe class Program
16+
{
17+
private static int s_ReturnCode = 100;
18+
19+
private Guid field;
20+
21+
private static Program s_Instance = new ();
22+
23+
private static Program GetClass() => throw new ProgramException();
24+
25+
private static Guid GetGuid() => throw new ProgramException();
26+
27+
private static IntPtr GetIntPtr() => throw new ProgramException();
28+
29+
private static int* GetPtr() => throw new ProgramException();
30+
31+
private static Span<byte> GetSpan() => throw new ProgramException();
32+
33+
private static int GetInt(object obj) => throw new ProgramException();
34+
35+
36+
[MethodImpl(MethodImplOptions.NoInlining)]
37+
private static void DoWork() => s_ReturnCode++;
38+
39+
private static void TestCond0()
40+
{
41+
if (GetClass() == default)
42+
DoWork();
43+
}
44+
45+
private static void TestCond1()
46+
{
47+
if (GetClass() is ProgramSubclass)
48+
DoWork();
49+
}
50+
51+
private static void TestCond2()
52+
{
53+
if (GetInt(ProgramSubclass.s_Obj) != 42)
54+
DoWork();
55+
}
56+
57+
private static void TestCond3()
58+
{
59+
if (GetClass() == s_Instance)
60+
DoWork();
61+
}
62+
63+
private static void TestCond4()
64+
{
65+
if (GetClass().field == Guid.NewGuid())
66+
DoWork();
67+
}
68+
69+
private static void TestCond5()
70+
{
71+
if (GetGuid() == default)
72+
DoWork();
73+
}
74+
75+
private static void TestCond6()
76+
{
77+
if (GetIntPtr() == (IntPtr)42)
78+
DoWork();
79+
}
80+
81+
private static void TestCond7()
82+
{
83+
if (*GetPtr() == 42)
84+
DoWork();
85+
}
86+
87+
private static void TestCond8()
88+
{
89+
if (GetSpan()[4] == 42)
90+
DoWork();
91+
}
92+
93+
private static bool TestRet1()
94+
{
95+
return GetClass() == default;
96+
}
97+
98+
private static bool TestRet2()
99+
{
100+
return GetClass() == s_Instance;
101+
}
102+
103+
private static bool TestRet3()
104+
{
105+
return GetClass() is ProgramSubclass;
106+
}
107+
108+
private static bool TestRet4()
109+
{
110+
return GetInt(ProgramSubclass.s_Obj) == 42;
111+
}
112+
113+
private static bool TestRet5()
114+
{
115+
return GetClass().field == Guid.NewGuid();
116+
}
117+
118+
private static bool TestRet6()
119+
{
120+
return GetGuid() == default;
121+
}
122+
123+
private static bool TestRet7()
124+
{
125+
return GetIntPtr() == (IntPtr)42;
126+
}
127+
128+
private static bool TestRet8()
129+
{
130+
return *GetPtr() == 42;
131+
}
132+
133+
private static bool TestRet9()
134+
{
135+
return GetSpan()[100] == 42;
136+
}
137+
138+
private static Program TestTailCall1()
139+
{
140+
return GetClass();
141+
}
142+
143+
private static Guid TestTailCall2()
144+
{
145+
return GetGuid();
146+
}
147+
148+
private static IntPtr TestTailCall3()
149+
{
150+
return GetIntPtr();
151+
}
152+
153+
private static int* TestTailCall4()
154+
{
155+
return GetPtr();
156+
}
157+
158+
public static int Main()
159+
{
160+
foreach (var method in typeof(Program)
161+
.GetMethods(BindingFlags.Static | BindingFlags.NonPublic)
162+
.Where(m => m.Name.StartsWith("Test")))
163+
{
164+
try
165+
{
166+
method.Invoke(null, null);
167+
}
168+
catch (TargetInvocationException tie)
169+
{
170+
if (tie.InnerException is ProgramException)
171+
{
172+
continue;
173+
}
174+
}
175+
176+
s_ReturnCode++;
177+
}
178+
return s_ReturnCode;
179+
}
180+
}

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

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
<Project Sdk="Microsoft.NET.Sdk">
2+
<PropertyGroup>
3+
<OutputType>Exe</OutputType>
4+
<Optimize>True</Optimize>
5+
<AllowUnsafeBlocks>true</AllowUnsafeBlocks>
6+
</PropertyGroup>
7+
<ItemGroup>
8+
<Compile Include="NonVoidThrowHelper.cs" />
9+
</ItemGroup>
10+
</Project>

‎src/tests/issues.targets

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2667,6 +2667,9 @@
26672667

26682668

26692669
<ItemGroup Condition=" '$(TargetArchitecture)' == 'wasm' " >
2670+
<ExcludeList Include = "$(XunitTestBinBase)/JIT/opt/ThrowHelper/NonVoidThrowHelper/**">
2671+
<Issue>https://github.com/dotnet/runtime/issues/48819</Issue>
2672+
</ExcludeList>
26702673
<ExcludeList Include = "$(XunitTestBinBase)/baseservices/threading/paramthreadstart/ThreadStartBool/**">
26712674
<Issue>https://github.com/dotnet/runtime/issues/41193</Issue>
26722675
</ExcludeList>

0 commit comments

Comments
 (0)