From 2c0c10a1034e0a4e1a49a7c94998d8516164bb99 Mon Sep 17 00:00:00 2001 From: Sergey Date: Thu, 1 Jul 2021 17:00:34 -0700 Subject: [PATCH] Delete `compQuirkForPPP`. --- src/coreclr/jit/codegencommon.cpp | 3 -- src/coreclr/jit/compiler.cpp | 88 ------------------------------- src/coreclr/jit/compiler.h | 4 -- src/coreclr/jit/layout.cpp | 32 ----------- src/coreclr/jit/layout.h | 17 ------ 5 files changed, 144 deletions(-) diff --git a/src/coreclr/jit/codegencommon.cpp b/src/coreclr/jit/codegencommon.cpp index cfbdf1e9033b8..78926e64fda65 100644 --- a/src/coreclr/jit/codegencommon.cpp +++ b/src/coreclr/jit/codegencommon.cpp @@ -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. diff --git a/src/coreclr/jit/compiler.cpp b/src/coreclr/jit/compiler.cpp index 6e691f73f77ae..7019d8afad6c5 100644 --- a/src/coreclr/jit/compiler.cpp +++ b/src/coreclr/jit/compiler.cpp @@ -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); @@ -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 diff --git a/src/coreclr/jit/compiler.h b/src/coreclr/jit/compiler.h index ca8daaf7a960b..36acfe6adecde 100644 --- a/src/coreclr/jit/compiler.h +++ b/src/coreclr/jit/compiler.h @@ -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)) @@ -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); diff --git a/src/coreclr/jit/layout.cpp b/src/coreclr/jit/layout.cpp index 3b07a5bada350..cbcb631185d9d 100644 --- a/src/coreclr/jit/layout.cpp +++ b/src/coreclr/jit/layout.cpp @@ -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. // diff --git a/src/coreclr/jit/layout.h b/src/coreclr/jit/layout.h index cef2fe6ded3ab..dcc443cdd44d5 100644 --- a/src/coreclr/jit/layout.h +++ b/src/coreclr/jit/layout.h @@ -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;) @@ -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 @@ -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 @@ -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;