Skip to content

Commit 415e98e

Browse files
erozenfelderozenfeld
committed
Fix TailCallStress mode.
Improve validation of tail calls that are not tail-prefixed in the IL but are marked as such because of TailCallStress. We now do the same correctness validation in morph for such tail calls as we do for implicit tail calls. That blocks tail calls when we have address-taken locals, struct promoted params, and pinned vars. Fixes #39398. Fixes #39309. Fixes #38892. Fixes #38889. Fixes #38887. Fixes #37117. Fixes #8017.
1 parent 4a436e3 commit 415e98e

12 files changed

Lines changed: 38 additions & 46 deletions

File tree

‎eng/pipelines/libraries/run-test-job.yml‎

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -143,10 +143,7 @@ jobs:
143143
- jitstress2
144144
- jitstress2_tiered
145145
- zapdisable
146-
# tailcallstress currently has hundreds of failures on Linux/arm32, so disable it.
147-
# Tracked by https://github.com/dotnet/runtime/issues/38892.
148-
- ${{ if or(eq(parameters.osGroup, 'Windows_NT'), ne(parameters.archType, 'arm')) }}:
149-
- tailcallstress
146+
- tailcallstress
150147
${{ if in(parameters.coreclrTestGroup, 'jitstressregs' ) }}:
151148
scenarios:
152149
- jitstressregs1

‎src/coreclr/src/jit/gentree.h‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4201,6 +4201,7 @@ struct GenTreeCall final : public GenTree
42014201
#define GTF_CALL_M_ALLOC_SIDE_EFFECTS 0x00400000 // GT_CALL -- this is a call to an allocator with side effects
42024202
#define GTF_CALL_M_SUPPRESS_GC_TRANSITION 0x00800000 // GT_CALL -- suppress the GC transition (i.e. during a pinvoke) but a separate GC safe point is required.
42034203
#define GTF_CALL_M_EXP_RUNTIME_LOOKUP 0x01000000 // GT_CALL -- this call needs to be tranformed into CFG for the dynamic dictionary expansion feature.
4204+
#define GTF_CALL_M_STRESS_TAILCALL 0x02000000 // GT_CALL -- the call is NOT "tail" prefixed but GTF_CALL_M_EXPLICIT_TAILCALL was added because of tail call stress mode
42044205

42054206
// clang-format on
42064207

@@ -4315,6 +4316,13 @@ struct GenTreeCall final : public GenTree
43154316
return (gtCallMoreFlags & GTF_CALL_M_EXPLICIT_TAILCALL) != 0;
43164317
}
43174318

4319+
// Returns true if this call didn't have an explicit tail. prefix in the IL
4320+
// but was marked as an explicit tail call because of tail call stress mode.
4321+
bool IsStressTailCall() const
4322+
{
4323+
return (gtCallMoreFlags & GTF_CALL_M_STRESS_TAILCALL) != 0;
4324+
}
4325+
43184326
// This method returning "true" implies that tail call flowgraph morhphing has
43194327
// performed final checks and committed to making a tail call.
43204328
bool IsTailCall() const

‎src/coreclr/src/jit/importer.cpp‎

Lines changed: 15 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -7526,11 +7526,13 @@ enum
75267526
PREFIX_TAILCALL_EXPLICIT = 0x00000001, // call has "tail" IL prefix
75277527
PREFIX_TAILCALL_IMPLICIT =
75287528
0x00000010, // call is treated as having "tail" prefix even though there is no "tail" IL prefix
7529-
PREFIX_TAILCALL = (PREFIX_TAILCALL_EXPLICIT | PREFIX_TAILCALL_IMPLICIT),
7530-
PREFIX_VOLATILE = 0x00000100,
7531-
PREFIX_UNALIGNED = 0x00001000,
7532-
PREFIX_CONSTRAINED = 0x00010000,
7533-
PREFIX_READONLY = 0x00100000
7529+
PREFIX_TAILCALL_STRESS =
7530+
0x00000100, // call doesn't "tail" IL prefix but is treated as explicit because of tail call stress
7531+
PREFIX_TAILCALL = (PREFIX_TAILCALL_EXPLICIT | PREFIX_TAILCALL_IMPLICIT | PREFIX_TAILCALL_STRESS),
7532+
PREFIX_VOLATILE = 0x00001000,
7533+
PREFIX_UNALIGNED = 0x00010000,
7534+
PREFIX_CONSTRAINED = 0x00100000,
7535+
PREFIX_READONLY = 0x01000000
75347536
};
75357537

75367538
/********************************************************************************
@@ -8674,6 +8676,7 @@ var_types Compiler::impImportCall(OPCODE opcode,
86748676
{
86758677
const bool isExplicitTailCall = (tailCallFlags & PREFIX_TAILCALL_EXPLICIT) != 0;
86768678
const bool isImplicitTailCall = (tailCallFlags & PREFIX_TAILCALL_IMPLICIT) != 0;
8679+
const bool isStressTailCall = (tailCallFlags & PREFIX_TAILCALL_STRESS) != 0;
86778680

86788681
// Exactly one of these should be true.
86798682
assert(isExplicitTailCall != isImplicitTailCall);
@@ -8740,6 +8743,12 @@ var_types Compiler::impImportCall(OPCODE opcode,
87408743
// for in-lining.
87418744
call->AsCall()->gtCallMoreFlags |= GTF_CALL_M_EXPLICIT_TAILCALL;
87428745
JITDUMP("\nGTF_CALL_M_EXPLICIT_TAILCALL set for call [%06u]\n", dspTreeID(call));
8746+
8747+
if (isStressTailCall)
8748+
{
8749+
call->AsCall()->gtCallMoreFlags |= GTF_CALL_M_STRESS_TAILCALL;
8750+
JITDUMP("\nGTF_CALL_M_STRESS_TAILCALL set for call [%06u]\n", dspTreeID(call));
8751+
}
87438752
}
87448753
else
87458754
{
@@ -14209,6 +14218,7 @@ void Compiler::impImportBlockCode(BasicBlock* block)
1420914218
// Stress the tailcall.
1421014219
JITDUMP(" (Tailcall stress: prefixFlags |= PREFIX_TAILCALL_EXPLICIT)");
1421114220
prefixFlags |= PREFIX_TAILCALL_EXPLICIT;
14221+
prefixFlags |= PREFIX_TAILCALL_STRESS;
1421214222
}
1421314223
else
1421414224
{

‎src/coreclr/src/jit/morph.cpp‎

Lines changed: 10 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -7235,7 +7235,7 @@ GenTree* Compiler::fgMorphPotentialTailCall(GenTreeCall* call)
72357235
// We still must check for any struct parameters and set 'hasStructParam'
72367236
// so that we won't transform the recursive tail call into a loop.
72377237
//
7238-
if (call->IsImplicitTailCall())
7238+
if (call->IsImplicitTailCall() || call->IsStressTailCall())
72397239
{
72407240
if (varDsc->lvHasLdAddrOp && !lvaIsImplicitByRefLocal(varNum))
72417241
{
@@ -8793,7 +8793,7 @@ GenTree* Compiler::fgMorphCall(GenTreeCall* call)
87938793
assert(!call->CanTailCall());
87948794

87958795
#if FEATURE_MULTIREG_RET
8796-
if (fgGlobalMorph && call->HasMultiRegRetVal())
8796+
if (fgGlobalMorph && call->HasMultiRegRetVal() && varTypeIsStruct(call->TypeGet()))
87978797
{
87988798
// The tail call has been rejected so we must finish the work deferred
87998799
// by impFixupCallStructReturn for multi-reg-returning calls and transform
@@ -8810,23 +8810,14 @@ GenTree* Compiler::fgMorphCall(GenTreeCall* call)
88108810
lvaGrabTemp(false DEBUGARG("Return value temp for multi-reg return (rejected tail call)."));
88118811
lvaTable[tmpNum].lvIsMultiRegRet = true;
88128812

8813-
GenTree* assg = nullptr;
8814-
if (varTypeIsStruct(call->TypeGet()))
8815-
{
8816-
CORINFO_CLASS_HANDLE structHandle = call->gtRetClsHnd;
8817-
assert(structHandle != NO_CLASS_HANDLE);
8818-
const bool unsafeValueClsCheck = false;
8819-
lvaSetStruct(tmpNum, structHandle, unsafeValueClsCheck);
8820-
var_types structType = lvaTable[tmpNum].lvType;
8821-
GenTree* dst = gtNewLclvNode(tmpNum, structType);
8822-
assg = gtNewAssignNode(dst, call);
8823-
}
8824-
else
8825-
{
8826-
assg = gtNewTempAssign(tmpNum, call);
8827-
}
8828-
8829-
assg = fgMorphTree(assg);
8813+
CORINFO_CLASS_HANDLE structHandle = call->gtRetClsHnd;
8814+
assert(structHandle != NO_CLASS_HANDLE);
8815+
const bool unsafeValueClsCheck = false;
8816+
lvaSetStruct(tmpNum, structHandle, unsafeValueClsCheck);
8817+
var_types structType = lvaTable[tmpNum].lvType;
8818+
GenTree* dst = gtNewLclvNode(tmpNum, structType);
8819+
GenTree* assg = gtNewAssignNode(dst, call);
8820+
assg = fgMorphTree(assg);
88308821

88318822
// Create the assignment statement and insert it before the current statement.
88328823
Statement* assgStmt = gtNewStmt(assg, compCurStmt->GetILOffsetX());

‎src/coreclr/tests/issues.targets‎

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -67,9 +67,6 @@
6767
<ExcludeList Include="$(XunitTestBinBase)/GC/Scenarios/muldimjagary/muldimjagary/*">
6868
<Issue>https://github.com/dotnet/runtime/issues/5933</Issue>
6969
</ExcludeList>
70-
<ExcludeList Include="$(XunitTestBinBase)/JIT/Regression/JitBlue/GitHub_11408/GitHub_11408/*">
71-
<Issue>https://github.com/dotnet/runtime/issues/8017</Issue>
72-
</ExcludeList>
7370
<ExcludeList Include="$(XunitTestBinBase)/baseservices/exceptions/StackTracePreserve/StackTracePreserveTests/*">
7471
<Issue>https://github.com/dotnet/runtime/issues/11213</Issue>
7572
</ExcludeList>

‎src/libraries/System.Collections.Concurrent/tests/ProducerConsumerCollectionTests.cs‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -98,7 +98,6 @@ public void Ctor_InitializeFromCollection_ContainsExpectedItems(int numItems)
9898
}
9999

100100
[ConditionalFact(typeof(PlatformDetection), nameof(PlatformDetection.IsThreadingSupported))]
101-
[SkipOnCoreClr("https://github.com/dotnet/runtime/issues/39398", RuntimeTestModes.TailcallStress)]
102101
public void Add_TakeFromAnotherThread_ExpectedItemsTaken()
103102
{
104103
IProducerConsumerCollection<int> c = CreateProducerConsumerCollection();

‎src/libraries/System.Drawing.Common/tests/FontTests.cs‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -784,7 +784,6 @@ public void SizeInPoints_Get_ReturnsExpected(GraphicsUnit unit)
784784
[InlineData(FontStyle.Strikeout | FontStyle.Bold | FontStyle.Italic, 255, true, "@", 700)]
785785
[InlineData(FontStyle.Regular, 0, false, "", 400)]
786786
[InlineData(FontStyle.Regular, 10, false, "", 400)]
787-
[SkipOnCoreClr("https://github.com/dotnet/runtime/issues/38889", RuntimeTestModes.TailcallStress)]
788787
public void ToLogFont_Invoke_ReturnsExpected(FontStyle fontStyle, byte gdiCharSet, bool gdiVerticalFont, string expectedNamePrefix, int expectedWeight)
789788
{
790789
using (FontFamily family = FontFamily.GenericMonospace)
@@ -818,7 +817,6 @@ public void ToLogFont_Invoke_ReturnsExpected(FontStyle fontStyle, byte gdiCharSe
818817
[InlineData(TextRenderingHint.SingleBitPerPixel)]
819818
[InlineData(TextRenderingHint.SingleBitPerPixelGridFit)]
820819
[InlineData(TextRenderingHint.ClearTypeGridFit)]
821-
[SkipOnCoreClr("https://github.com/dotnet/runtime/issues/38889", RuntimeTestModes.TailcallStress)]
822820
public void ToLogFont_InvokeGraphics_ReturnsExpected(TextRenderingHint textRenderingHint)
823821
{
824822
using (FontFamily family = FontFamily.GenericMonospace)

‎src/libraries/System.Net.HttpListener/tests/AssemblyInfo.cs‎

Lines changed: 0 additions & 6 deletions
This file was deleted.

‎src/libraries/System.Net.HttpListener/tests/System.Net.HttpListener.Tests.csproj‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,6 @@
55
<TargetFrameworks>$(NetCoreAppCurrent)-Windows_NT;$(NetCoreAppCurrent)-Unix;$(NetCoreAppCurrent)-Browser;$(NetCoreAppCurrent)-OSX</TargetFrameworks>
66
</PropertyGroup>
77
<ItemGroup>
8-
<Compile Include="AssemblyInfo.cs" />
98
<Compile Include="GetContextHelper.cs" />
109
<Compile Include="HttpListenerFactory.cs" />
1110
<Compile Include="HttpListenerAuthenticationTests.cs" />

‎src/libraries/System.Security.Cryptography.X509Certificates/tests/RevocationTests/AiaTests.cs‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,6 @@ public static void EmptyAiaResponseIsIgnored()
4242
}
4343

4444
[Fact]
45-
[SkipOnCoreClr("https://github.com/dotnet/runtime/issues/38887", RuntimeTestModes.TailcallStress)]
4645
public static void DisableAiaOptionWorks()
4746
{
4847
CertificateAuthority.BuildPrivatePki(

0 commit comments

Comments
 (0)