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 f28c4cab5c57a..560c257dc372d 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 ef6032d34570f..22798c5144464 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 db0e588bc5656..b8cad08133746 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 560c257dc372d..544b1ff148db0 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 f27c4302fe4e1..7c9230aeea4fa 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 c5d245ba83c66..c0291b6c0b3dd 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 544b1ff148db0..ed714bf20fc75 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 ed714bf20fc75..fefe2549aa027 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 7c9230aeea4fa..55c3fb6725baa 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 c0291b6c0b3dd..bd3a615420b39 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 fefe2549aa027..f43ef18f0ab21 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 f43ef18f0ab21..d9dcace172607 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 7fb9387204cae..f05753ab41bf9 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 de1185dda5c52e8b9c7c7b76234ade05f1a13537 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 d9dcace172607..9bef734c3a05d 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 861a02886bcc47b6004a0cce852f55a1dbd58639 Mon Sep 17 00:00:00 2001 From: Brian Bohe Date: Tue, 25 Oct 2022 13:10:11 -0700 Subject: [PATCH 08/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 9bef734c3a05d..7ca68f8224f7b 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 f05753ab41bf9..7fb9387204cae 100644 --- a/src/coreclr/jit/ee_il_dll.cpp +++ b/src/coreclr/jit/ee_il_dll.cpp @@ -842,9 +842,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 2717653532c391e0f360d3ab504c70d2ed63b22f Mon Sep 17 00:00:00 2001 From: Brian Bohe Date: Tue, 25 Oct 2022 13:13:27 -0700 Subject: [PATCH 09/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 7ca68f8224f7b..12ca273e9af30 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 a6ad2b307efbf7566ec03c9e0cbe23aea5020d97 Mon Sep 17 00:00:00 2001 From: Brian Bohe Date: Tue, 25 Oct 2022 13:14:02 -0700 Subject: [PATCH 10/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 12ca273e9af30..85d007feac467 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 55c3fb6725baa..7c9230aeea4fa 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 bd3a615420b39..c0291b6c0b3dd 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 6d6f7a753c70f6323b64074c1007ddb444145b79 Mon Sep 17 00:00:00 2001 From: Brian Bohe Date: Tue, 25 Oct 2022 13:14:54 -0700 Subject: [PATCH 11/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 85d007feac467..4b631ba336d82 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 c4e4818f02f0c0c023b887e04748433e5a07eb9b Mon Sep 17 00:00:00 2001 From: Brian Bohe Date: Tue, 25 Oct 2022 13:15:17 -0700 Subject: [PATCH 12/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 4b631ba336d82..5bb23fee74bdc 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 7c9230aeea4fa..f27c4302fe4e1 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 c0291b6c0b3dd..c5d245ba83c66 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 4e5acdf69301420061b67482abddcaba6b4c1362 Mon Sep 17 00:00:00 2001 From: Brian Bohe Date: Tue, 25 Oct 2022 13:16:19 -0700 Subject: [PATCH 13/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 5bb23fee74bdc..c24d17ae3b04e 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 22798c5144464..ef6032d34570f 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 b8cad08133746..db0e588bc5656 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 */ From 8364689007f6f9611098b2b985b8fc6fbc312047 Mon Sep 17 00:00:00 2001 From: Brian Bohe Date: Fri, 28 Oct 2022 14:33:14 -0700 Subject: [PATCH 14/16] Freeing vm memory when there is no debug info --- src/coreclr/jit/ee_il_dll.cpp | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/src/coreclr/jit/ee_il_dll.cpp b/src/coreclr/jit/ee_il_dll.cpp index 7fb9387204cae..8b1bd4088be22 100644 --- a/src/coreclr/jit/ee_il_dll.cpp +++ b/src/coreclr/jit/ee_il_dll.cpp @@ -693,8 +693,14 @@ void Compiler::eeSetLVdone() eeDispVars(info.compMethodHnd, eeVarsCount, (ICorDebugInfo::NativeVarInfo*)eeVars); } #endif // DEBUG - - info.compCompHnd->setVars(info.compMethodHnd, eeVarsCount, (ICorDebugInfo::NativeVarInfo*)eeVars); + if (0 < eeVarsCount) + { + info.compCompHnd->setVars(info.compMethodHnd, eeVarsCount, (ICorDebugInfo::NativeVarInfo*)eeVars); + } + else if (eeVars != nullptr) + { + info.compCompHnd->freeArray(eeVars); + } eeVars = nullptr; // We give up ownership after setVars() } From 719f9e461c29a06daa11132c1e0b7125feb75e1d Mon Sep 17 00:00:00 2001 From: Brian Bohe Date: Mon, 31 Oct 2022 11:57:53 -0700 Subject: [PATCH 15/16] Persisting JIT-EE contract on empty debug info --- src/coreclr/jit/ee_il_dll.cpp | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/coreclr/jit/ee_il_dll.cpp b/src/coreclr/jit/ee_il_dll.cpp index 8b1bd4088be22..b4ed04f1ff55a 100644 --- a/src/coreclr/jit/ee_il_dll.cpp +++ b/src/coreclr/jit/ee_il_dll.cpp @@ -693,15 +693,15 @@ void Compiler::eeSetLVdone() eeDispVars(info.compMethodHnd, eeVarsCount, (ICorDebugInfo::NativeVarInfo*)eeVars); } #endif // DEBUG - if (0 < eeVarsCount) - { - info.compCompHnd->setVars(info.compMethodHnd, eeVarsCount, (ICorDebugInfo::NativeVarInfo*)eeVars); - } - else if (eeVars != nullptr) + if (0 == eeVarsCount && eeVars != nullptr) { + // We still call setVars with nullptr when eeVarsCount is 0 as part of the contract. + // We also need to free the nonused memory. info.compCompHnd->freeArray(eeVars); + eeVars = nullptr; } + info.compCompHnd->setVars(info.compMethodHnd, eeVarsCount, (ICorDebugInfo::NativeVarInfo*)eeVars); eeVars = nullptr; // We give up ownership after setVars() } From 70fb789c7219dcc07c5b50a7a2548516734f8587 Mon Sep 17 00:00:00 2001 From: Brian Bohe Date: Tue, 1 Nov 2022 14:16:22 -0700 Subject: [PATCH 16/16] Update src/coreclr/jit/ee_il_dll.cpp Co-authored-by: Jakob Botsch Nielsen --- src/coreclr/jit/ee_il_dll.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/coreclr/jit/ee_il_dll.cpp b/src/coreclr/jit/ee_il_dll.cpp index b4ed04f1ff55a..383525a677393 100644 --- a/src/coreclr/jit/ee_il_dll.cpp +++ b/src/coreclr/jit/ee_il_dll.cpp @@ -693,7 +693,7 @@ void Compiler::eeSetLVdone() eeDispVars(info.compMethodHnd, eeVarsCount, (ICorDebugInfo::NativeVarInfo*)eeVars); } #endif // DEBUG - if (0 == eeVarsCount && eeVars != nullptr) + if ((eeVarsCount == 0) && (eeVars != nullptr)) { // We still call setVars with nullptr when eeVarsCount is 0 as part of the contract. // We also need to free the nonused memory.