Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Do not run stack level setter in release except for x86. #42197

Closed
wants to merge 3 commits into from
Closed
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
12 changes: 6 additions & 6 deletions src/coreclr/src/jit/codegencommon.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2355,25 +2355,25 @@ void CodeGen::genEmitMachineCode()
}
#endif

#if EMIT_TRACK_STACK_DEPTH
#if EMIT_TRACK_STACK_DEPTH && defined(DEBUG)
// Check our max stack level. Needed for fgAddCodeRef().
// We need to relax the assert as our estimation won't include code-gen
// stack changes (which we know don't affect fgAddCodeRef()).
// NOTE: after emitEndCodeGen (including here), emitMaxStackDepth is a
// count of DWORD-sized arguments, NOT argument size in bytes.
{
unsigned maxAllowedStackDepth = compiler->fgPtrArgCntMax + // Max number of pointer-sized stack arguments.
compiler->compHndBBtabCount + // Return address for locally-called finallys
genTypeStSz(TYP_LONG) + // longs/doubles may be transferred via stack, etc
unsigned maxAllowedStackDepth = compiler->fgGetPtrArgCntMax() + // Max number of pointer-sized stack arguments.
compiler->compHndBBtabCount + // Return address for locally-called finallys
genTypeStSz(TYP_LONG) + // longs/doubles may be transferred via stack, etc
(compiler->compTailCallUsed ? 4 : 0); // CORINFO_HELP_TAILCALL args
#if defined(UNIX_X86_ABI)
// Convert maxNestedAlignment to DWORD count before adding to maxAllowedStackDepth.
assert(maxNestedAlignment % sizeof(int) == 0);
maxAllowedStackDepth += maxNestedAlignment / sizeof(int);
#endif
noway_assert(GetEmitter()->emitMaxStackDepth <= maxAllowedStackDepth);
assert(GetEmitter()->emitMaxStackDepth <= maxAllowedStackDepth);
}
#endif // EMIT_TRACK_STACK_DEPTH
#endif // EMIT_TRACK_STACK_DEPTH && DEBUG

*nativeSizeOfCode = codeSize;
compiler->info.compNativeCodeSize = (UNATIVE_OFFSET)codeSize;
Expand Down
4 changes: 2 additions & 2 deletions src/coreclr/src/jit/codegenxarch.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -8514,7 +8514,7 @@ void CodeGen::genProfilingEnterCallback(regNumber initReg, bool* pInitRegZeroed)
EA_UNKNOWN); // retSize

// Check that we have place for the push.
assert(compiler->fgPtrArgCntMax >= 1);
assert(compiler->fgGetPtrArgCntMax() >= 1);

#if defined(UNIX_X86_ABI)
// Restoring alignment manually. This is similar to CodeGen::genRemoveAlignmentAfterCall
Expand Down Expand Up @@ -8595,7 +8595,7 @@ void CodeGen::genProfilingLeaveCallback(unsigned helper)
genEmitHelperCall(helper, argSize, EA_UNKNOWN /* retSize */);

// Check that we have place for the push.
assert(compiler->fgPtrArgCntMax >= 1);
assert(compiler->fgGetPtrArgCntMax() >= 1);

#if defined(UNIX_X86_ABI)
// Restoring alignment manually. This is similar to CodeGen::genRemoveAlignmentAfterCall
Expand Down
6 changes: 4 additions & 2 deletions src/coreclr/src/jit/compiler.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -4917,10 +4917,12 @@ void Compiler::compCompile(void** methodCodePtr, ULONG* methodCodeSize, JitFlags
m_pLowering = new (this, CMK_LSRA) Lowering(this, m_pLinearScan); // PHASE_LOWERING
m_pLowering->Run();

// Set stack levels
//
#if defined(TARGET_X86) || defined(DEBUG)
// Set stack levels, this informmation is necessary for x86
// but on other platforms it is used only in asserts.
StackLevelSetter stackLevelSetter(this);
stackLevelSetter.Run();
#endif // X86 || debug

// We can not add any new tracked variables after this point.
lvaTrackedFixed = true;
Expand Down
4 changes: 4 additions & 0 deletions src/coreclr/src/jit/compiler.h
Original file line number Diff line number Diff line change
Expand Up @@ -5451,6 +5451,9 @@ class Compiler

//------------------------- Morphing --------------------------------------


#if defined(TARGET_X86) || defined(DEBUG)
private:
unsigned fgPtrArgCntMax;

public:
Expand All @@ -5474,6 +5477,7 @@ class Compiler
{
fgPtrArgCntMax = argCntMax;
}
#endif // X86 || DEBUG

bool compCanEncodePtrArgCntMax();

Expand Down
2 changes: 2 additions & 0 deletions src/coreclr/src/jit/flowgraph.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -116,8 +116,10 @@ void Compiler::fgInit()
fgExcptnTargetCache[i] = nullptr;
}

#if defined(TARGET_X86) || defined(DEBUG)
/* Keep track of the max count of pointer arguments */
fgPtrArgCntMax = 0;
#endif

/* This global flag is set whenever we remove a statement */
fgStmtRemoved = false;
Expand Down
13 changes: 12 additions & 1 deletion src/coreclr/src/jit/stacklevelsetter.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@
#pragma hdrstop
#endif

#if defined(TARGET_X86) || defined(DEBUG)

#include "stacklevelsetter.h"

StackLevelSetter::StackLevelSetter(Compiler* compiler)
Expand Down Expand Up @@ -304,7 +306,7 @@ void StackLevelSetter::SubStackLevel(unsigned value)
//
// Notes:
// CheckArgCnt records the maximum number of pushed arguments.
// Depending upon this value of the maximum number of pushed arguments
// On x86 depending upon this value of the maximum number of pushed arguments
// we may need to use an EBP frame or be partially interuptible.
// This functionality has to be called after maxStackLevel is set.
//
Expand All @@ -314,6 +316,7 @@ void StackLevelSetter::SubStackLevel(unsigned value)
//
void StackLevelSetter::CheckArgCnt()
{
#if defined(TARGET_X86)
if (!comp->compCanEncodePtrArgCntMax())
{
#ifdef DEBUG
Expand All @@ -325,6 +328,11 @@ void StackLevelSetter::CheckArgCnt()
#endif
comp->SetInterruptible(false);
}
#else
assert(comp->compCanEncodePtrArgCntMax());
#endif

#if defined(TARGET_X86)
if (maxStackLevel >= sizeof(unsigned))
Copy link
Contributor Author

Choose a reason for hiding this comment

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

this is the actual change, as you can see from the comment it is applicable only to x86 where we push arguments (and use EBP) but it was forgotten to be guarded as such before CoreCLR initial PR.

{
#ifdef DEBUG
Expand All @@ -335,6 +343,7 @@ void StackLevelSetter::CheckArgCnt()
#endif
comp->codeGen->setFramePointerRequired(true);
}
#endif
}

//------------------------------------------------------------------------
Expand All @@ -356,3 +365,5 @@ void StackLevelSetter::CheckAdditionalArgs()
}
#endif // TARGET_X86
}

#endif // X86 || DEBUG
4 changes: 4 additions & 0 deletions src/coreclr/src/jit/stacklevelsetter.h
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@

#pragma once

#if defined(TARGET_X86) || defined(DEBUG)

#include "compiler.h"
#include "phase.h"

Expand Down Expand Up @@ -42,3 +44,5 @@ class StackLevelSetter final : public Phase
bool throwHelperBlocksUsed; // Were any throw helper blocks created for this method.
#endif // !FEATURE_FIXED_OUT_ARGS
};

#endif // X86 || DEBUG