From 66d18e031f83c8efe864981b35c0548af49e0714 Mon Sep 17 00:00:00 2001 From: Brian Bohe Date: Thu, 20 Oct 2022 12:08:28 -0700 Subject: [PATCH 01/16] Removing empty variable live ranges The debugger is not using empty variable live ranges. We are reporting them because they can get extended later if the variable becomes alive in the immediately next emitted instruction. If an empty live range is not getting extended, which we can realize after emitting all the code or creating a new live range for the same variable, we can remove it. --- src/coreclr/jit/codegencommon.cpp | 52 ++++++++++++++++++++++++++++-- src/coreclr/jit/codegeninterface.h | 7 ++-- src/coreclr/jit/codegenlinear.cpp | 2 ++ 3 files changed, 57 insertions(+), 4 deletions(-) diff --git a/src/coreclr/jit/codegencommon.cpp b/src/coreclr/jit/codegencommon.cpp index f28c4cab5c57a8..560c257dc372d1 100644 --- a/src/coreclr/jit/codegencommon.cpp +++ b/src/coreclr/jit/codegencommon.cpp @@ -8721,7 +8721,7 @@ CodeGenInterface::VariableLiveKeeper::LiveRangeList* CodeGenInterface::VariableL // beginning and exclusive of the end. // void CodeGenInterface::VariableLiveKeeper::VariableLiveDescriptor::startLiveRangeFromEmitter( - CodeGenInterface::siVarLoc varLocation, emitter* emit) const + CodeGenInterface::siVarLoc varLocation, emitter* emit) { noway_assert(emit != nullptr); @@ -8740,6 +8740,10 @@ void CodeGenInterface::VariableLiveKeeper::VariableLiveDescriptor::startLiveRang } else { + if (!m_VariableLiveRanges->empty() && isLastRangeEmpty()) + { + removeLastRange(); + } JITDUMP("New debug range: %s\n", m_VariableLiveRanges->empty() ? "first" @@ -8763,6 +8767,29 @@ void CodeGenInterface::VariableLiveKeeper::VariableLiveDescriptor::startLiveRang noway_assert(!m_VariableLiveRanges->back().m_EndEmitLocation.Valid()); } +//------------------------------------------------------------------------ +// isLastRangeEmpty: Returns whether the last live range [A,B) is empty, +// meaning A == B. +// +bool CodeGenInterface::VariableLiveKeeper::VariableLiveDescriptor::isLastRangeEmpty() const +{ + return !m_VariableLiveRanges->empty() && + m_VariableLiveRanges->back().m_StartEmitLocation == m_VariableLiveRanges->back().m_EndEmitLocation; +} + +//------------------------------------------------------------------------ +// removeLastRange: Removes last live range from list. +// +// Assumptions: +// There should be at least one live range reported for the variable. +// +void CodeGenInterface::VariableLiveKeeper::VariableLiveDescriptor::removeLastRange() +{ + noway_assert(!m_VariableLiveRanges->empty()); + JITDUMP("Removing last entry"); + m_VariableLiveRanges->erase(m_VariableLiveRanges->backPosition()); +} + //------------------------------------------------------------------------ // endLiveRangeAtEmitter: Report this variable as becoming dead since the // instruction where "emit" is located. @@ -8807,7 +8834,7 @@ void CodeGenInterface::VariableLiveKeeper::VariableLiveDescriptor::endLiveRangeA // beginning and exclusive of the end. // void CodeGenInterface::VariableLiveKeeper::VariableLiveDescriptor::updateLiveRangeAtEmitter( - CodeGenInterface::siVarLoc varLocation, emitter* emit) const + CodeGenInterface::siVarLoc varLocation, emitter* emit) { // This variable is changing home so it has been started before during this block noway_assert(m_VariableLiveRanges != nullptr && !m_VariableLiveRanges->empty()); @@ -8894,6 +8921,27 @@ CodeGenInterface::VariableLiveKeeper* CodeGenInterface::getVariableLiveKeeper() return varLiveKeeper; }; +//------------------------------------------------------------------------ +// siRemoveEmptyRanges: Removes all last variable ranges that are empty +// +// Notes: +// After finishing generating code, the last variable range reported for +// a variable can be empty. This happens when a variable becomes alive +// and die before the following instruction is emitted, e.g. if the death +// takes place in the operands of the next instruction. +// +void CodeGenInterface::VariableLiveKeeper::siRemoveEmptyRanges() +{ + for (unsigned int varNum = 0; varNum < m_LiveDscCount; varNum++) + { + VariableLiveDescriptor* varLiveDsc = m_vlrLiveDsc + varNum; + if (varLiveDsc->isLastRangeEmpty()) + { + varLiveDsc->removeLastRange(); + } + } +} + //------------------------------------------------------------------------ // VariableLiveKeeper: Create an instance of the object in charge of managing // VariableLiveRanges and initialize the array "m_vlrLiveDsc". diff --git a/src/coreclr/jit/codegeninterface.h b/src/coreclr/jit/codegeninterface.h index ef6032d34570f8..22798c51444643 100644 --- a/src/coreclr/jit/codegeninterface.h +++ b/src/coreclr/jit/codegeninterface.h @@ -697,9 +697,11 @@ class CodeGenInterface bool hasVariableLiveRangeOpen() const; LiveRangeList* getLiveRanges() const; - void startLiveRangeFromEmitter(CodeGenInterface::siVarLoc varLocation, emitter* emit) const; + void startLiveRangeFromEmitter(CodeGenInterface::siVarLoc varLocation, emitter* emit); void endLiveRangeAtEmitter(emitter* emit) const; - void updateLiveRangeAtEmitter(CodeGenInterface::siVarLoc varLocation, emitter* emit) const; + void updateLiveRangeAtEmitter(CodeGenInterface::siVarLoc varLocation, emitter* emit); + bool isLastRangeEmpty() const; + void removeLastRange(); #ifdef DEBUG void dumpAllRegisterLiveRangesForBlock(emitter* emit, const CodeGenInterface* codeGen) const; @@ -738,6 +740,7 @@ class CodeGenInterface void siUpdateVariableLiveRange(const LclVarDsc* varDsc, unsigned int varNum); void siEndAllVariableLiveRange(VARSET_VALARG_TP varsToClose); void siEndAllVariableLiveRange(); + void siRemoveEmptyRanges(); // Remove empty variable live ranges LiveRangeList* getLiveRangesForVarForBody(unsigned int varNum) const; LiveRangeList* getLiveRangesForVarForProlog(unsigned int varNum) const; diff --git a/src/coreclr/jit/codegenlinear.cpp b/src/coreclr/jit/codegenlinear.cpp index db0e588bc56564..b8cad08133746b 100644 --- a/src/coreclr/jit/codegenlinear.cpp +++ b/src/coreclr/jit/codegenlinear.cpp @@ -859,6 +859,8 @@ void CodeGen::genCodeForBBlist() // There could be variables alive at this point. For example see lvaKeepAliveAndReportThis. // This call is for cleaning the GC refs genUpdateLife(VarSetOps::MakeEmpty(compiler)); + // No more code is generated so we can safely remove empty variable debug info ranges + getVariableLiveKeeper()->siRemoveEmptyRanges(); /* Finalize the spill tracking logic */ From 609605a1ca7cf1c0c843dbaf353432ec9ed846e2 Mon Sep 17 00:00:00 2001 From: Brian Bohe Date: Fri, 21 Oct 2022 14:52:08 -0700 Subject: [PATCH 02/16] Extending variable live ranges in more cases When the emitter moved to the next group but has not emitted any instruction, and the variable died and becomes alive again, we would like to extend its range. --- src/coreclr/jit/codegencommon.cpp | 2 +- src/coreclr/jit/emit.cpp | 9 +++++---- src/coreclr/jit/emit.h | 2 +- 3 files changed, 7 insertions(+), 6 deletions(-) diff --git a/src/coreclr/jit/codegencommon.cpp b/src/coreclr/jit/codegencommon.cpp index 560c257dc372d1..544b1ff148db0b 100644 --- a/src/coreclr/jit/codegencommon.cpp +++ b/src/coreclr/jit/codegencommon.cpp @@ -8730,7 +8730,7 @@ void CodeGenInterface::VariableLiveKeeper::VariableLiveDescriptor::startLiveRang if (!m_VariableLiveRanges->empty() && siVarLoc::Equals(&varLocation, &(m_VariableLiveRanges->back().m_VarLocation)) && - m_VariableLiveRanges->back().m_EndEmitLocation.IsPreviousInsNum(emit)) + m_VariableLiveRanges->back().m_EndEmitLocation.IsLessOneInsAway(emit)) { JITDUMP("Extending debug range...\n"); diff --git a/src/coreclr/jit/emit.cpp b/src/coreclr/jit/emit.cpp index f27c4302fe4e1e..7c9230aeea4fa0 100644 --- a/src/coreclr/jit/emit.cpp +++ b/src/coreclr/jit/emit.cpp @@ -65,13 +65,14 @@ UNATIVE_OFFSET emitLocation::GetFuncletPrologOffset(emitter* emit) const } //------------------------------------------------------------------------ -// IsPreviousInsNum: Returns true if the emitter is on the next instruction -// of the same group as this emitLocation. +// IsLessOneInsAway: Returns true if the emitter is on the same or next +// emitted location. That means the same or next instruction at the same +// group as this emitLocation or at the beginning of the next group. // // Arguments: // emit - an emitter* instance // -bool emitLocation::IsPreviousInsNum(emitter* emit) const +bool emitLocation::IsLessOneInsAway(emitter* emit) const { assert(Valid()); @@ -84,7 +85,7 @@ bool emitLocation::IsPreviousInsNum(emitter* emit) const // Spanning an IG boundary? if (ig->igNext == emit->emitCurIG) { - return (emitGetInsNumFromCodePos(codePos) == ig->igInsCnt) && (emit->emitCurIGinsCnt == 1); + return (emitGetInsNumFromCodePos(codePos) == ig->igInsCnt) && (emit->emitCurIGinsCnt <= 1); } return false; diff --git a/src/coreclr/jit/emit.h b/src/coreclr/jit/emit.h index c5d245ba83c66d..c0291b6c0b3dd8 100644 --- a/src/coreclr/jit/emit.h +++ b/src/coreclr/jit/emit.h @@ -190,7 +190,7 @@ class emitLocation UNATIVE_OFFSET GetFuncletPrologOffset(emitter* emit) const; - bool IsPreviousInsNum(emitter* emit) const; + bool IsLessOneInsAway(emitter* emit) const; #ifdef DEBUG void Print(LONG compMethodID) const; From a11fd5d0ffaa98631d731bec1f2619f1d108d33a Mon Sep 17 00:00:00 2001 From: Brian Bohe Date: Fri, 21 Oct 2022 16:49:42 -0700 Subject: [PATCH 03/16] Avoiding creating a new debug range when previous is empty --- src/coreclr/jit/codegencommon.cpp | 18 ++++++++++++------ 1 file changed, 12 insertions(+), 6 deletions(-) diff --git a/src/coreclr/jit/codegencommon.cpp b/src/coreclr/jit/codegencommon.cpp index 544b1ff148db0b..ed714bf20fc751 100644 --- a/src/coreclr/jit/codegencommon.cpp +++ b/src/coreclr/jit/codegencommon.cpp @@ -8740,18 +8740,24 @@ void CodeGenInterface::VariableLiveKeeper::VariableLiveDescriptor::startLiveRang } else { - if (!m_VariableLiveRanges->empty() && isLastRangeEmpty()) - { - removeLastRange(); - } JITDUMP("New debug range: %s\n", m_VariableLiveRanges->empty() ? "first" : siVarLoc::Equals(&varLocation, &(m_VariableLiveRanges->back().m_VarLocation)) ? "new var or location" : "not adjacent"); - // Creates new live range with invalid end - m_VariableLiveRanges->emplace_back(varLocation, emitLocation(), emitLocation()); + // If we can tell from emitLocations that the previous debug range was empty + // we are replacing it with the new one, otherwise creating a space of it. + // The new range has undefined end. + if (isLastRangeEmpty()) + { + m_VariableLiveRanges->back().m_VarLocation = varLocation; + m_VariableLiveRanges->back().m_EndEmitLocation.Init(); + } + else + { + m_VariableLiveRanges->emplace_back(varLocation, emitLocation(), emitLocation()); + } m_VariableLiveRanges->back().m_StartEmitLocation.CaptureLocation(emit); } From e8b102d489d79028750068decf4f427ad8e5f69f Mon Sep 17 00:00:00 2001 From: Brian Bohe Date: Fri, 21 Oct 2022 16:50:21 -0700 Subject: [PATCH 04/16] Updating check for empty debug ranges --- src/coreclr/jit/codegencommon.cpp | 6 ++++-- src/coreclr/jit/emit.cpp | 23 +++++++++++++++++++++++ src/coreclr/jit/emit.h | 1 + 3 files changed, 28 insertions(+), 2 deletions(-) diff --git a/src/coreclr/jit/codegencommon.cpp b/src/coreclr/jit/codegencommon.cpp index ed714bf20fc751..fefe2549aa0270 100644 --- a/src/coreclr/jit/codegencommon.cpp +++ b/src/coreclr/jit/codegencommon.cpp @@ -8775,12 +8775,14 @@ void CodeGenInterface::VariableLiveKeeper::VariableLiveDescriptor::startLiveRang //------------------------------------------------------------------------ // isLastRangeEmpty: Returns whether the last live range [A,B) is empty, -// meaning A == B. +// meaning A == B. This is an approximation as instructions may be removed +// after being emitted. // bool CodeGenInterface::VariableLiveKeeper::VariableLiveDescriptor::isLastRangeEmpty() const { return !m_VariableLiveRanges->empty() && - m_VariableLiveRanges->back().m_StartEmitLocation == m_VariableLiveRanges->back().m_EndEmitLocation; + m_VariableLiveRanges->back().m_StartEmitLocation.noDistanceWith( + m_VariableLiveRanges->back().m_EndEmitLocation); } //------------------------------------------------------------------------ diff --git a/src/coreclr/jit/emit.cpp b/src/coreclr/jit/emit.cpp index 7c9230aeea4fa0..55c3fb6725baaa 100644 --- a/src/coreclr/jit/emit.cpp +++ b/src/coreclr/jit/emit.cpp @@ -91,6 +91,29 @@ bool emitLocation::IsLessOneInsAway(emitter* emit) const return false; } +//------------------------------------------------------------------------ +// noDistanceWith: Returns true if no instruction was emitted between +// both locations. This response is an approximation. There might be emitted +// instructions that were removed after the emitLocatiosn were generated. +// +// Arguments: +// loc - an emitLocation instance +// +// Assumptions: +// this emitLocation was captured before loc. +bool emitLocation::noDistanceWith(emitLocation& loc) const +{ + assert(Valid()); + assert(loc.Valid()); + // Spanning an IG boundary? + if (ig->igNext == loc.ig) + { + return (emitGetInsNumFromCodePos(codePos) == ig->igInsCnt) && (emitGetInsNumFromCodePos(loc.codePos) == 0); + } + // otherwise + return this->operator==(loc); +} + #ifdef DEBUG void emitLocation::Print(LONG compMethodID) const { diff --git a/src/coreclr/jit/emit.h b/src/coreclr/jit/emit.h index c0291b6c0b3dd8..bd3a615420b39f 100644 --- a/src/coreclr/jit/emit.h +++ b/src/coreclr/jit/emit.h @@ -191,6 +191,7 @@ class emitLocation UNATIVE_OFFSET GetFuncletPrologOffset(emitter* emit) const; bool IsLessOneInsAway(emitter* emit) const; + bool noDistanceWith(emitLocation& lhs) const; #ifdef DEBUG void Print(LONG compMethodID) const; From 7b79b0d955daa4dc9770b604d6780591b51c9ee1 Mon Sep 17 00:00:00 2001 From: Brian Bohe Date: Fri, 21 Oct 2022 16:53:08 -0700 Subject: [PATCH 05/16] Updating print --- src/coreclr/jit/codegencommon.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/coreclr/jit/codegencommon.cpp b/src/coreclr/jit/codegencommon.cpp index fefe2549aa0270..f43ef18f0ab210 100644 --- a/src/coreclr/jit/codegencommon.cpp +++ b/src/coreclr/jit/codegencommon.cpp @@ -8794,7 +8794,7 @@ bool CodeGenInterface::VariableLiveKeeper::VariableLiveDescriptor::isLastRangeEm void CodeGenInterface::VariableLiveKeeper::VariableLiveDescriptor::removeLastRange() { noway_assert(!m_VariableLiveRanges->empty()); - JITDUMP("Removing last entry"); + JITDUMP("Removing last debug range entry\n"); m_VariableLiveRanges->erase(m_VariableLiveRanges->backPosition()); } From 4e1cf47dd6cdf9d45ce4a51eaa05b3ec6e4b3b41 Mon Sep 17 00:00:00 2001 From: Brian Bohe Date: Mon, 24 Oct 2022 00:03:05 -0700 Subject: [PATCH 06/16] Avoiding printing twice variable live range --- src/coreclr/jit/codegencommon.cpp | 7 ------- src/coreclr/jit/ee_il_dll.cpp | 5 +++-- 2 files changed, 3 insertions(+), 9 deletions(-) diff --git a/src/coreclr/jit/codegencommon.cpp b/src/coreclr/jit/codegencommon.cpp index f43ef18f0ab210..d9dcace1726072 100644 --- a/src/coreclr/jit/codegencommon.cpp +++ b/src/coreclr/jit/codegencommon.cpp @@ -2059,13 +2059,6 @@ void CodeGen::genEmitUnwindDebugGCandEH() genSetScopeInfo(); -#if defined(USING_VARIABLE_LIVE_RANGE) && defined(DEBUG) - if (compiler->verbose) - { - varLiveKeeper->dumpLvaVariableLiveRanges(); - } -#endif // defined(USING_VARIABLE_LIVE_RANGE) && defined(DEBUG) - #ifdef LATE_DISASM unsigned finalHotCodeSize; unsigned finalColdCodeSize; diff --git a/src/coreclr/jit/ee_il_dll.cpp b/src/coreclr/jit/ee_il_dll.cpp index 7fb9387204cae4..f05753ab41bf96 100644 --- a/src/coreclr/jit/ee_il_dll.cpp +++ b/src/coreclr/jit/ee_il_dll.cpp @@ -842,8 +842,9 @@ void Compiler::eeDispVar(ICorDebugInfo::NativeVarInfo* var) { name = "typeCtx"; } - printf("%3d(%10s) : From %08Xh to %08Xh, in ", var->varNumber, - (VarNameToStr(name) == nullptr) ? "UNKNOWN" : VarNameToStr(name), var->startOffset, var->endOffset); + gtDispLclVar(var->varNumber, false); + printf("(%10s) : From %08Xh to %08Xh, in ", (VarNameToStr(name) == nullptr) ? "UNKNOWN" : VarNameToStr(name), + var->startOffset, var->endOffset); switch ((CodeGenInterface::siVarLocType)var->loc.vlType) { From e16588f55699f80207449e9a3cea6edd30600471 Mon Sep 17 00:00:00 2001 From: Brian Bohe Date: Mon, 24 Oct 2022 00:13:38 -0700 Subject: [PATCH 07/16] Avoiding reporting empty variable ranges to the vm --- src/coreclr/jit/codegencommon.cpp | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/src/coreclr/jit/codegencommon.cpp b/src/coreclr/jit/codegencommon.cpp index d9dcace1726072..9bef734c3a05db 100644 --- a/src/coreclr/jit/codegencommon.cpp +++ b/src/coreclr/jit/codegencommon.cpp @@ -6952,8 +6952,11 @@ void CodeGen::genSetScopeInfoUsingVariableRanges() end++; } - genSetScopeInfo(liveRangeIndex, start, end - start, varNum, varNum, true, loc); - liveRangeIndex++; + if (start < end) + { + genSetScopeInfo(liveRangeIndex, start, end - start, varNum, varNum, true, loc); + liveRangeIndex++; + } }; siVarLoc* curLoc = nullptr; From b89c8e367c3d740d17ac2c71609be3e0641b44d0 Mon Sep 17 00:00:00 2001 From: Brian Bohe Date: Mon, 24 Oct 2022 01:23:48 -0700 Subject: [PATCH 08/16] Asking the vm for the exact debug info --- src/coreclr/jit/compiler.h | 7 ++++++- src/coreclr/jit/ee_il_dll.cpp | 27 +++++++++++++++++++-------- 2 files changed, 25 insertions(+), 9 deletions(-) diff --git a/src/coreclr/jit/compiler.h b/src/coreclr/jit/compiler.h index dc3e3c4171e798..c68d574f8200bd 100644 --- a/src/coreclr/jit/compiler.h +++ b/src/coreclr/jit/compiler.h @@ -7718,7 +7718,12 @@ class Compiler UNATIVE_OFFSET endOffset; DWORD varNumber; CodeGenInterface::siVarLoc loc; - } * eeVars; + }; + // We use a temporary buffer to copy all variable debug info, + // and once we know exactly the amount of info to report, + // we ask the vm for memory and copy it. + VarResultInfo* eeJitTempVars; // temporary buffer + VarResultInfo* eeVars; void eeSetLVcount(unsigned count); void eeSetLVinfo(unsigned which, UNATIVE_OFFSET startOffs, diff --git a/src/coreclr/jit/ee_il_dll.cpp b/src/coreclr/jit/ee_il_dll.cpp index f05753ab41bf96..d3dfbe9b3a4bfd 100644 --- a/src/coreclr/jit/ee_il_dll.cpp +++ b/src/coreclr/jit/ee_il_dll.cpp @@ -646,16 +646,16 @@ void Compiler::eeSetLVcount(unsigned count) { assert(opts.compScopeInfo); - JITDUMP("VarLocInfo count is %d\n", count); + JITDUMP("VarLocInfo estimated count is %d\n", count); eeVarsCount = count; if (eeVarsCount) { - eeVars = (VarResultInfo*)info.compCompHnd->allocateArray(eeVarsCount * sizeof(eeVars[0])); + eeJitTempVars = new (this, CMK_DebugInfo) VarResultInfo[count]; } else { - eeVars = nullptr; + eeJitTempVars = nullptr; } } @@ -672,12 +672,12 @@ void Compiler::eeSetLVinfo(unsigned which, assert(eeVarsCount > 0); assert(which < eeVarsCount); - if (eeVars != nullptr) + if (eeJitTempVars != nullptr) { - eeVars[which].startOffset = startOffs; - eeVars[which].endOffset = startOffs + length; - eeVars[which].varNumber = varNum; - eeVars[which].loc = varLoc; + eeJitTempVars[which].startOffset = startOffs; + eeJitTempVars[which].endOffset = startOffs + length; + eeJitTempVars[which].varNumber = varNum; + eeJitTempVars[which].loc = varLoc; } } @@ -687,6 +687,17 @@ void Compiler::eeSetLVdone() assert(sizeof(eeVars[0]) == sizeof(ICorDebugInfo::NativeVarInfo)); assert(opts.compScopeInfo); + if (eeVarsCount) + { + // Ask the vm for memory and copy the pre-calculated VarResultInfo into it + eeVars = (VarResultInfo*)info.compCompHnd->allocateArray(eeVarsCount * sizeof(eeVars[0])); + memcpy(eeVars, eeJitTempVars, eeVarsCount * sizeof(eeVars[0])); + } + else + { + eeVars = nullptr; + } + #ifdef DEBUG if (verbose || opts.dspDebugInfo) { From 05b521f9961219f71263cb1cbb429cca1ba4912f Mon Sep 17 00:00:00 2001 From: Brian Bohe Date: Mon, 24 Oct 2022 09:51:42 -0700 Subject: [PATCH 09/16] Updating comparator Co-authored-by: Jakob Botsch Nielsen --- src/coreclr/jit/emit.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/coreclr/jit/emit.cpp b/src/coreclr/jit/emit.cpp index 55c3fb6725baaa..ec7d45e662e9dd 100644 --- a/src/coreclr/jit/emit.cpp +++ b/src/coreclr/jit/emit.cpp @@ -111,7 +111,7 @@ bool emitLocation::noDistanceWith(emitLocation& loc) const return (emitGetInsNumFromCodePos(codePos) == ig->igInsCnt) && (emitGetInsNumFromCodePos(loc.codePos) == 0); } // otherwise - return this->operator==(loc); + return *this == loc; } #ifdef DEBUG From d3f665228af785fbe878d337971675d85f3cc2de Mon Sep 17 00:00:00 2001 From: Brian Bohe Date: Tue, 25 Oct 2022 13:33:04 -0700 Subject: [PATCH 10/16] Revert "Updating comparator" This reverts commit 05b521f9961219f71263cb1cbb429cca1ba4912f. --- src/coreclr/jit/emit.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/coreclr/jit/emit.cpp b/src/coreclr/jit/emit.cpp index ec7d45e662e9dd..55c3fb6725baaa 100644 --- a/src/coreclr/jit/emit.cpp +++ b/src/coreclr/jit/emit.cpp @@ -111,7 +111,7 @@ bool emitLocation::noDistanceWith(emitLocation& loc) const return (emitGetInsNumFromCodePos(codePos) == ig->igInsCnt) && (emitGetInsNumFromCodePos(loc.codePos) == 0); } // otherwise - return *this == loc; + return this->operator==(loc); } #ifdef DEBUG From ef4ddce4939f518d8bcb4fdb69319dff65d2c113 Mon Sep 17 00:00:00 2001 From: Brian Bohe Date: Tue, 25 Oct 2022 13:34:00 -0700 Subject: [PATCH 11/16] Revert "Avoiding printing twice variable live range" This reverts commit 4e1cf47dd6cdf9d45ce4a51eaa05b3ec6e4b3b41. --- src/coreclr/jit/codegencommon.cpp | 7 +++++++ src/coreclr/jit/ee_il_dll.cpp | 5 ++--- 2 files changed, 9 insertions(+), 3 deletions(-) diff --git a/src/coreclr/jit/codegencommon.cpp b/src/coreclr/jit/codegencommon.cpp index 9bef734c3a05db..7ca68f8224f7b0 100644 --- a/src/coreclr/jit/codegencommon.cpp +++ b/src/coreclr/jit/codegencommon.cpp @@ -2059,6 +2059,13 @@ void CodeGen::genEmitUnwindDebugGCandEH() genSetScopeInfo(); +#if defined(USING_VARIABLE_LIVE_RANGE) && defined(DEBUG) + if (compiler->verbose) + { + varLiveKeeper->dumpLvaVariableLiveRanges(); + } +#endif // defined(USING_VARIABLE_LIVE_RANGE) && defined(DEBUG) + #ifdef LATE_DISASM unsigned finalHotCodeSize; unsigned finalColdCodeSize; diff --git a/src/coreclr/jit/ee_il_dll.cpp b/src/coreclr/jit/ee_il_dll.cpp index d3dfbe9b3a4bfd..16f81f360200bb 100644 --- a/src/coreclr/jit/ee_il_dll.cpp +++ b/src/coreclr/jit/ee_il_dll.cpp @@ -853,9 +853,8 @@ void Compiler::eeDispVar(ICorDebugInfo::NativeVarInfo* var) { name = "typeCtx"; } - gtDispLclVar(var->varNumber, false); - printf("(%10s) : From %08Xh to %08Xh, in ", (VarNameToStr(name) == nullptr) ? "UNKNOWN" : VarNameToStr(name), - var->startOffset, var->endOffset); + printf("%3d(%10s) : From %08Xh to %08Xh, in ", var->varNumber, + (VarNameToStr(name) == nullptr) ? "UNKNOWN" : VarNameToStr(name), var->startOffset, var->endOffset); switch ((CodeGenInterface::siVarLocType)var->loc.vlType) { From d9b8f107d64bc604706b9c8610566ed08f4c28a9 Mon Sep 17 00:00:00 2001 From: Brian Bohe Date: Tue, 25 Oct 2022 13:34:25 -0700 Subject: [PATCH 12/16] Revert "Updating print" This reverts commit 7b79b0d955daa4dc9770b604d6780591b51c9ee1. --- src/coreclr/jit/codegencommon.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/coreclr/jit/codegencommon.cpp b/src/coreclr/jit/codegencommon.cpp index 7ca68f8224f7b0..12ca273e9af30b 100644 --- a/src/coreclr/jit/codegencommon.cpp +++ b/src/coreclr/jit/codegencommon.cpp @@ -8797,7 +8797,7 @@ bool CodeGenInterface::VariableLiveKeeper::VariableLiveDescriptor::isLastRangeEm void CodeGenInterface::VariableLiveKeeper::VariableLiveDescriptor::removeLastRange() { noway_assert(!m_VariableLiveRanges->empty()); - JITDUMP("Removing last debug range entry\n"); + JITDUMP("Removing last entry"); m_VariableLiveRanges->erase(m_VariableLiveRanges->backPosition()); } From bb73a9ae5089c68a855abefdcc24d1cd4118359a Mon Sep 17 00:00:00 2001 From: Brian Bohe Date: Tue, 25 Oct 2022 13:34:34 -0700 Subject: [PATCH 13/16] Revert "Updating check for empty debug ranges" This reverts commit e8b102d489d79028750068decf4f427ad8e5f69f. --- src/coreclr/jit/codegencommon.cpp | 6 ++---- src/coreclr/jit/emit.cpp | 23 ----------------------- src/coreclr/jit/emit.h | 1 - 3 files changed, 2 insertions(+), 28 deletions(-) diff --git a/src/coreclr/jit/codegencommon.cpp b/src/coreclr/jit/codegencommon.cpp index 12ca273e9af30b..85d007feac467e 100644 --- a/src/coreclr/jit/codegencommon.cpp +++ b/src/coreclr/jit/codegencommon.cpp @@ -8778,14 +8778,12 @@ void CodeGenInterface::VariableLiveKeeper::VariableLiveDescriptor::startLiveRang //------------------------------------------------------------------------ // isLastRangeEmpty: Returns whether the last live range [A,B) is empty, -// meaning A == B. This is an approximation as instructions may be removed -// after being emitted. +// meaning A == B. // bool CodeGenInterface::VariableLiveKeeper::VariableLiveDescriptor::isLastRangeEmpty() const { return !m_VariableLiveRanges->empty() && - m_VariableLiveRanges->back().m_StartEmitLocation.noDistanceWith( - m_VariableLiveRanges->back().m_EndEmitLocation); + m_VariableLiveRanges->back().m_StartEmitLocation == m_VariableLiveRanges->back().m_EndEmitLocation; } //------------------------------------------------------------------------ diff --git a/src/coreclr/jit/emit.cpp b/src/coreclr/jit/emit.cpp index 55c3fb6725baaa..7c9230aeea4fa0 100644 --- a/src/coreclr/jit/emit.cpp +++ b/src/coreclr/jit/emit.cpp @@ -91,29 +91,6 @@ bool emitLocation::IsLessOneInsAway(emitter* emit) const return false; } -//------------------------------------------------------------------------ -// noDistanceWith: Returns true if no instruction was emitted between -// both locations. This response is an approximation. There might be emitted -// instructions that were removed after the emitLocatiosn were generated. -// -// Arguments: -// loc - an emitLocation instance -// -// Assumptions: -// this emitLocation was captured before loc. -bool emitLocation::noDistanceWith(emitLocation& loc) const -{ - assert(Valid()); - assert(loc.Valid()); - // Spanning an IG boundary? - if (ig->igNext == loc.ig) - { - return (emitGetInsNumFromCodePos(codePos) == ig->igInsCnt) && (emitGetInsNumFromCodePos(loc.codePos) == 0); - } - // otherwise - return this->operator==(loc); -} - #ifdef DEBUG void emitLocation::Print(LONG compMethodID) const { diff --git a/src/coreclr/jit/emit.h b/src/coreclr/jit/emit.h index bd3a615420b39f..c0291b6c0b3dd8 100644 --- a/src/coreclr/jit/emit.h +++ b/src/coreclr/jit/emit.h @@ -191,7 +191,6 @@ class emitLocation UNATIVE_OFFSET GetFuncletPrologOffset(emitter* emit) const; bool IsLessOneInsAway(emitter* emit) const; - bool noDistanceWith(emitLocation& lhs) const; #ifdef DEBUG void Print(LONG compMethodID) const; From 21b09af63a8880604e7837b9aa481eafe918aa1c Mon Sep 17 00:00:00 2001 From: Brian Bohe Date: Tue, 25 Oct 2022 13:34:44 -0700 Subject: [PATCH 14/16] Revert "Avoiding creating a new debug range when previous is empty" This reverts commit a11fd5d0ffaa98631d731bec1f2619f1d108d33a. --- src/coreclr/jit/codegencommon.cpp | 18 ++++++------------ 1 file changed, 6 insertions(+), 12 deletions(-) diff --git a/src/coreclr/jit/codegencommon.cpp b/src/coreclr/jit/codegencommon.cpp index 85d007feac467e..4b631ba336d82e 100644 --- a/src/coreclr/jit/codegencommon.cpp +++ b/src/coreclr/jit/codegencommon.cpp @@ -8743,24 +8743,18 @@ void CodeGenInterface::VariableLiveKeeper::VariableLiveDescriptor::startLiveRang } else { + if (!m_VariableLiveRanges->empty() && isLastRangeEmpty()) + { + removeLastRange(); + } JITDUMP("New debug range: %s\n", m_VariableLiveRanges->empty() ? "first" : siVarLoc::Equals(&varLocation, &(m_VariableLiveRanges->back().m_VarLocation)) ? "new var or location" : "not adjacent"); - // If we can tell from emitLocations that the previous debug range was empty - // we are replacing it with the new one, otherwise creating a space of it. - // The new range has undefined end. - if (isLastRangeEmpty()) - { - m_VariableLiveRanges->back().m_VarLocation = varLocation; - m_VariableLiveRanges->back().m_EndEmitLocation.Init(); - } - else - { - m_VariableLiveRanges->emplace_back(varLocation, emitLocation(), emitLocation()); - } + // Creates new live range with invalid end + m_VariableLiveRanges->emplace_back(varLocation, emitLocation(), emitLocation()); m_VariableLiveRanges->back().m_StartEmitLocation.CaptureLocation(emit); } From b7a7d47080500ecbd3e434dc961da7a801a44da9 Mon Sep 17 00:00:00 2001 From: Brian Bohe Date: Tue, 25 Oct 2022 13:34:57 -0700 Subject: [PATCH 15/16] Revert "Extending variable live ranges in more cases" This reverts commit 609605a1ca7cf1c0c843dbaf353432ec9ed846e2. --- src/coreclr/jit/codegencommon.cpp | 2 +- src/coreclr/jit/emit.cpp | 9 ++++----- src/coreclr/jit/emit.h | 2 +- 3 files changed, 6 insertions(+), 7 deletions(-) diff --git a/src/coreclr/jit/codegencommon.cpp b/src/coreclr/jit/codegencommon.cpp index 4b631ba336d82e..5bb23fee74bdcb 100644 --- a/src/coreclr/jit/codegencommon.cpp +++ b/src/coreclr/jit/codegencommon.cpp @@ -8733,7 +8733,7 @@ void CodeGenInterface::VariableLiveKeeper::VariableLiveDescriptor::startLiveRang if (!m_VariableLiveRanges->empty() && siVarLoc::Equals(&varLocation, &(m_VariableLiveRanges->back().m_VarLocation)) && - m_VariableLiveRanges->back().m_EndEmitLocation.IsLessOneInsAway(emit)) + m_VariableLiveRanges->back().m_EndEmitLocation.IsPreviousInsNum(emit)) { JITDUMP("Extending debug range...\n"); diff --git a/src/coreclr/jit/emit.cpp b/src/coreclr/jit/emit.cpp index 7c9230aeea4fa0..f27c4302fe4e1e 100644 --- a/src/coreclr/jit/emit.cpp +++ b/src/coreclr/jit/emit.cpp @@ -65,14 +65,13 @@ UNATIVE_OFFSET emitLocation::GetFuncletPrologOffset(emitter* emit) const } //------------------------------------------------------------------------ -// IsLessOneInsAway: Returns true if the emitter is on the same or next -// emitted location. That means the same or next instruction at the same -// group as this emitLocation or at the beginning of the next group. +// IsPreviousInsNum: Returns true if the emitter is on the next instruction +// of the same group as this emitLocation. // // Arguments: // emit - an emitter* instance // -bool emitLocation::IsLessOneInsAway(emitter* emit) const +bool emitLocation::IsPreviousInsNum(emitter* emit) const { assert(Valid()); @@ -85,7 +84,7 @@ bool emitLocation::IsLessOneInsAway(emitter* emit) const // Spanning an IG boundary? if (ig->igNext == emit->emitCurIG) { - return (emitGetInsNumFromCodePos(codePos) == ig->igInsCnt) && (emit->emitCurIGinsCnt <= 1); + return (emitGetInsNumFromCodePos(codePos) == ig->igInsCnt) && (emit->emitCurIGinsCnt == 1); } return false; diff --git a/src/coreclr/jit/emit.h b/src/coreclr/jit/emit.h index c0291b6c0b3dd8..c5d245ba83c66d 100644 --- a/src/coreclr/jit/emit.h +++ b/src/coreclr/jit/emit.h @@ -190,7 +190,7 @@ class emitLocation UNATIVE_OFFSET GetFuncletPrologOffset(emitter* emit) const; - bool IsLessOneInsAway(emitter* emit) const; + bool IsPreviousInsNum(emitter* emit) const; #ifdef DEBUG void Print(LONG compMethodID) const; From 6c926b80d6e892f6796fdd6857e5f502e2ba7ddf Mon Sep 17 00:00:00 2001 From: Brian Bohe Date: Tue, 25 Oct 2022 13:35:09 -0700 Subject: [PATCH 16/16] Revert "Removing empty variable live ranges" This reverts commit 66d18e031f83c8efe864981b35c0548af49e0714. --- src/coreclr/jit/codegencommon.cpp | 52 ++---------------------------- src/coreclr/jit/codegeninterface.h | 7 ++-- src/coreclr/jit/codegenlinear.cpp | 2 -- 3 files changed, 4 insertions(+), 57 deletions(-) diff --git a/src/coreclr/jit/codegencommon.cpp b/src/coreclr/jit/codegencommon.cpp index 5bb23fee74bdcb..c24d17ae3b04e9 100644 --- a/src/coreclr/jit/codegencommon.cpp +++ b/src/coreclr/jit/codegencommon.cpp @@ -8724,7 +8724,7 @@ CodeGenInterface::VariableLiveKeeper::LiveRangeList* CodeGenInterface::VariableL // beginning and exclusive of the end. // void CodeGenInterface::VariableLiveKeeper::VariableLiveDescriptor::startLiveRangeFromEmitter( - CodeGenInterface::siVarLoc varLocation, emitter* emit) + CodeGenInterface::siVarLoc varLocation, emitter* emit) const { noway_assert(emit != nullptr); @@ -8743,10 +8743,6 @@ void CodeGenInterface::VariableLiveKeeper::VariableLiveDescriptor::startLiveRang } else { - if (!m_VariableLiveRanges->empty() && isLastRangeEmpty()) - { - removeLastRange(); - } JITDUMP("New debug range: %s\n", m_VariableLiveRanges->empty() ? "first" @@ -8770,29 +8766,6 @@ void CodeGenInterface::VariableLiveKeeper::VariableLiveDescriptor::startLiveRang noway_assert(!m_VariableLiveRanges->back().m_EndEmitLocation.Valid()); } -//------------------------------------------------------------------------ -// isLastRangeEmpty: Returns whether the last live range [A,B) is empty, -// meaning A == B. -// -bool CodeGenInterface::VariableLiveKeeper::VariableLiveDescriptor::isLastRangeEmpty() const -{ - return !m_VariableLiveRanges->empty() && - m_VariableLiveRanges->back().m_StartEmitLocation == m_VariableLiveRanges->back().m_EndEmitLocation; -} - -//------------------------------------------------------------------------ -// removeLastRange: Removes last live range from list. -// -// Assumptions: -// There should be at least one live range reported for the variable. -// -void CodeGenInterface::VariableLiveKeeper::VariableLiveDescriptor::removeLastRange() -{ - noway_assert(!m_VariableLiveRanges->empty()); - JITDUMP("Removing last entry"); - m_VariableLiveRanges->erase(m_VariableLiveRanges->backPosition()); -} - //------------------------------------------------------------------------ // endLiveRangeAtEmitter: Report this variable as becoming dead since the // instruction where "emit" is located. @@ -8837,7 +8810,7 @@ void CodeGenInterface::VariableLiveKeeper::VariableLiveDescriptor::endLiveRangeA // beginning and exclusive of the end. // void CodeGenInterface::VariableLiveKeeper::VariableLiveDescriptor::updateLiveRangeAtEmitter( - CodeGenInterface::siVarLoc varLocation, emitter* emit) + CodeGenInterface::siVarLoc varLocation, emitter* emit) const { // This variable is changing home so it has been started before during this block noway_assert(m_VariableLiveRanges != nullptr && !m_VariableLiveRanges->empty()); @@ -8924,27 +8897,6 @@ CodeGenInterface::VariableLiveKeeper* CodeGenInterface::getVariableLiveKeeper() return varLiveKeeper; }; -//------------------------------------------------------------------------ -// siRemoveEmptyRanges: Removes all last variable ranges that are empty -// -// Notes: -// After finishing generating code, the last variable range reported for -// a variable can be empty. This happens when a variable becomes alive -// and die before the following instruction is emitted, e.g. if the death -// takes place in the operands of the next instruction. -// -void CodeGenInterface::VariableLiveKeeper::siRemoveEmptyRanges() -{ - for (unsigned int varNum = 0; varNum < m_LiveDscCount; varNum++) - { - VariableLiveDescriptor* varLiveDsc = m_vlrLiveDsc + varNum; - if (varLiveDsc->isLastRangeEmpty()) - { - varLiveDsc->removeLastRange(); - } - } -} - //------------------------------------------------------------------------ // VariableLiveKeeper: Create an instance of the object in charge of managing // VariableLiveRanges and initialize the array "m_vlrLiveDsc". diff --git a/src/coreclr/jit/codegeninterface.h b/src/coreclr/jit/codegeninterface.h index 22798c51444643..ef6032d34570f8 100644 --- a/src/coreclr/jit/codegeninterface.h +++ b/src/coreclr/jit/codegeninterface.h @@ -697,11 +697,9 @@ class CodeGenInterface bool hasVariableLiveRangeOpen() const; LiveRangeList* getLiveRanges() const; - void startLiveRangeFromEmitter(CodeGenInterface::siVarLoc varLocation, emitter* emit); + void startLiveRangeFromEmitter(CodeGenInterface::siVarLoc varLocation, emitter* emit) const; void endLiveRangeAtEmitter(emitter* emit) const; - void updateLiveRangeAtEmitter(CodeGenInterface::siVarLoc varLocation, emitter* emit); - bool isLastRangeEmpty() const; - void removeLastRange(); + void updateLiveRangeAtEmitter(CodeGenInterface::siVarLoc varLocation, emitter* emit) const; #ifdef DEBUG void dumpAllRegisterLiveRangesForBlock(emitter* emit, const CodeGenInterface* codeGen) const; @@ -740,7 +738,6 @@ class CodeGenInterface void siUpdateVariableLiveRange(const LclVarDsc* varDsc, unsigned int varNum); void siEndAllVariableLiveRange(VARSET_VALARG_TP varsToClose); void siEndAllVariableLiveRange(); - void siRemoveEmptyRanges(); // Remove empty variable live ranges LiveRangeList* getLiveRangesForVarForBody(unsigned int varNum) const; LiveRangeList* getLiveRangesForVarForProlog(unsigned int varNum) const; diff --git a/src/coreclr/jit/codegenlinear.cpp b/src/coreclr/jit/codegenlinear.cpp index b8cad08133746b..db0e588bc56564 100644 --- a/src/coreclr/jit/codegenlinear.cpp +++ b/src/coreclr/jit/codegenlinear.cpp @@ -859,8 +859,6 @@ void CodeGen::genCodeForBBlist() // There could be variables alive at this point. For example see lvaKeepAliveAndReportThis. // This call is for cleaning the GC refs genUpdateLife(VarSetOps::MakeEmpty(compiler)); - // No more code is generated so we can safely remove empty variable debug info ranges - getVariableLiveKeeper()->siRemoveEmptyRanges(); /* Finalize the spill tracking logic */