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

Delete compQuirkForPPP. #55050

Merged
merged 1 commit into from
Jul 2, 2021
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
3 changes: 0 additions & 3 deletions src/coreclr/jit/codegencommon.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -122,9 +122,6 @@ CodeGen::CodeGen(Compiler* theCompiler) : CodeGenInterface(theCompiler)
#ifdef TARGET_AMD64
// This will be set before final frame layout.
compiler->compVSQuirkStackPaddingNeeded = 0;

// Set to true if we perform the Quirk that fixes the PPP issue
compiler->compQuirkForPPPflag = false;
#endif // TARGET_AMD64

// Initialize the IP-mapping logic.
Expand Down
88 changes: 0 additions & 88 deletions src/coreclr/jit/compiler.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -5099,11 +5099,6 @@ void Compiler::compCompile(void** methodCodePtr, uint32_t* methodCodeSize, JitFl
}
}

#ifdef TARGET_AMD64
// Check if we need to add the Quirk for the PPP backward compat issue
compQuirkForPPPflag = compQuirkForPPP();
#endif

// Insert GC Polls
DoPhase(this, PHASE_INSERT_GC_POLLS, &Compiler::fgInsertGCPolls);

Expand Down Expand Up @@ -5397,89 +5392,6 @@ void Compiler::ProcessShutdownWork(ICorStaticInfo* statInfo)
{
}

#ifdef TARGET_AMD64
// Check if we need to add the Quirk for the PPP backward compat issue.
// This Quirk addresses a compatibility issue between the new RyuJit and the previous JIT64.
// A backward compatibity issue called 'PPP' exists where a PInvoke call passes a 32-byte struct
// into a native API which basically writes 48 bytes of data into the struct.
// With the stack frame layout used by the RyuJIT the extra 16 bytes written corrupts a
// caller saved register and this leads to an A/V in the calling method.
// The older JIT64 jit compiler just happened to have a different stack layout and/or
// caller saved register set so that it didn't hit the A/V in the caller.
// By increasing the amount of stack allocted for the struct by 32 bytes we can fix this.
//
// Return true if we actually perform the Quirk, otherwise return false
//
bool Compiler::compQuirkForPPP()
{
if (lvaCount != 2)
{ // We require that there are exactly two locals
return false;
}

if (compTailCallUsed)
{ // Don't try this quirk if a tail call was used
return false;
}

bool hasOutArgs = false;
LclVarDsc* varDscExposedStruct = nullptr;

unsigned lclNum;
LclVarDsc* varDsc;

/* Look for struct locals that are address taken */
for (lclNum = 0, varDsc = lvaTable; lclNum < lvaCount; lclNum++, varDsc++)
{
if (varDsc->lvIsParam) // It can't be a parameter
{
continue;
}

// We require that the OutgoingArg space lclVar exists
if (lclNum == lvaOutgoingArgSpaceVar)
{
hasOutArgs = true; // Record that we saw it
continue;
}

// Look for a 32-byte address exposed Struct and record its varDsc
if ((varDsc->TypeGet() == TYP_STRUCT) && varDsc->lvAddrExposed && (varDsc->lvExactSize == 32))
{
varDscExposedStruct = varDsc;
}
}

// We only perform the Quirk when there are two locals
// one of them is a address exposed struct of size 32
// and the other is the outgoing arg space local
//
if (hasOutArgs && (varDscExposedStruct != nullptr))
{
#ifdef DEBUG
if (verbose)
{
printf("\nAdding a backwards compatibility quirk for the 'PPP' issue\n");
}
#endif // DEBUG

// Increase the exact size of this struct by 32 bytes
// This fixes the PPP backward compat issue
varDscExposedStruct->lvExactSize += 32;

// The struct is now 64 bytes.
// We're on x64 so this should be 8 pointer slots.
assert((varDscExposedStruct->lvExactSize / TARGET_POINTER_SIZE) == 8);

varDscExposedStruct->SetLayout(
varDscExposedStruct->GetLayout()->GetPPPQuirkLayout(getAllocator(CMK_ClassLayout)));

return true;
}
return false;
}
#endif // TARGET_AMD64

/*****************************************************************************/

#ifdef DEBUG
Expand Down
4 changes: 0 additions & 4 deletions src/coreclr/jit/compiler.h
Original file line number Diff line number Diff line change
Expand Up @@ -9935,7 +9935,6 @@ XXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXX
// Bytes of padding between save-reg area and locals.
#define VSQUIRK_STACK_PAD (2 * REGSIZE_BYTES)
unsigned compVSQuirkStackPaddingNeeded;
bool compQuirkForPPPflag;
#endif

unsigned compArgSize; // total size of arguments in bytes (including register args (lvIsRegArg))
Expand Down Expand Up @@ -10155,9 +10154,6 @@ XXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXX
bool compProfilerMethHndIndirected; // Whether compProfilerHandle is pointer to the handle or is an actual handle
#endif

#ifdef TARGET_AMD64
bool compQuirkForPPP(); // Check if this method should be Quirked for the PPP issue
#endif
public:
// Assumes called as part of process shutdown; does any compiler-specific work associated with that.
static void ProcessShutdownWork(ICorStaticInfo* statInfo);
Expand Down
32 changes: 0 additions & 32 deletions src/coreclr/jit/layout.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -382,38 +382,6 @@ void ClassLayout::InitializeGCPtrs(Compiler* compiler)
INDEBUG(m_gcPtrsInitialized = true;)
}

#ifdef TARGET_AMD64
ClassLayout* ClassLayout::GetPPPQuirkLayout(CompAllocator alloc)
{
assert(m_gcPtrsInitialized);
assert(m_classHandle != NO_CLASS_HANDLE);
assert(m_isValueClass);
assert(m_size == 32);

if (m_pppQuirkLayout == nullptr)
{
m_pppQuirkLayout = new (alloc) ClassLayout(m_classHandle, m_isValueClass, 64 DEBUGARG(m_className));
m_pppQuirkLayout->m_gcPtrCount = m_gcPtrCount;

static_assert_no_msg(_countof(m_gcPtrsArray) == 8);

for (int i = 0; i < 4; i++)
{
m_pppQuirkLayout->m_gcPtrsArray[i] = m_gcPtrsArray[i];
}

for (int i = 4; i < 8; i++)
{
m_pppQuirkLayout->m_gcPtrsArray[i] = TYPE_GC_NONE;
}

INDEBUG(m_pppQuirkLayout->m_gcPtrsInitialized = true;)
}

return m_pppQuirkLayout;
}
#endif // TARGET_AMD64

//------------------------------------------------------------------------
// AreCompatible: check if 2 layouts are the same for copying.
//
Expand Down
17 changes: 0 additions & 17 deletions src/coreclr/jit/layout.h
Original file line number Diff line number Diff line change
Expand Up @@ -35,12 +35,6 @@ class ClassLayout
BYTE m_gcPtrsArray[sizeof(BYTE*)];
};

#ifdef TARGET_AMD64
// A layout that has its size artificially inflated to avoid stack corruption due to
// bugs in user code - see Compiler::compQuirkForPPP for details.
ClassLayout* m_pppQuirkLayout;
#endif

// Class name as reported by ICorJitInfo::getClassName
INDEBUG(const char* m_className;)

Expand All @@ -56,9 +50,6 @@ class ClassLayout
#endif
, m_gcPtrCount(0)
, m_gcPtrs(nullptr)
#ifdef TARGET_AMD64
, m_pppQuirkLayout(nullptr)
#endif
#ifdef DEBUG
, m_className("block")
#endif
Expand All @@ -76,9 +67,6 @@ class ClassLayout
#endif
, m_gcPtrCount(0)
, m_gcPtrs(nullptr)
#ifdef TARGET_AMD64
, m_pppQuirkLayout(nullptr)
#endif
#ifdef DEBUG
, m_className(className)
#endif
Expand All @@ -89,11 +77,6 @@ class ClassLayout
void InitializeGCPtrs(Compiler* compiler);

public:
#ifdef TARGET_AMD64
// Get the layout for the PPP quirk - see Compiler::compQuirkForPPP for details.
ClassLayout* GetPPPQuirkLayout(CompAllocator alloc);
#endif

CORINFO_CLASS_HANDLE GetClassHandle() const
{
return m_classHandle;
Expand Down