Skip to content

Commit

Permalink
Delete compQuirkForPPP. (#55050)
Browse files Browse the repository at this point in the history
  • Loading branch information
Sergey Andreenko authored Jul 2, 2021
1 parent 66cf2ab commit e263adc
Show file tree
Hide file tree
Showing 5 changed files with 0 additions and 144 deletions.
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

0 comments on commit e263adc

Please sign in to comment.