Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 1 addition & 4 deletions eng/pipelines/libraries/run-test-job.yml
Original file line number Diff line number Diff line change
Expand Up @@ -143,10 +143,7 @@ jobs:
- jitstress2
- jitstress2_tiered
- zapdisable
# tailcallstress currently has hundreds of failures on Linux/arm32, so disable it.
# Tracked by https://github.com/dotnet/runtime/issues/38892.
- ${{ if or(eq(parameters.osGroup, 'Windows_NT'), ne(parameters.archType, 'arm')) }}:
- tailcallstress
- tailcallstress
${{ if in(parameters.coreclrTestGroup, 'jitstressregs' ) }}:
scenarios:
- jitstressregs1
Expand Down
8 changes: 8 additions & 0 deletions src/coreclr/src/jit/gentree.h
Original file line number Diff line number Diff line change
Expand Up @@ -4201,6 +4201,7 @@ struct GenTreeCall final : public GenTree
#define GTF_CALL_M_ALLOC_SIDE_EFFECTS 0x00400000 // GT_CALL -- this is a call to an allocator with side effects
#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.
#define GTF_CALL_M_EXP_RUNTIME_LOOKUP 0x01000000 // GT_CALL -- this call needs to be tranformed into CFG for the dynamic dictionary expansion feature.
#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

// clang-format on

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

// Returns true if this call didn't have an explicit tail. prefix in the IL
// but was marked as an explicit tail call because of tail call stress mode.
bool IsStressTailCall() const
{
return (gtCallMoreFlags & GTF_CALL_M_STRESS_TAILCALL) != 0;
}

// This method returning "true" implies that tail call flowgraph morhphing has
// performed final checks and committed to making a tail call.
bool IsTailCall() const
Expand Down
20 changes: 15 additions & 5 deletions src/coreclr/src/jit/importer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -7526,11 +7526,13 @@ enum
PREFIX_TAILCALL_EXPLICIT = 0x00000001, // call has "tail" IL prefix
PREFIX_TAILCALL_IMPLICIT =
0x00000010, // call is treated as having "tail" prefix even though there is no "tail" IL prefix
PREFIX_TAILCALL = (PREFIX_TAILCALL_EXPLICIT | PREFIX_TAILCALL_IMPLICIT),
PREFIX_VOLATILE = 0x00000100,
PREFIX_UNALIGNED = 0x00001000,
PREFIX_CONSTRAINED = 0x00010000,
PREFIX_READONLY = 0x00100000
PREFIX_TAILCALL_STRESS =
0x00000100, // call doesn't "tail" IL prefix but is treated as explicit because of tail call stress
PREFIX_TAILCALL = (PREFIX_TAILCALL_EXPLICIT | PREFIX_TAILCALL_IMPLICIT | PREFIX_TAILCALL_STRESS),
PREFIX_VOLATILE = 0x00001000,
PREFIX_UNALIGNED = 0x00010000,
PREFIX_CONSTRAINED = 0x00100000,
PREFIX_READONLY = 0x01000000
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why do these flags only use every forth bit?
i.e. 0x1000, 0x2000, 0x4000, 0x8000

Copy link
Member Author

@erozenfeld erozenfeld Aug 14, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't see any reason for that. I was just following the existing pattern.

};

/********************************************************************************
Expand Down Expand Up @@ -8674,6 +8676,7 @@ var_types Compiler::impImportCall(OPCODE opcode,
{
const bool isExplicitTailCall = (tailCallFlags & PREFIX_TAILCALL_EXPLICIT) != 0;
const bool isImplicitTailCall = (tailCallFlags & PREFIX_TAILCALL_IMPLICIT) != 0;
const bool isStressTailCall = (tailCallFlags & PREFIX_TAILCALL_STRESS) != 0;

// Exactly one of these should be true.
assert(isExplicitTailCall != isImplicitTailCall);
Expand Down Expand Up @@ -8740,6 +8743,12 @@ var_types Compiler::impImportCall(OPCODE opcode,
// for in-lining.
call->AsCall()->gtCallMoreFlags |= GTF_CALL_M_EXPLICIT_TAILCALL;
JITDUMP("\nGTF_CALL_M_EXPLICIT_TAILCALL set for call [%06u]\n", dspTreeID(call));

if (isStressTailCall)
{
call->AsCall()->gtCallMoreFlags |= GTF_CALL_M_STRESS_TAILCALL;
JITDUMP("\nGTF_CALL_M_STRESS_TAILCALL set for call [%06u]\n", dspTreeID(call));
}
}
else
{
Expand Down Expand Up @@ -14209,6 +14218,7 @@ void Compiler::impImportBlockCode(BasicBlock* block)
// Stress the tailcall.
JITDUMP(" (Tailcall stress: prefixFlags |= PREFIX_TAILCALL_EXPLICIT)");
prefixFlags |= PREFIX_TAILCALL_EXPLICIT;
prefixFlags |= PREFIX_TAILCALL_STRESS;
}
else
{
Expand Down
29 changes: 10 additions & 19 deletions src/coreclr/src/jit/morph.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -7235,7 +7235,7 @@ GenTree* Compiler::fgMorphPotentialTailCall(GenTreeCall* call)
// We still must check for any struct parameters and set 'hasStructParam'
// so that we won't transform the recursive tail call into a loop.
//
if (call->IsImplicitTailCall())
if (call->IsImplicitTailCall() || call->IsStressTailCall())
{
if (varDsc->lvHasLdAddrOp && !lvaIsImplicitByRefLocal(varNum))
{
Expand Down Expand Up @@ -8793,7 +8793,7 @@ GenTree* Compiler::fgMorphCall(GenTreeCall* call)
assert(!call->CanTailCall());

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

GenTree* assg = nullptr;
if (varTypeIsStruct(call->TypeGet()))
{
CORINFO_CLASS_HANDLE structHandle = call->gtRetClsHnd;
assert(structHandle != NO_CLASS_HANDLE);
const bool unsafeValueClsCheck = false;
lvaSetStruct(tmpNum, structHandle, unsafeValueClsCheck);
var_types structType = lvaTable[tmpNum].lvType;
GenTree* dst = gtNewLclvNode(tmpNum, structType);
assg = gtNewAssignNode(dst, call);
}
else
{
assg = gtNewTempAssign(tmpNum, call);
}

assg = fgMorphTree(assg);
CORINFO_CLASS_HANDLE structHandle = call->gtRetClsHnd;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@erozenfeld Can you please expain what this part does?

Copy link
Contributor

@sandreenko sandreenko Aug 13, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It was marking long on x86 as lvIsMultiRegRet that was not expected by decomposeLong (firing an assert).
Right now decomposeLong has its own mechanism to deal with such lclVars.

A long term solution could be to mark long lclVar as lvIsMultiRegRet on x86 and delete the logic from decomposeLong but for now it is not necessary.

assert(structHandle != NO_CLASS_HANDLE);
const bool unsafeValueClsCheck = false;
lvaSetStruct(tmpNum, structHandle, unsafeValueClsCheck);
var_types structType = lvaTable[tmpNum].lvType;
GenTree* dst = gtNewLclvNode(tmpNum, structType);
GenTree* assg = gtNewAssignNode(dst, call);
assg = fgMorphTree(assg);

// Create the assignment statement and insert it before the current statement.
Statement* assgStmt = gtNewStmt(assg, compCurStmt->GetILOffsetX());
Expand Down
3 changes: 0 additions & 3 deletions src/coreclr/tests/issues.targets
Original file line number Diff line number Diff line change
Expand Up @@ -67,9 +67,6 @@
<ExcludeList Include="$(XunitTestBinBase)/GC/Scenarios/muldimjagary/muldimjagary/*">
<Issue>https://github.com/dotnet/runtime/issues/5933</Issue>
</ExcludeList>
<ExcludeList Include="$(XunitTestBinBase)/JIT/Regression/JitBlue/GitHub_11408/GitHub_11408/*">
<Issue>https://github.com/dotnet/runtime/issues/8017</Issue>
</ExcludeList>
<ExcludeList Include="$(XunitTestBinBase)/baseservices/exceptions/StackTracePreserve/StackTracePreserveTests/*">
<Issue>https://github.com/dotnet/runtime/issues/11213</Issue>
</ExcludeList>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,6 @@ public void Ctor_InitializeFromCollection_ContainsExpectedItems(int numItems)
}

[ConditionalFact(typeof(PlatformDetection), nameof(PlatformDetection.IsThreadingSupported))]
[SkipOnCoreClr("https://github.com/dotnet/runtime/issues/39398", RuntimeTestModes.TailcallStress)]
public void Add_TakeFromAnotherThread_ExpectedItemsTaken()
{
IProducerConsumerCollection<int> c = CreateProducerConsumerCollection();
Expand Down
2 changes: 0 additions & 2 deletions src/libraries/System.Drawing.Common/tests/FontTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -784,7 +784,6 @@ public void SizeInPoints_Get_ReturnsExpected(GraphicsUnit unit)
[InlineData(FontStyle.Strikeout | FontStyle.Bold | FontStyle.Italic, 255, true, "@", 700)]
[InlineData(FontStyle.Regular, 0, false, "", 400)]
[InlineData(FontStyle.Regular, 10, false, "", 400)]
[SkipOnCoreClr("https://github.com/dotnet/runtime/issues/38889", RuntimeTestModes.TailcallStress)]
public void ToLogFont_Invoke_ReturnsExpected(FontStyle fontStyle, byte gdiCharSet, bool gdiVerticalFont, string expectedNamePrefix, int expectedWeight)
{
using (FontFamily family = FontFamily.GenericMonospace)
Expand Down Expand Up @@ -818,7 +817,6 @@ public void ToLogFont_Invoke_ReturnsExpected(FontStyle fontStyle, byte gdiCharSe
[InlineData(TextRenderingHint.SingleBitPerPixel)]
[InlineData(TextRenderingHint.SingleBitPerPixelGridFit)]
[InlineData(TextRenderingHint.ClearTypeGridFit)]
[SkipOnCoreClr("https://github.com/dotnet/runtime/issues/38889", RuntimeTestModes.TailcallStress)]
public void ToLogFont_InvokeGraphics_ReturnsExpected(TextRenderingHint textRenderingHint)
{
using (FontFamily family = FontFamily.GenericMonospace)
Expand Down
6 changes: 0 additions & 6 deletions src/libraries/System.Net.HttpListener/tests/AssemblyInfo.cs

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@
<TargetFrameworks>$(NetCoreAppCurrent)-Windows_NT;$(NetCoreAppCurrent)-Unix;$(NetCoreAppCurrent)-Browser;$(NetCoreAppCurrent)-OSX</TargetFrameworks>
</PropertyGroup>
<ItemGroup>
<Compile Include="AssemblyInfo.cs" />
<Compile Include="GetContextHelper.cs" />
<Compile Include="HttpListenerFactory.cs" />
<Compile Include="HttpListenerAuthenticationTests.cs" />
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,6 @@ public static void EmptyAiaResponseIsIgnored()
}

[Fact]
[SkipOnCoreClr("https://github.com/dotnet/runtime/issues/38887", RuntimeTestModes.TailcallStress)]
public static void DisableAiaOptionWorks()
{
CertificateAuthority.BuildPrivatePki(
Expand Down
4 changes: 4 additions & 0 deletions src/tests/JIT/opt/OSR/tailrecursetry.csproj
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,10 @@
<OutputType>Exe</OutputType>
<DebugType />
<Optimize>True</Optimize>
<!-- This test currently fails for TailcallStress. Disable for JIT stress modes until this is fixed.
Issue: https://github.com/dotnet/runtime/issues/35687
-->
<JitOptimizationSensitive>true</JitOptimizationSensitive>
</PropertyGroup>
<ItemGroup>
<Compile Include="$(MSBuildProjectName).cs" />
Expand Down
4 changes: 0 additions & 4 deletions src/tests/profiler/unittest/getappdomainstaticaddress.csproj
Original file line number Diff line number Diff line change
Expand Up @@ -6,10 +6,6 @@
<AllowUnsafeBlocks>true</AllowUnsafeBlocks>
<CLRTestPriority>0</CLRTestPriority>
<Optimize>true</Optimize>
<!-- This test currently fails for TailcallStress. Disable for JIT stress modes until this is fixed.
Issue: https://github.com/dotnet/runtime/issues/37117
-->
<JitOptimizationSensitive>true</JitOptimizationSensitive>
<!-- This test provides no interesting scenarios for GCStress -->
<GCStressIncompatible>true</GCStressIncompatible>
</PropertyGroup>
Expand Down