From 3024a1d66b79c7089958ad50cf6ca7e9e0aac210 Mon Sep 17 00:00:00 2001 From: Brian Bohe Date: Wed, 22 Jan 2020 18:18:11 -0800 Subject: [PATCH 1/5] Change debug info default variable tracking system. Varible Scope info is being disabled. Variable Live Range is being enabled. Debug info generated for debug code is the same, changes only impact on optimized code. Variable Live Range updates variable's location each time something happens to variable's liveness. For more information see https://github.com/dotnet/runtime/blob/master/docs/design/features/variabletracking.md --- src/coreclr/src/jit/codegenarmarch.cpp | 6 +++--- src/coreclr/src/jit/codegeninterface.h | 4 ++-- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/src/coreclr/src/jit/codegenarmarch.cpp b/src/coreclr/src/jit/codegenarmarch.cpp index 47678ff275adc4..4350e1d75c0151 100644 --- a/src/coreclr/src/jit/codegenarmarch.cpp +++ b/src/coreclr/src/jit/codegenarmarch.cpp @@ -2412,11 +2412,11 @@ void CodeGen::genRegCopy(GenTree* treeNode) #ifdef USING_VARIABLE_LIVE_RANGE // Report the home change for this variable - varLiveKeeper->siUpdateVariableLiveRange(varDsc, lcl->GetLclNum()) + varLiveKeeper->siUpdateVariableLiveRange(varDsc, lcl->GetLclNum()); #endif // USING_VARIABLE_LIVE_RANGE - // The new location is going live - genUpdateRegLife(varDsc, /*isBorn*/ true, /*isDying*/ false DEBUGARG(treeNode)); + // The new location is going live + genUpdateRegLife(varDsc, /*isBorn*/ true, /*isDying*/ false DEBUGARG(treeNode)); } } } diff --git a/src/coreclr/src/jit/codegeninterface.h b/src/coreclr/src/jit/codegeninterface.h index f9cb28e446e396..5868529b85463e 100644 --- a/src/coreclr/src/jit/codegeninterface.h +++ b/src/coreclr/src/jit/codegeninterface.h @@ -26,11 +26,11 @@ #include "treelifeupdater.h" #include "emit.h" -#if 1 +#if 0 // Enable USING_SCOPE_INFO flag to use psiScope/siScope info to report variables' locations. #define USING_SCOPE_INFO #endif -#if 0 +#if 1 // Enable USING_VARIABLE_LIVE_RANGE flag to use VariableLiveRange info to report variables' locations. // Note: if both USING_SCOPE_INFO and USING_VARIABLE_LIVE_RANGE are defined, then USING_SCOPE_INFO // information is reported to the debugger. From 93c209cb5d70de59637cc83e712b719fea51b520 Mon Sep 17 00:00:00 2001 From: Brian Bohe Date: Fri, 21 Feb 2020 11:43:36 -0800 Subject: [PATCH 2/5] Updating VariableLiveRange When a variable is being born at the same location it died at the previous assembly instruction (in the same group of instructions), we reported as it was alive the whole time. --- src/coreclr/src/jit/codegencommon.cpp | 19 +++++++++++++++---- src/coreclr/src/jit/emit.cpp | 9 +++++++++ src/coreclr/src/jit/emit.h | 4 +++- src/coreclr/src/jit/emitpub.h | 2 +- 4 files changed, 28 insertions(+), 6 deletions(-) diff --git a/src/coreclr/src/jit/codegencommon.cpp b/src/coreclr/src/jit/codegencommon.cpp index 1af9bc029c5be8..0bcd8365ae1f61 100644 --- a/src/coreclr/src/jit/codegencommon.cpp +++ b/src/coreclr/src/jit/codegencommon.cpp @@ -11466,10 +11466,21 @@ void CodeGenInterface::VariableLiveKeeper::VariableLiveDescriptor::startLiveRang // Is the first "VariableLiveRange" or the previous one has been closed so its "m_EndEmitLocation" is valid noway_assert(m_VariableLiveRanges->empty() || m_VariableLiveRanges->back().m_EndEmitLocation.Valid()); - // Creates new live range with invalid end - m_VariableLiveRanges->emplace_back(varLocation, emitLocation(), emitLocation()); - m_VariableLiveRanges->back().m_StartEmitLocation.CaptureLocation(emit); - + if (!m_VariableLiveRanges->empty() && + m_VariableLiveRanges->back().m_EndEmitLocation.IsPreviousIns(emit) && + siVarLoc::Equals(&varLocation, &(m_VariableLiveRanges->back().m_VarLocation))) + { + // The variable is being born at the exactly same place just one assembly instructtion. + // We assume it never died. + m_VariableLiveRanges->back().m_EndEmitLocation.Init(); + } + else + { + // Creates new live range with invalid end + m_VariableLiveRanges->emplace_back(varLocation, emitLocation(), emitLocation()); + m_VariableLiveRanges->back().m_StartEmitLocation.CaptureLocation(emit); + } + #ifdef DEBUG if (!m_VariableLifeBarrier->hasLiveRangesToDump()) { diff --git a/src/coreclr/src/jit/emit.cpp b/src/coreclr/src/jit/emit.cpp index 9288f6c2c7392d..a95e70c76c7036 100644 --- a/src/coreclr/src/jit/emit.cpp +++ b/src/coreclr/src/jit/emit.cpp @@ -65,6 +65,15 @@ UNATIVE_OFFSET emitLocation::GetFuncletPrologOffset(emitter* emit) const return emit->emitCurIGsize; } +// Returns true if the emitter is on the next instruction of the same group than this emitLocation +bool emitLocation::IsPreviousIns(const emitter* emit) const +{ + assert(Valid()); + bool sameGroup = ig == emit->emitCurIG; + bool sameInsNum = emitGetInsNumFromCodePos(codePos) == emitGetInsNumFromCodePos(emit->emitCurOffset()) - 1; + return sameGroup && sameInsNum; +} + #ifdef DEBUG void emitLocation::Print() const { diff --git a/src/coreclr/src/jit/emit.h b/src/coreclr/src/jit/emit.h index 37a1795aa13e24..89d09e36949e62 100644 --- a/src/coreclr/src/jit/emit.h +++ b/src/coreclr/src/jit/emit.h @@ -191,6 +191,8 @@ class emitLocation UNATIVE_OFFSET GetFuncletPrologOffset(emitter* emit) const; + bool emitLocation::IsPreviousIns(const emitter* emit) const; + #ifdef DEBUG void Print() const; #endif // DEBUG @@ -2389,7 +2391,7 @@ inline unsigned emitGetInsOfsFromCodePos(unsigned codePos) return (codePos >> 16); } -inline unsigned emitter::emitCurOffset() +inline unsigned emitter::emitCurOffset() const { unsigned codePos = emitCurIGinsCnt + (emitCurIGsize << 16); diff --git a/src/coreclr/src/jit/emitpub.h b/src/coreclr/src/jit/emitpub.h index cd28e0355c03ba..bc8abf362cf739 100644 --- a/src/coreclr/src/jit/emitpub.h +++ b/src/coreclr/src/jit/emitpub.h @@ -65,7 +65,7 @@ void emitFinishPrologEpilogGeneration(); /************************************************************************/ void* emitCurBlock(); -unsigned emitCurOffset(); +unsigned emitCurOffset() const; UNATIVE_OFFSET emitCodeOffset(void* blockPtr, unsigned codeOffs); From 9f55da8cd3b60b6feb5a01b37005cb1345d0f1e5 Mon Sep 17 00:00:00 2001 From: Brian Bohe Date: Fri, 21 Feb 2020 14:51:13 -0800 Subject: [PATCH 3/5] Fixing format problems --- src/coreclr/src/jit/codegencommon.cpp | 5 ++--- src/coreclr/src/jit/emit.cpp | 2 +- 2 files changed, 3 insertions(+), 4 deletions(-) diff --git a/src/coreclr/src/jit/codegencommon.cpp b/src/coreclr/src/jit/codegencommon.cpp index 0bcd8365ae1f61..703b8e699cf7d8 100644 --- a/src/coreclr/src/jit/codegencommon.cpp +++ b/src/coreclr/src/jit/codegencommon.cpp @@ -11466,8 +11466,7 @@ void CodeGenInterface::VariableLiveKeeper::VariableLiveDescriptor::startLiveRang // Is the first "VariableLiveRange" or the previous one has been closed so its "m_EndEmitLocation" is valid noway_assert(m_VariableLiveRanges->empty() || m_VariableLiveRanges->back().m_EndEmitLocation.Valid()); - if (!m_VariableLiveRanges->empty() && - m_VariableLiveRanges->back().m_EndEmitLocation.IsPreviousIns(emit) && + if (!m_VariableLiveRanges->empty() && m_VariableLiveRanges->back().m_EndEmitLocation.IsPreviousIns(emit) && siVarLoc::Equals(&varLocation, &(m_VariableLiveRanges->back().m_VarLocation))) { // The variable is being born at the exactly same place just one assembly instructtion. @@ -11480,7 +11479,7 @@ void CodeGenInterface::VariableLiveKeeper::VariableLiveDescriptor::startLiveRang m_VariableLiveRanges->emplace_back(varLocation, emitLocation(), emitLocation()); m_VariableLiveRanges->back().m_StartEmitLocation.CaptureLocation(emit); } - + #ifdef DEBUG if (!m_VariableLifeBarrier->hasLiveRangesToDump()) { diff --git a/src/coreclr/src/jit/emit.cpp b/src/coreclr/src/jit/emit.cpp index a95e70c76c7036..9770adda993395 100644 --- a/src/coreclr/src/jit/emit.cpp +++ b/src/coreclr/src/jit/emit.cpp @@ -69,7 +69,7 @@ UNATIVE_OFFSET emitLocation::GetFuncletPrologOffset(emitter* emit) const bool emitLocation::IsPreviousIns(const emitter* emit) const { assert(Valid()); - bool sameGroup = ig == emit->emitCurIG; + bool sameGroup = ig == emit->emitCurIG; bool sameInsNum = emitGetInsNumFromCodePos(codePos) == emitGetInsNumFromCodePos(emit->emitCurOffset()) - 1; return sameGroup && sameInsNum; } From 15ecc0b1325897726de09a354246e25a787570fa Mon Sep 17 00:00:00 2001 From: Brian Bohe Date: Fri, 21 Feb 2020 14:56:32 -0800 Subject: [PATCH 4/5] Adding comments --- src/coreclr/src/jit/emit.cpp | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/src/coreclr/src/jit/emit.cpp b/src/coreclr/src/jit/emit.cpp index 9770adda993395..0fc99a69749139 100644 --- a/src/coreclr/src/jit/emit.cpp +++ b/src/coreclr/src/jit/emit.cpp @@ -65,7 +65,13 @@ UNATIVE_OFFSET emitLocation::GetFuncletPrologOffset(emitter* emit) const return emit->emitCurIGsize; } -// Returns true if the emitter is on the next instruction of the same group than this emitLocation +//------------------------------------------------------------------------ +// IsPreviousIns: Returns true if the emitter is on the next instruction +// of the same group than this emitLocation. +// +// Arguments: +// emit - an emitter* instance +// bool emitLocation::IsPreviousIns(const emitter* emit) const { assert(Valid()); From 5c4f17d0a1ddcf441222809b3f51735825eb4894 Mon Sep 17 00:00:00 2001 From: Brian Bohe Date: Mon, 24 Feb 2020 10:15:17 -0800 Subject: [PATCH 5/5] =?UTF-8?q?Renaming=20functions=20and=20updating=20com?= =?UTF-8?q?ments=C2=A0?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- src/coreclr/src/jit/codegencommon.cpp | 6 +++--- src/coreclr/src/jit/emit.cpp | 12 ++++++------ src/coreclr/src/jit/emit.h | 2 +- 3 files changed, 10 insertions(+), 10 deletions(-) diff --git a/src/coreclr/src/jit/codegencommon.cpp b/src/coreclr/src/jit/codegencommon.cpp index 703b8e699cf7d8..ac143352f91a31 100644 --- a/src/coreclr/src/jit/codegencommon.cpp +++ b/src/coreclr/src/jit/codegencommon.cpp @@ -11466,11 +11466,11 @@ void CodeGenInterface::VariableLiveKeeper::VariableLiveDescriptor::startLiveRang // Is the first "VariableLiveRange" or the previous one has been closed so its "m_EndEmitLocation" is valid noway_assert(m_VariableLiveRanges->empty() || m_VariableLiveRanges->back().m_EndEmitLocation.Valid()); - if (!m_VariableLiveRanges->empty() && m_VariableLiveRanges->back().m_EndEmitLocation.IsPreviousIns(emit) && + if (!m_VariableLiveRanges->empty() && m_VariableLiveRanges->back().m_EndEmitLocation.IsPreviousInsNum(emit) && siVarLoc::Equals(&varLocation, &(m_VariableLiveRanges->back().m_VarLocation))) { - // The variable is being born at the exactly same place just one assembly instructtion. - // We assume it never died. + // The variable is being born just after the instruction at which it died. + // In this case, i.e. an update of the variable's value, we coalesce the live ranges. m_VariableLiveRanges->back().m_EndEmitLocation.Init(); } else diff --git a/src/coreclr/src/jit/emit.cpp b/src/coreclr/src/jit/emit.cpp index 0fc99a69749139..65e688621afe38 100644 --- a/src/coreclr/src/jit/emit.cpp +++ b/src/coreclr/src/jit/emit.cpp @@ -66,18 +66,18 @@ UNATIVE_OFFSET emitLocation::GetFuncletPrologOffset(emitter* emit) const } //------------------------------------------------------------------------ -// IsPreviousIns: Returns true if the emitter is on the next instruction -// of the same group than this emitLocation. +// 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::IsPreviousIns(const emitter* emit) const +bool emitLocation::IsPreviousInsNum(const emitter* emit) const { assert(Valid()); - bool sameGroup = ig == emit->emitCurIG; - bool sameInsNum = emitGetInsNumFromCodePos(codePos) == emitGetInsNumFromCodePos(emit->emitCurOffset()) - 1; - return sameGroup && sameInsNum; + bool isSameGroup = (ig == emit->emitCurIG); + bool isSameInsNum = (emitGetInsNumFromCodePos(codePos) == emitGetInsNumFromCodePos(emit->emitCurOffset()) - 1); + return isSameGroup && isSameInsNum; } #ifdef DEBUG diff --git a/src/coreclr/src/jit/emit.h b/src/coreclr/src/jit/emit.h index 89d09e36949e62..622d6d621ef5b5 100644 --- a/src/coreclr/src/jit/emit.h +++ b/src/coreclr/src/jit/emit.h @@ -191,7 +191,7 @@ class emitLocation UNATIVE_OFFSET GetFuncletPrologOffset(emitter* emit) const; - bool emitLocation::IsPreviousIns(const emitter* emit) const; + bool emitLocation::IsPreviousInsNum(const emitter* emit) const; #ifdef DEBUG void Print() const;