From: Sergey Andreenko Date: Thu, 30 Aug 2018 02:55:24 +0000 (-0700) Subject: Delete code that tracks stack level in morph. (dotnet/coreclr#19703) X-Git-Tag: submit/tizen/20210909.063632~11030^2~4010 X-Git-Url: http://review.tizen.org/git/?a=commitdiff_plain;h=78f74b4e8a7964dc7b7a81f99dd4ec3614e81197;p=platform%2Fupstream%2Fdotnet%2Fruntime.git Delete code that tracks stack level in morph. (dotnet/coreclr#19703) * call fgCheckArgCnt only from stackLevelSetter * delete changing fgPtrArgCntMax from codegencommon * delete fgPtrArgCntCur * reset write phase only once * delete gtStkDepth * add headers for the new fucntions * fix comments Commit migrated from https://github.com/dotnet/coreclr/commit/94b847fc65117d30c62e182b724ac1ffd5ac575c --- diff --git a/src/coreclr/src/jit/codegencommon.cpp b/src/coreclr/src/jit/codegencommon.cpp index cc93f66..e4ba8e6 100644 --- a/src/coreclr/src/jit/codegencommon.cpp +++ b/src/coreclr/src/jit/codegencommon.cpp @@ -6968,14 +6968,8 @@ void CodeGen::genProfilingEnterCallback(regNumber initReg, bool* pInitRegZeroed) EA_UNKNOWN); // retSize #if defined(_TARGET_X86_) - // - // Adjust the number of stack slots used by this managed method if necessary. - // - if (compiler->fgPtrArgCntMax < 1) - { - JITDUMP("Upping fgPtrArgCntMax from %d to 1\n", compiler->fgPtrArgCntMax); - compiler->fgPtrArgCntMax = 1; - } + // Check that we have place for the push. + assert(compiler->fgPtrArgCntMax >= 1); #elif defined(_TARGET_ARM_) if (initReg == argReg) { @@ -7168,14 +7162,8 @@ void CodeGen::genProfilingLeaveCallback(unsigned helper /*= CORINFO_HELP_PROF_FC sizeof(int) * 1, // argSize EA_UNKNOWN); // retSize - // - // Adjust the number of stack slots used by this managed method if necessary. - // - if (compiler->fgPtrArgCntMax < 1) - { - JITDUMP("Upping fgPtrArgCntMax from %d to 1\n", compiler->fgPtrArgCntMax); - compiler->fgPtrArgCntMax = 1; - } + // Check that we have place for the push. + assert(compiler->fgPtrArgCntMax >= 1); #elif defined(_TARGET_ARM_) // diff --git a/src/coreclr/src/jit/compiler.h b/src/coreclr/src/jit/compiler.h index 11067ba..cb1ca16 100644 --- a/src/coreclr/src/jit/compiler.h +++ b/src/coreclr/src/jit/compiler.h @@ -1580,9 +1580,6 @@ public: void EvalArgsToTemps(); - void RecordStkLevel(unsigned stkLvl); - unsigned RetrieveStkLevel(); - unsigned ArgCount() { return argCount; @@ -4001,7 +3998,6 @@ public: bool fgMorphBlockStmt(BasicBlock* block, GenTreeStmt* stmt DEBUGARG(const char* msg)); - void fgCheckArgCnt(); void fgSetOptions(); #ifdef DEBUG @@ -4889,7 +4885,6 @@ private: //------------------------- Morphing -------------------------------------- - unsigned fgPtrArgCntCur; unsigned fgPtrArgCntMax; public: @@ -4914,15 +4909,15 @@ public: fgPtrArgCntMax = argCntMax; } + bool compCanEncodePtrArgCntMax(); + private: hashBv* fgOutgoingArgTemps; hashBv* fgCurrentlyInUseArgTemps; - bool compCanEncodePtrArgCntMax(); - void fgSetRngChkTarget(GenTree* tree, bool delay = true); - BasicBlock* fgSetRngChkTargetInner(SpecialCodeKind kind, bool delay, unsigned* stkDepth); + BasicBlock* fgSetRngChkTargetInner(SpecialCodeKind kind, bool delay); #if REARRANGE_ADDS void fgMoveOpsLeft(GenTree* tree); @@ -5114,9 +5109,9 @@ private: bool fgRngChkThrowAdded; AddCodeDsc* fgExcptnTargetCache[SCK_COUNT]; - BasicBlock* fgRngChkTarget(BasicBlock* block, unsigned stkDepth, SpecialCodeKind kind); + BasicBlock* fgRngChkTarget(BasicBlock* block, SpecialCodeKind kind); - BasicBlock* fgAddCodeRef(BasicBlock* srcBlk, unsigned refData, SpecialCodeKind kind, unsigned stkDepth = 0); + BasicBlock* fgAddCodeRef(BasicBlock* srcBlk, unsigned refData, SpecialCodeKind kind); public: AddCodeDsc* fgFindExcptnTarget(SpecialCodeKind kind, unsigned refData); diff --git a/src/coreclr/src/jit/flowgraph.cpp b/src/coreclr/src/jit/flowgraph.cpp index 9b361ff..872fc1d 100644 --- a/src/coreclr/src/jit/flowgraph.cpp +++ b/src/coreclr/src/jit/flowgraph.cpp @@ -116,8 +116,6 @@ void Compiler::fgInit() } /* Keep track of the max count of pointer arguments */ - - fgPtrArgCntCur = 0; fgPtrArgCntMax = 0; /* This global flag is set whenever we remove a statement */ @@ -18119,13 +18117,18 @@ unsigned Compiler::acdHelper(SpecialCodeKind codeKind) } } -/***************************************************************************** - * - * Find/create an added code entry associated with the given block and with - * the given kind. - */ - -BasicBlock* Compiler::fgAddCodeRef(BasicBlock* srcBlk, unsigned refData, SpecialCodeKind kind, unsigned stkDepth) +//------------------------------------------------------------------------ +// fgAddCodeRef: Find/create an added code entry associated with the given block and with the given kind. +// +// Arguments: +// srcBlk - the block that needs an entry; +// refData - the index to use as the cache key for sharing throw blocks; +// kind - the kind of exception; +// +// Return Value: +// The target throw helper block or nullptr if throw helper blocks are disabled. +// +BasicBlock* Compiler::fgAddCodeRef(BasicBlock* srcBlk, unsigned refData, SpecialCodeKind kind) { // Record that the code will call a THROW_HELPER // so on Windows Amd64 we can allocate the 4 outgoing @@ -18155,43 +18158,6 @@ BasicBlock* Compiler::fgAddCodeRef(BasicBlock* srcBlk, unsigned refData, Special if (add) // found it { -#if !FEATURE_FIXED_OUT_ARGS - // If different range checks happen at different stack levels, - // they can't all jump to the same "call @rngChkFailed" AND have - // frameless methods, as the rngChkFailed may need to unwind the - // stack, and we have to be able to report the stack level. - // - // The following check forces most methods that reference an - // array element in a parameter list to have an EBP frame, - // this restriction could be removed with more careful code - // generation for BBJ_THROW (i.e. range check failed). - // - // For Linux/x86, we possibly need to insert stack alignment adjustment - // before the first stack argument pushed for every call. But we - // don't know what the stack alignment adjustment will be when - // we morph a tree that calls fgAddCodeRef(), so the stack depth - // number will be incorrect. For now, simply force all functions with - // these helpers to have EBP frames. It might be possible to make - // this less conservative. E.g., for top-level (not nested) calls - // without stack args, the stack pointer hasn't changed and stack - // depth will be known to be zero. Or, figure out a way to update - // or generate all required helpers after all stack alignment - // has been added, and the stack level at each call to fgAddCodeRef() - // is known, or can be recalculated. - CLANG_FORMAT_COMMENT_ANCHOR; - -#if defined(UNIX_X86_ABI) - codeGen->setFrameRequired(true); - codeGen->setFramePointerRequiredGCInfo(true); -#else // !defined(UNIX_X86_ABI) - if (add->acdStkLvl != stkDepth) - { - codeGen->setFrameRequired(true); - codeGen->setFramePointerRequiredGCInfo(true); - } -#endif // !defined(UNIX_X86_ABI) -#endif // !FEATURE_FIXED_OUT_ARGS - return add->acdDstBlk; } @@ -18202,7 +18168,7 @@ BasicBlock* Compiler::fgAddCodeRef(BasicBlock* srcBlk, unsigned refData, Special add->acdKind = kind; add->acdNext = fgAddCodeList; #if !FEATURE_FIXED_OUT_ARGS - add->acdStkLvl = stkDepth; + add->acdStkLvl = 0; add->acdStkLvlInit = false; #endif // !FEATURE_FIXED_OUT_ARGS @@ -18267,15 +18233,10 @@ BasicBlock* Compiler::fgAddCodeRef(BasicBlock* srcBlk, unsigned refData, Special break; } - printf("\nfgAddCodeRef - Add BB in %s%s, new block %s, stkDepth is %d\n", msgWhere, msg, - add->acdDstBlk->dspToString(), stkDepth); + printf("\nfgAddCodeRef - Add BB in %s%s, new block %s\n", msgWhere, msg, add->acdDstBlk->dspToString()); } #endif // DEBUG -#ifdef DEBUG - newBlk->bbTgtStkDepth = stkDepth; -#endif // DEBUG - /* Mark the block as added by the compiler and not removable by future flow graph optimizations. Note that no bbJumpDest points to these blocks. */ @@ -18384,12 +18345,22 @@ Compiler::AddCodeDsc* Compiler::fgFindExcptnTarget(SpecialCodeKind kind, unsigne * range check is to jump to upon failure. */ -BasicBlock* Compiler::fgRngChkTarget(BasicBlock* block, unsigned stkDepth, SpecialCodeKind kind) +//------------------------------------------------------------------------ +// fgRngChkTarget: Create/find the appropriate "range-fail" label for the block. +// +// Arguments: +// srcBlk - the block that needs an entry; +// kind - the kind of exception; +// +// Return Value: +// The target throw helper block this check jumps to upon failure. +// +BasicBlock* Compiler::fgRngChkTarget(BasicBlock* block, SpecialCodeKind kind) { #ifdef DEBUG if (verbose) { - printf("*** Computing fgRngChkTarget for block " FMT_BB " to stkDepth %d\n", block->bbNum, stkDepth); + printf("*** Computing fgRngChkTarget for block " FMT_BB "\n", block->bbNum); if (!block->IsLIR()) { gtDispTree(compCurStmt); @@ -18399,7 +18370,7 @@ BasicBlock* Compiler::fgRngChkTarget(BasicBlock* block, unsigned stkDepth, Speci /* We attach the target label to the containing try block (if any) */ noway_assert(!compIsForInlining()); - return fgAddCodeRef(block, bbThrowIndex(block), kind, stkDepth); + return fgAddCodeRef(block, bbThrowIndex(block), kind); } // Sequences the tree. diff --git a/src/coreclr/src/jit/gentree.cpp b/src/coreclr/src/jit/gentree.cpp index 466d4f7..620087a 100644 --- a/src/coreclr/src/jit/gentree.cpp +++ b/src/coreclr/src/jit/gentree.cpp @@ -7443,7 +7443,6 @@ GenTree* Compiler::gtCloneExpr( asIndAddr->gtStructElemClass, asIndAddr->gtElemSize, asIndAddr->gtLenOffset, asIndAddr->gtElemOffset); copy->AsIndexAddr()->gtIndRngFailBB = asIndAddr->gtIndRngFailBB; - copy->AsIndexAddr()->gtStkDepth = asIndAddr->gtStkDepth; } break; @@ -7782,7 +7781,6 @@ GenTree* Compiler::gtCloneExpr( gtCloneExpr(tree->gtBoundsChk.gtArrLen, addFlags, deepVarNum, deepVarVal), tree->gtBoundsChk.gtThrowKind); copy->gtBoundsChk.gtIndRngFailBB = tree->gtBoundsChk.gtIndRngFailBB; - copy->gtBoundsChk.gtStkDepth = tree->gtBoundsChk.gtStkDepth; break; case GT_STORE_DYN_BLK: diff --git a/src/coreclr/src/jit/gentree.h b/src/coreclr/src/jit/gentree.h index 5841e9e..006812b 100644 --- a/src/coreclr/src/jit/gentree.h +++ b/src/coreclr/src/jit/gentree.h @@ -4223,7 +4223,6 @@ struct GenTreeIndexAddr : public GenTreeOp CORINFO_CLASS_HANDLE gtStructElemClass; // If the element type is a struct, this is the struct type. GenTree* gtIndRngFailBB; // Label to jump to for array-index-out-of-range - unsigned gtStkDepth; // Stack depth at which the jump occurs (required for fgSetRngChkTarget) var_types gtElemType; // The element type of the array. unsigned gtElemSize; // size of elements in the array @@ -4240,7 +4239,6 @@ struct GenTreeIndexAddr : public GenTreeOp : GenTreeOp(GT_INDEX_ADDR, TYP_BYREF, arr, ind) , gtStructElemClass(structElemClass) , gtIndRngFailBB(nullptr) - , gtStkDepth(0) , gtElemType(elemType) , gtElemSize(elemSize) , gtLenOffset(lenOffset) @@ -4314,18 +4312,8 @@ struct GenTreeBoundsChk : public GenTree GenTree* gtIndRngFailBB; // Label to jump to for array-index-out-of-range SpecialCodeKind gtThrowKind; // Kind of throw block to branch to on failure - /* Only out-of-ranges at same stack depth can jump to the same label (finding return address is easier) - For delayed calling of fgSetRngChkTarget() so that the - optimizer has a chance of eliminating some of the rng checks */ - unsigned gtStkDepth; - GenTreeBoundsChk(genTreeOps oper, var_types type, GenTree* index, GenTree* arrLen, SpecialCodeKind kind) - : GenTree(oper, type) - , gtIndex(index) - , gtArrLen(arrLen) - , gtIndRngFailBB(nullptr) - , gtThrowKind(kind) - , gtStkDepth(0) + : GenTree(oper, type), gtIndex(index), gtArrLen(arrLen), gtIndRngFailBB(nullptr), gtThrowKind(kind) { // Effects flags propagate upwards. gtFlags |= (arrLen->gtFlags & GTF_ALL_EFFECT); diff --git a/src/coreclr/src/jit/morph.cpp b/src/coreclr/src/jit/morph.cpp index 26edaaf..2868c36 100644 --- a/src/coreclr/src/jit/morph.cpp +++ b/src/coreclr/src/jit/morph.cpp @@ -751,7 +751,7 @@ OPTIMIZECAST: if (tree->gtOverflow()) { - fgAddCodeRef(compCurBB, bbThrowIndex(compCurBB), SCK_OVERFLOW, fgPtrArgCntCur); + fgAddCodeRef(compCurBB, bbThrowIndex(compCurBB), SCK_OVERFLOW); } return tree; @@ -2563,18 +2563,6 @@ GenTree* fgArgInfo::GetLateArg(unsigned argIndex) unreached(); } -void fgArgInfo::RecordStkLevel(unsigned stkLvl) -{ - assert(!IsUninitialized(stkLvl)); - this->stkLevel = stkLvl; -} - -unsigned fgArgInfo::RetrieveStkLevel() -{ - assert(!IsUninitialized(stkLevel)); - return stkLevel; -} - // Return a conservative estimate of the stack size in bytes. // It will be used only on the intercepted-for-host code path to copy the arguments. int Compiler::fgEstimateCallStackSize(GenTreeCall* call) @@ -2711,8 +2699,7 @@ GenTreeCall* Compiler::fgMorphArgs(GenTreeCall* call) GenTree* args; GenTree* argx; - unsigned flagsSummary = 0; - unsigned genPtrArgCntSav = fgPtrArgCntCur; + unsigned flagsSummary = 0; unsigned argIndex = 0; @@ -2859,11 +2846,13 @@ GenTreeCall* Compiler::fgMorphArgs(GenTreeCall* call) // On remorph, we grab it from the arg table. unsigned numArgs = 0; + // TODO: this comment is no longer correct, do an appropriate fix. // Process the late arguments (which were determined by a previous caller). // Do this before resetting fgPtrArgCntCur as fgMorphTree(call->gtCallLateArgs) // may need to refer to it. if (reMorphing) { + // TODO: this comment is no longer correct, do an appropriate fix. // We need to reMorph the gtCallLateArgs early since that is what triggers // the expression folding and we need to have the final folded gtCallLateArgs // available when we call RemorphRegArg so that we correctly update the fgArgInfo @@ -2881,11 +2870,8 @@ GenTreeCall* Compiler::fgMorphArgs(GenTreeCall* call) // if (call->gtCallLateArgs != nullptr) { - unsigned callStkLevel = call->fgArgInfo->RetrieveStkLevel(); - fgPtrArgCntCur += callStkLevel; call->gtCallLateArgs = fgMorphTree(call->gtCallLateArgs)->AsArgList(); flagsSummary |= call->gtCallLateArgs->gtFlags; - fgPtrArgCntCur -= callStkLevel; } assert(call->fgArgInfo != nullptr); call->fgArgInfo->RemorphReset(); @@ -4191,7 +4177,6 @@ GenTreeCall* Compiler::fgMorphArgs(GenTreeCall* call) unsigned numRegsPartial = MAX_REG_ARG - intArgRegNum; assert((unsigned char)numRegsPartial == numRegsPartial); call->fgArgInfo->SplitArg(argIndex, numRegsPartial, size - numRegsPartial); - fgPtrArgCntCur += size - numRegsPartial; } #endif // FEATURE_ARG_SPLIT @@ -4222,8 +4207,6 @@ GenTreeCall* Compiler::fgMorphArgs(GenTreeCall* call) } else // We have an argument that is not passed in a register { - fgPtrArgCntCur += size; - // If the register arguments have not been determined then we must fill in the argInfo if (reMorphing) @@ -4356,30 +4339,6 @@ GenTreeCall* Compiler::fgMorphArgs(GenTreeCall* call) call->gtCallAddr = fgMorphTree(call->gtCallAddr); } - call->fgArgInfo->RecordStkLevel(fgPtrArgCntCur); - - if ((call->gtCallType == CT_INDIRECT) && (call->gtCallCookie != nullptr)) - { - fgPtrArgCntCur++; - } - - /* Remember the maximum value we ever see */ - - if (fgPtrArgCntMax < fgPtrArgCntCur) - { - JITDUMP("Upping fgPtrArgCntMax from %d to %d\n", fgPtrArgCntMax, fgPtrArgCntCur); - fgPtrArgCntMax = fgPtrArgCntCur; - } - - assert(fgPtrArgCntCur >= genPtrArgCntSav); -#if defined(UNIX_X86_ABI) - call->fgArgInfo->SetStkSizeBytes((fgPtrArgCntCur - genPtrArgCntSav) * TARGET_POINTER_SIZE); -#endif // UNIX_X86_ABI - - /* The call will pop all the arguments we pushed */ - - fgPtrArgCntCur = genPtrArgCntSav; - #if FEATURE_FIXED_OUT_ARGS // Record the outgoing argument size. If the call is a fast tail @@ -5612,7 +5571,7 @@ void Compiler::fgSetRngChkTarget(GenTree* tree, bool delay) if (tree->OperIsBoundsCheck()) { GenTreeBoundsChk* const boundsChk = tree->AsBoundsChk(); - BasicBlock* const failBlock = fgSetRngChkTargetInner(boundsChk->gtThrowKind, delay, &boundsChk->gtStkDepth); + BasicBlock* const failBlock = fgSetRngChkTargetInner(boundsChk->gtThrowKind, delay); if (failBlock != nullptr) { boundsChk->gtIndRngFailBB = gtNewCodeRef(failBlock); @@ -5621,7 +5580,7 @@ void Compiler::fgSetRngChkTarget(GenTree* tree, bool delay) else if (tree->OperIs(GT_INDEX_ADDR)) { GenTreeIndexAddr* const indexAddr = tree->AsIndexAddr(); - BasicBlock* const failBlock = fgSetRngChkTargetInner(SCK_RNGCHK_FAIL, delay, &indexAddr->gtStkDepth); + BasicBlock* const failBlock = fgSetRngChkTargetInner(SCK_RNGCHK_FAIL, delay); if (failBlock != nullptr) { indexAddr->gtIndRngFailBB = gtNewCodeRef(failBlock); @@ -5630,51 +5589,23 @@ void Compiler::fgSetRngChkTarget(GenTree* tree, bool delay) else { noway_assert(tree->OperIs(GT_ARR_ELEM, GT_ARR_INDEX)); - fgSetRngChkTargetInner(SCK_RNGCHK_FAIL, delay, nullptr); + fgSetRngChkTargetInner(SCK_RNGCHK_FAIL, delay); } } -BasicBlock* Compiler::fgSetRngChkTargetInner(SpecialCodeKind kind, bool delay, unsigned* stkDepth) +BasicBlock* Compiler::fgSetRngChkTargetInner(SpecialCodeKind kind, bool delay) { if (opts.MinOpts()) { delay = false; - -#if !FEATURE_FIXED_OUT_ARGS - // we need to initialize this field - if (fgGlobalMorph && (stkDepth != nullptr)) - { - *stkDepth = fgPtrArgCntCur; - } -#endif // !FEATURE_FIXED_OUT_ARGS } if (!opts.compDbgCode) { - if (delay || compIsForInlining()) + if (!delay && !compIsForInlining()) { -#if !FEATURE_FIXED_OUT_ARGS - // We delay this until after loop-oriented range check analysis. For now we merely store the current stack - // level in the tree node. - if (stkDepth != nullptr) - { - *stkDepth = fgPtrArgCntCur; - } -#endif // !FEATURE_FIXED_OUT_ARGS - } - else - { -#if !FEATURE_FIXED_OUT_ARGS - // fgPtrArgCntCur is only valid for global morph or if we walk full stmt. - noway_assert(fgGlobalMorph || (stkDepth != nullptr)); - const unsigned theStkDepth = fgGlobalMorph ? fgPtrArgCntCur : *stkDepth; -#else - // only x86 pushes args - const unsigned theStkDepth = 0; -#endif - // Create/find the appropriate "range-fail" label - return fgRngChkTarget(compCurBB, theStkDepth, kind); + return fgRngChkTarget(compCurBB, kind); } } @@ -11757,6 +11688,7 @@ GenTree* Compiler::fgMorphSmpOp(GenTree* tree, MorphAddrContext* mac) USE_HELPER_FOR_ARITH: { + // TODO: this comment is wrong now, do an appropriate fix. /* We have to morph these arithmetic operations into helper calls before morphing the arguments (preorder), else the arguments won't get correct values of fgPtrArgCntCur. @@ -12896,13 +12828,13 @@ DONE_MORPHING_CHILDREN: if (!varTypeIsFloating(tree->gtType)) { // Codegen for this instruction needs to be able to throw two exceptions: - fgAddCodeRef(compCurBB, bbThrowIndex(compCurBB), SCK_OVERFLOW, fgPtrArgCntCur); - fgAddCodeRef(compCurBB, bbThrowIndex(compCurBB), SCK_DIV_BY_ZERO, fgPtrArgCntCur); + fgAddCodeRef(compCurBB, bbThrowIndex(compCurBB), SCK_OVERFLOW); + fgAddCodeRef(compCurBB, bbThrowIndex(compCurBB), SCK_DIV_BY_ZERO); } break; case GT_UDIV: // Codegen for this instruction needs to be able to throw one exception: - fgAddCodeRef(compCurBB, bbThrowIndex(compCurBB), SCK_DIV_BY_ZERO, fgPtrArgCntCur); + fgAddCodeRef(compCurBB, bbThrowIndex(compCurBB), SCK_DIV_BY_ZERO); break; #endif @@ -12915,7 +12847,7 @@ DONE_MORPHING_CHILDREN: // Add the excptn-throwing basic block to jump to on overflow - fgAddCodeRef(compCurBB, bbThrowIndex(compCurBB), SCK_OVERFLOW, fgPtrArgCntCur); + fgAddCodeRef(compCurBB, bbThrowIndex(compCurBB), SCK_OVERFLOW); // We can't do any commutative morphing for overflow instructions @@ -13211,7 +13143,7 @@ DONE_MORPHING_CHILDREN: noway_assert(varTypeIsFloating(op1->TypeGet())); - fgAddCodeRef(compCurBB, bbThrowIndex(compCurBB), SCK_ARITH_EXCPN, fgPtrArgCntCur); + fgAddCodeRef(compCurBB, bbThrowIndex(compCurBB), SCK_ARITH_EXCPN); break; case GT_OBJ: @@ -14574,7 +14506,6 @@ GenTree* Compiler::fgMorphToEmulatedFP(GenTree* tree) { int helper; GenTree* args; - size_t argc = genTypeStSz(typ); /* Not all FP operations need helper calls */ @@ -14600,17 +14531,10 @@ GenTree* Compiler::fgMorphToEmulatedFP(GenTree* tree) /* Keep track of how many arguments we're passing */ - fgPtrArgCntCur += argc; - /* Is this a binary operator? */ if (op2) { - /* Add the second operand to the argument count */ - - fgPtrArgCntCur += argc; - argc *= 2; - /* What kind of an operator do we have? */ switch (oper) @@ -14709,13 +14633,6 @@ GenTree* Compiler::fgMorphToEmulatedFP(GenTree* tree) tree = fgMorphIntoHelperCall(tree, helper, args); - if (fgPtrArgCntMax < fgPtrArgCntCur) - { - JITDUMP("Upping fgPtrArgCntMax from %d to %d\n", fgPtrArgCntMax, fgPtrArgCntCur); - fgPtrArgCntMax = fgPtrArgCntCur; - } - - fgPtrArgCntCur -= argc; return tree; case GT_RETURN: @@ -15865,8 +15782,6 @@ void Compiler::fgMorphStmts(BasicBlock* block, bool* lnot, bool* loadw) stmt->gtStmtExpr = tree = morph; - noway_assert(fgPtrArgCntCur == 0); - if (fgRemoveRestOfBlock) { continue; @@ -16130,45 +16045,6 @@ void Compiler::fgMorphBlocks() #endif } -//------------------------------------------------------------------------ -// fgCheckArgCnt: Check whether the maximum arg size will change codegen requirements -// -// Notes: -// fpPtrArgCntMax records the maximum number of pushed arguments. -// 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 been factored out of fgSetOptions() because -// the Rationalizer can create new calls. -// -// Assumptions: -// This must be called before isFramePointerRequired() is called, because it is a -// phased variable (can only be written before it has been read). -// -void Compiler::fgCheckArgCnt() -{ - if (!compCanEncodePtrArgCntMax()) - { -#ifdef DEBUG - if (verbose) - { - printf("Too many pushed arguments for fully interruptible encoding, marking method as partially " - "interruptible\n"); - } -#endif - genInterruptible = false; - } - if (fgPtrArgCntMax >= sizeof(unsigned)) - { -#ifdef DEBUG - if (verbose) - { - printf("Too many pushed arguments for an ESP based encoding, forcing an EBP frame\n"); - } -#endif - codeGen->setFramePointerRequired(true); - } -} - /***************************************************************************** * * Make some decisions about the kind of code to generate. @@ -16255,8 +16131,6 @@ void Compiler::fgSetOptions() } #endif // UNIX_X86_ABI - fgCheckArgCnt(); - if (info.compCallUnmanaged) { codeGen->setFramePointerRequired(true); // Setup of Pinvoke frame currently requires an EBP style frame diff --git a/src/coreclr/src/jit/rationalize.cpp b/src/coreclr/src/jit/rationalize.cpp index 209026d..7e72993 100644 --- a/src/coreclr/src/jit/rationalize.cpp +++ b/src/coreclr/src/jit/rationalize.cpp @@ -164,8 +164,6 @@ void Rationalizer::RewriteNodeAsCall(GenTree** use, #endif call = comp->fgMorphArgs(call); - // Determine if this call has changed any codegen requirements. - comp->fgCheckArgCnt(); // Replace "tree" with "call" if (parents.Height() > 1) diff --git a/src/coreclr/src/jit/stacklevelsetter.cpp b/src/coreclr/src/jit/stacklevelsetter.cpp index 4d17e7e..60371c3 100644 --- a/src/coreclr/src/jit/stacklevelsetter.cpp +++ b/src/coreclr/src/jit/stacklevelsetter.cpp @@ -20,6 +20,8 @@ StackLevelSetter::StackLevelSetter(Compiler* compiler) , throwHelperBlocksUsed(comp->fgUseThrowHelperBlocks() && comp->compUsesThrowHelper) #endif // !FEATURE_FIXED_OUT_ARGS { + // The constructor reads this value to skip iterations that could set it if it is already set. + compiler->codeGen->resetWritePhaseForFramePointerRequired(); } //------------------------------------------------------------------------ @@ -43,19 +45,16 @@ void StackLevelSetter::DoPhase() } #if !FEATURE_FIXED_OUT_ARGS - if (framePointerRequired && !comp->codeGen->isFramePointerRequired()) + if (framePointerRequired) { - JITDUMP("framePointerRequired is not set when it is required\n"); - comp->codeGen->resetWritePhaseForFramePointerRequired(); comp->codeGen->setFramePointerRequired(true); } #endif // !FEATURE_FIXED_OUT_ARGS - if (maxStackLevel != comp->fgGetPtrArgCntMax()) - { - JITDUMP("fgPtrArgCntMax was calculated wrong during the morph, the old value: %u, the right value: %u.\n", - comp->fgGetPtrArgCntMax(), maxStackLevel); - comp->fgSetPtrArgCntMax(maxStackLevel); - } + + CheckAdditionalArgs(); + + comp->fgSetPtrArgCntMax(maxStackLevel); + CheckArgCnt(); } //------------------------------------------------------------------------ @@ -97,11 +96,9 @@ void StackLevelSetter::ProcessBlock(BasicBlock* block) if (node->IsCall()) { - GenTreeCall* call = node->AsCall(); - - unsigned usedStackSlotsCount = PopArgumentsFromCall(call); + GenTreeCall* call = node->AsCall(); + unsigned usedStackSlotsCount = PopArgumentsFromCall(call); #if defined(UNIX_X86_ABI) - assert(call->fgArgInfo->GetStkSizeBytes() == usedStackSlotsCount * TARGET_POINTER_SIZE); call->fgArgInfo->SetStkSizeBytes(usedStackSlotsCount * TARGET_POINTER_SIZE); #endif // UNIX_X86_ABI } @@ -179,10 +176,37 @@ void StackLevelSetter::SetThrowHelperBlock(SpecialCodeKind kind, BasicBlock* blo assert(add != nullptr); if (add->acdStkLvlInit) { + // If different range checks happen at different stack levels, + // they can't all jump to the same "call @rngChkFailed" AND have + // frameless methods, as the rngChkFailed may need to unwind the + // stack, and we have to be able to report the stack level. + // + // The following check forces most methods that reference an + // array element in a parameter list to have an EBP frame, + // this restriction could be removed with more careful code + // generation for BBJ_THROW (i.e. range check failed). + // + // For Linux/x86, we possibly need to insert stack alignment adjustment + // before the first stack argument pushed for every call. But we + // don't know what the stack alignment adjustment will be when + // we morph a tree that calls fgAddCodeRef(), so the stack depth + // number will be incorrect. For now, simply force all functions with + // these helpers to have EBP frames. It might be possible to make + // this less conservative. E.g., for top-level (not nested) calls + // without stack args, the stack pointer hasn't changed and stack + // depth will be known to be zero. Or, figure out a way to update + // or generate all required helpers after all stack alignment + // has been added, and the stack level at each call to fgAddCodeRef() + // is known, or can be recalculated. + CLANG_FORMAT_COMMENT_ANCHOR; +#if defined(UNIX_X86_ABI) + framePointerRequired = true; +#else // !defined(UNIX_X86_ABI) if (add->acdStkLvl != currentStackLevel) { framePointerRequired = true; } +#endif // !defined(UNIX_X86_ABI) } else { @@ -268,3 +292,61 @@ void StackLevelSetter::SubStackLevel(unsigned value) assert(currentStackLevel >= value); currentStackLevel -= value; } + +//------------------------------------------------------------------------ +// CheckArgCnt: Check whether the maximum arg size will change codegen requirements. +// +// Notes: +// CheckArgCnt records the maximum number of pushed arguments. +// 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. +// +// Assumptions: +// This must be called when isFramePointerRequired() is in a write phase, because it is a +// phased variable (can only be written before it has been read). +// +void StackLevelSetter::CheckArgCnt() +{ + if (!comp->compCanEncodePtrArgCntMax()) + { +#ifdef DEBUG + if (comp->verbose) + { + printf("Too many pushed arguments for fully interruptible encoding, marking method as partially " + "interruptible\n"); + } +#endif + comp->genInterruptible = false; + } + if (maxStackLevel >= sizeof(unsigned)) + { +#ifdef DEBUG + if (comp->verbose) + { + printf("Too many pushed arguments for an ESP based encoding, forcing an EBP frame\n"); + } +#endif + comp->codeGen->setFramePointerRequired(true); + } +} + +//------------------------------------------------------------------------ +// CheckAdditionalArgs: Check if there are additional args that need stack slots. +// +// Notes: +// Currently only x86 profiler hook needs it. +// +void StackLevelSetter::CheckAdditionalArgs() +{ +#if defined(_TARGET_X86_) + if (comp->compIsProfilerHookNeeded()) + { + if (maxStackLevel == 0) + { + JITDUMP("Upping fgPtrArgCntMax from %d to 1\n", maxStackLevel); + maxStackLevel = 1; + } + } +#endif // _TARGET_X86_ +} diff --git a/src/coreclr/src/jit/stacklevelsetter.h b/src/coreclr/src/jit/stacklevelsetter.h index 6a5b61d..984cf54 100644 --- a/src/coreclr/src/jit/stacklevelsetter.h +++ b/src/coreclr/src/jit/stacklevelsetter.h @@ -26,6 +26,9 @@ private: void AddStackLevel(unsigned value); void SubStackLevel(unsigned value); + void CheckArgCnt(); + void CheckAdditionalArgs(); + private: unsigned currentStackLevel; // current number of stack slots used by arguments. unsigned maxStackLevel; // max number of stack slots for arguments.