diff --git a/src/coreclr/gcdump/i386/gcdumpx86.cpp b/src/coreclr/gcdump/i386/gcdumpx86.cpp index 1bf3e61011bd3..c5d359aac517e 100644 --- a/src/coreclr/gcdump/i386/gcdumpx86.cpp +++ b/src/coreclr/gcdump/i386/gcdumpx86.cpp @@ -154,7 +154,8 @@ size_t GCDump::DumpInfoHdr (PTR_CBYTE gcInfoBlock, gcPrintf(" GuardStack cookie = [%s%u]\n", header->ebpFrame ? "EBP-" : "ESP+", header->gsCookieOffset); if (header->syncStartOffset != INVALID_SYNC_OFFSET) - gcPrintf(" Sync region = [%u,%u]\n", + gcPrintf(" Sync region = [%u,%u] ([0x%x,0x%x])\n", + header->syncStartOffset, header->syncEndOffset, header->syncStartOffset, header->syncEndOffset); if (header->epilogCount > 1 || (header->epilogCount != 0 && diff --git a/src/coreclr/jit/codegencommon.cpp b/src/coreclr/jit/codegencommon.cpp index db035461a3136..b12b584038c53 100644 --- a/src/coreclr/jit/codegencommon.cpp +++ b/src/coreclr/jit/codegencommon.cpp @@ -7499,6 +7499,7 @@ void CodeGen::genLongReturn(GenTree* treeNode) //------------------------------------------------------------------------ // genReturn: Generates code for return statement. // In case of struct return, delegates to the genStructReturn method. +// In case of LONG return on 32-bit, delegates to the genLongReturn method. // // Arguments: // treeNode - The GT_RETURN or GT_RETFILT tree node. @@ -7508,7 +7509,8 @@ void CodeGen::genLongReturn(GenTree* treeNode) // void CodeGen::genReturn(GenTree* treeNode) { - assert(treeNode->OperGet() == GT_RETURN || treeNode->OperGet() == GT_RETFILT); + assert(treeNode->OperIs(GT_RETURN, GT_RETFILT)); + GenTree* op1 = treeNode->gtGetOp1(); var_types targetType = treeNode->TypeGet(); @@ -7609,9 +7611,9 @@ void CodeGen::genReturn(GenTree* treeNode) // maintain such an invariant irrespective of whether profiler hook needed or not. // Also, there is not much to be gained by materializing it as an explicit node. // - // There should be a single return block while generating profiler ELT callbacks, - // so we just look for that block to trigger insertion of the profile hook. - if ((compiler->compCurBB == compiler->genReturnBB) && compiler->compIsProfilerHookNeeded()) + // There should be a single GT_RETURN while generating profiler ELT callbacks. + // + if (treeNode->OperIs(GT_RETURN) && compiler->compIsProfilerHookNeeded()) { // !! NOTE !! // Since we are invalidating the assumption that we would slip into the epilog diff --git a/src/coreclr/jit/codegenlinear.cpp b/src/coreclr/jit/codegenlinear.cpp index 81c84a10f45a2..67f4bb73c0eb1 100644 --- a/src/coreclr/jit/codegenlinear.cpp +++ b/src/coreclr/jit/codegenlinear.cpp @@ -835,6 +835,25 @@ void CodeGen::genCodeForBBlist() #endif // DEBUG } //------------------ END-FOR each block of the method ------------------- +#if !defined(FEATURE_EH_FUNCLETS) + // If this is a synchronized method on x86, and we generated all the code without + // generating the "exit monitor" call, then we must have deleted the single return block + // with that call because it was dead code. We still need to report the monitor range + // to the VM in the GC info, so create a label at the very end so we have a marker for + // the monitor end range. + // + // Do this before cleaning the GC refs below; we don't want to create an IG that clears + // the `this` pointer for lvaKeepAliveAndReportThis. + + if ((compiler->info.compFlags & CORINFO_FLG_SYNCH) && (compiler->syncEndEmitCookie == nullptr)) + { + JITDUMP("Synchronized method with missing exit monitor call; adding final label\n"); + compiler->syncEndEmitCookie = + GetEmitter()->emitAddLabel(gcInfo.gcVarPtrSetCur, gcInfo.gcRegGCrefSetCur, gcInfo.gcRegByrefSetCur); + noway_assert(compiler->syncEndEmitCookie != nullptr); + } +#endif // !FEATURE_EH_FUNCLETS + // There could be variables alive at this point. For example see lvaKeepAliveAndReportThis. // This call is for cleaning the GC refs genUpdateLife(VarSetOps::MakeEmpty(compiler)); diff --git a/src/coreclr/jit/codegenxarch.cpp b/src/coreclr/jit/codegenxarch.cpp index e44c4247fe552..43c006291dd3e 100644 --- a/src/coreclr/jit/codegenxarch.cpp +++ b/src/coreclr/jit/codegenxarch.cpp @@ -6176,17 +6176,17 @@ void CodeGen::genCall(GenTreeCall* call) { case CORINFO_HELP_MON_ENTER: case CORINFO_HELP_MON_ENTER_STATIC: - noway_assert(compiler->syncStartEmitCookie == NULL); + noway_assert(compiler->syncStartEmitCookie == nullptr); compiler->syncStartEmitCookie = GetEmitter()->emitAddLabel(gcInfo.gcVarPtrSetCur, gcInfo.gcRegGCrefSetCur, gcInfo.gcRegByrefSetCur); - noway_assert(compiler->syncStartEmitCookie != NULL); + noway_assert(compiler->syncStartEmitCookie != nullptr); break; case CORINFO_HELP_MON_EXIT: case CORINFO_HELP_MON_EXIT_STATIC: - noway_assert(compiler->syncEndEmitCookie == NULL); + noway_assert(compiler->syncEndEmitCookie == nullptr); compiler->syncEndEmitCookie = GetEmitter()->emitAddLabel(gcInfo.gcVarPtrSetCur, gcInfo.gcRegGCrefSetCur, gcInfo.gcRegByrefSetCur); - noway_assert(compiler->syncEndEmitCookie != NULL); + noway_assert(compiler->syncEndEmitCookie != nullptr); break; default: break; diff --git a/src/coreclr/jit/compiler.cpp b/src/coreclr/jit/compiler.cpp index 3efa0e844b014..9e88c6ab09130 100644 --- a/src/coreclr/jit/compiler.cpp +++ b/src/coreclr/jit/compiler.cpp @@ -3332,6 +3332,12 @@ void Compiler::compInitOptions(JitFlags* jitFlags) printf("OPTIONS: compProcedureSplitting = %s\n", dspBool(opts.compProcedureSplitting)); printf("OPTIONS: compProcedureSplittingEH = %s\n", dspBool(opts.compProcedureSplittingEH)); + // This is rare; don't clutter up the dump with it normally. + if (compProfilerHookNeeded) + { + printf("OPTIONS: compProfilerHookNeeded = %s\n", dspBool(compProfilerHookNeeded)); + } + if (jitFlags->IsSet(JitFlags::JIT_FLAG_BBOPT)) { printf("OPTIONS: optimizer should use profile data\n"); diff --git a/src/coreclr/jit/compiler.h b/src/coreclr/jit/compiler.h index 36c8725b3d143..80972c4c36ca6 100644 --- a/src/coreclr/jit/compiler.h +++ b/src/coreclr/jit/compiler.h @@ -5358,8 +5358,6 @@ class Compiler IL_OFFSET fgFindBlockILOffset(BasicBlock* block); void fgFixEntryFlowForOSR(); - void fgUpdateSingleReturnBlock(BasicBlock* block); - BasicBlock* fgSplitBlockAtBeginning(BasicBlock* curr); BasicBlock* fgSplitBlockAtEnd(BasicBlock* curr); BasicBlock* fgSplitBlockAfterStatement(BasicBlock* curr, Statement* stmt); @@ -10745,14 +10743,14 @@ XXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXX unsigned compHndBBtabCount; // element count of used elements in EH data array unsigned compHndBBtabAllocCount; // element count of allocated elements in EH data array -#if defined(TARGET_X86) +#if !defined(FEATURE_EH_FUNCLETS) //------------------------------------------------------------------------- // Tracking of region covered by the monitor in synchronized methods void* syncStartEmitCookie; // the emitter cookie for first instruction after the call to MON_ENTER void* syncEndEmitCookie; // the emitter cookie for first instruction after the call to MON_EXIT -#endif // !TARGET_X86 +#endif // !FEATURE_EH_FUNCLETS Phases mostRecentlyActivePhase; // the most recently active phase PhaseChecks activePhaseChecks; // the currently active phase checks diff --git a/src/coreclr/jit/compiler.hpp b/src/coreclr/jit/compiler.hpp index a686de150d5fe..b00f3e6613921 100644 --- a/src/coreclr/jit/compiler.hpp +++ b/src/coreclr/jit/compiler.hpp @@ -2320,7 +2320,7 @@ inline void LclVarDsc::incRefCnts(weight_t weight, Compiler* comp, RefCountState inline bool Compiler::lvaKeepAliveAndReportThis() { - if (info.compIsStatic || lvaTable[0].TypeGet() != TYP_REF) + if (info.compIsStatic || (lvaTable[0].TypeGet() != TYP_REF)) { return false; } @@ -2354,7 +2354,7 @@ inline bool Compiler::lvaKeepAliveAndReportThis() // We keep it alive in the lookup scenario, even when the VM didn't ask us to, // because collectible types need the generics context when gc-ing. // - // Methoods that can inspire OSR methods must always report context as live + // Methods that can inspire OSR methods must always report context as live // if (genericsContextIsThis) { @@ -4977,8 +4977,9 @@ unsigned Compiler::fgRunDfs(VisitPreorder visitPreorder, VisitPostorder visitPos dfsFrom(fgEntryBB); } - if ((genReturnBB != nullptr) && !BitVecOps::IsMember(&traits, visited, genReturnBB->bbNum) && !fgGlobalMorphDone) + if ((genReturnBB != nullptr) && !BitVecOps::IsMember(&traits, visited, genReturnBB->bbNum)) { + assert(!fgGlobalMorphDone); // We introduce the merged return BB before morph and will redirect // other returns to it as part of morph; keep it reachable. dfsFrom(genReturnBB); diff --git a/src/coreclr/jit/emit.cpp b/src/coreclr/jit/emit.cpp index b993c29e1dda3..b6adc24d27520 100644 --- a/src/coreclr/jit/emit.cpp +++ b/src/coreclr/jit/emit.cpp @@ -2333,16 +2333,11 @@ void emitter::emitGeneratePrologEpilog() void emitter::emitStartPrologEpilogGeneration() { - /* Save the current IG if it's non-empty */ - - if (emitCurIGnonEmpty()) + // Save the current IG if we have one. It might be empty if we added an end-of-compilation label. + if (emitCurIG != nullptr) { emitSavIG(); } - else - { - assert(emitCurIG == nullptr); - } } /***************************************************************************** diff --git a/src/coreclr/jit/fgbasic.cpp b/src/coreclr/jit/fgbasic.cpp index a7a7cc0b5d980..d88ed59a8f196 100644 --- a/src/coreclr/jit/fgbasic.cpp +++ b/src/coreclr/jit/fgbasic.cpp @@ -4755,24 +4755,6 @@ IL_OFFSET Compiler::fgFindBlockILOffset(BasicBlock* block) return BAD_IL_OFFSET; } -//------------------------------------------------------------------------------ -// fgUpdateSingleReturnBlock : A block has been split. If it was the single return -// block, then update the single return block pointer. -// -// Arguments: -// block - The block that was split -// -void Compiler::fgUpdateSingleReturnBlock(BasicBlock* block) -{ - assert(block->KindIs(BBJ_ALWAYS)); - if (genReturnBB == block) - { - assert(block->GetTarget()->KindIs(BBJ_RETURN)); - JITDUMP("Updating genReturnBB from " FMT_BB " to " FMT_BB "\n", block->bbNum, block->GetTarget()->bbNum); - genReturnBB = block->GetTarget(); - } -} - //------------------------------------------------------------------------------ // fgSplitBlockAtEnd - split the given block into two blocks. // All code in the block stays in the original block. @@ -4849,8 +4831,6 @@ BasicBlock* Compiler::fgSplitBlockAtEnd(BasicBlock* curr) fgAddRefPred(newBlock, curr); - fgUpdateSingleReturnBlock(curr); - return newBlock; } diff --git a/src/coreclr/jit/fgdiagnostic.cpp b/src/coreclr/jit/fgdiagnostic.cpp index 1b8ed455ae5a6..c0f1ade1d077e 100644 --- a/src/coreclr/jit/fgdiagnostic.cpp +++ b/src/coreclr/jit/fgdiagnostic.cpp @@ -2255,10 +2255,10 @@ void Compiler::fgTableDispBasicBlock(BasicBlock* block, int ibcColWidth /* = 0 * } } - // Indicate if it's the single return block + // Indicate if it's the merged return block. if (block == genReturnBB) { - printf(" one-return"); + printf(" merged-return"); } printf("\n"); diff --git a/src/coreclr/jit/flowgraph.cpp b/src/coreclr/jit/flowgraph.cpp index 1b33f59358133..e65d161b8813a 100644 --- a/src/coreclr/jit/flowgraph.cpp +++ b/src/coreclr/jit/flowgraph.cpp @@ -1868,7 +1868,7 @@ class MergedReturns return; } - // We'e reached our threshold + // We've reached our threshold mergingReturns = true; // Merge any returns we've already identified @@ -2478,29 +2478,25 @@ PhaseStatus Compiler::fgAddInternal() if (info.compFlags & CORINFO_FLG_SYNCH) { - GenTree* tree = NULL; + GenTree* tree = nullptr; /* Insert the expression "enterCrit(this)" or "enterCrit(handle)" */ if (info.compIsStatic) { tree = fgGetCritSectOfStaticMethod(); - tree = gtNewHelperCallNode(CORINFO_HELP_MON_ENTER_STATIC, TYP_VOID, tree); } else { noway_assert(lvaTable[info.compThisArg].lvType == TYP_REF); - tree = gtNewLclvNode(info.compThisArg, TYP_REF); - tree = gtNewHelperCallNode(CORINFO_HELP_MON_ENTER, TYP_VOID, tree); } /* Create a new basic block and stick the call in it */ fgEnsureFirstBBisScratch(); - fgNewStmtAtEnd(fgFirstBB, tree); #ifdef DEBUG @@ -2522,13 +2518,11 @@ PhaseStatus Compiler::fgAddInternal() if (info.compIsStatic) { tree = fgGetCritSectOfStaticMethod(); - tree = gtNewHelperCallNode(CORINFO_HELP_MON_EXIT_STATIC, TYP_VOID, tree); } else { tree = gtNewLclvNode(info.compThisArg, TYP_REF); - tree = gtNewHelperCallNode(CORINFO_HELP_MON_EXIT, TYP_VOID, tree); } @@ -2537,15 +2531,16 @@ PhaseStatus Compiler::fgAddInternal() #ifdef DEBUG if (verbose) { - printf("\nSynchronized method - Add exit expression "); - printTreeID(tree); + printf("\nSynchronized method - Add exitCrit statement in single return block %s\n", + genReturnBB->dspToString()); + gtDispTree(tree); printf("\n"); } #endif // Reset cookies used to track start and end of the protected region in synchronized methods - syncStartEmitCookie = NULL; - syncEndEmitCookie = NULL; + syncStartEmitCookie = nullptr; + syncEndEmitCookie = nullptr; madeChanges = true; } diff --git a/src/coreclr/jit/gcencode.cpp b/src/coreclr/jit/gcencode.cpp index ec92b88605f15..ad75dbf627099 100644 --- a/src/coreclr/jit/gcencode.cpp +++ b/src/coreclr/jit/gcencode.cpp @@ -1563,15 +1563,16 @@ size_t GCInfo::gcInfoBlockHdrSave( header->syncStartOffset = INVALID_SYNC_OFFSET; header->syncEndOffset = INVALID_SYNC_OFFSET; + #ifndef UNIX_X86_ABI // JIT is responsible for synchronization on funclet-based EH model that x86/Linux uses. if (compiler->info.compFlags & CORINFO_FLG_SYNCH) { - assert(compiler->syncStartEmitCookie != NULL); + assert(compiler->syncStartEmitCookie != nullptr); header->syncStartOffset = compiler->GetEmitter()->emitCodeOffset(compiler->syncStartEmitCookie, 0); assert(header->syncStartOffset != INVALID_SYNC_OFFSET); - assert(compiler->syncEndEmitCookie != NULL); + assert(compiler->syncEndEmitCookie != nullptr); header->syncEndOffset = compiler->GetEmitter()->emitCodeOffset(compiler->syncEndEmitCookie, 0); assert(header->syncEndOffset != INVALID_SYNC_OFFSET); diff --git a/src/coreclr/jit/lower.cpp b/src/coreclr/jit/lower.cpp index dd5fc6b3673ba..27079d52e3bfb 100644 --- a/src/coreclr/jit/lower.cpp +++ b/src/coreclr/jit/lower.cpp @@ -4271,7 +4271,7 @@ void Lowering::LowerRet(GenTreeUnOp* ret) } // Method doing PInvokes has exactly one return block unless it has tail calls. - if (comp->compMethodRequiresPInvokeFrame() && (comp->compCurBB == comp->genReturnBB)) + if (comp->compMethodRequiresPInvokeFrame()) { InsertPInvokeMethodEpilog(comp->compCurBB DEBUGARG(ret)); } @@ -5369,7 +5369,7 @@ void Lowering::InsertPInvokeMethodEpilog(BasicBlock* returnBB DEBUGARG(GenTree* JITDUMP("======= Inserting PInvoke method epilog\n"); // Method doing PInvoke calls has exactly one return block unless it has "jmp" or tail calls. - assert(((returnBB == comp->genReturnBB) && returnBB->KindIs(BBJ_RETURN)) || returnBB->endsWithTailCallOrJmp(comp)); + assert(returnBB->KindIs(BBJ_RETURN) || returnBB->endsWithTailCallOrJmp(comp)); LIR::Range& returnBlockRange = LIR::AsRange(returnBB); diff --git a/src/coreclr/jit/morph.cpp b/src/coreclr/jit/morph.cpp index ca7c46ef6654d..1bf3288fa7c3c 100644 --- a/src/coreclr/jit/morph.cpp +++ b/src/coreclr/jit/morph.cpp @@ -14140,6 +14140,14 @@ PhaseStatus Compiler::fgMorphBlocks() fgEntryBB = nullptr; } + // We don't maintain `genReturnBB` after this point. + if (genReturnBB != nullptr) + { + // It no longer needs special "keep" treatment. + genReturnBB->RemoveFlags(BBF_DONT_REMOVE); + genReturnBB = nullptr; + } + // We are done with the global morphing phase // fgInvalidateDfsTree();