Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Re-enable improvements to debug emission for optimized code #38894

Closed
wants to merge 2 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
27 changes: 23 additions & 4 deletions src/coreclr/src/jit/codegencommon.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -12314,9 +12314,28 @@ 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() &&
siVarLoc::Equals(&varLocation, &(m_VariableLiveRanges->back().m_VarLocation)) &&
m_VariableLiveRanges->back().m_EndEmitLocation.IsPreviousInsNum(emit))
{
JITDUMP("Extending debug range...\n");

// 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
{
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());
m_VariableLiveRanges->back().m_StartEmitLocation.CaptureLocation(emit);
}

#ifdef DEBUG
if (!m_VariableLifeBarrier->hasLiveRangesToDump())
Expand Down Expand Up @@ -12707,7 +12726,7 @@ void CodeGenInterface::VariableLiveKeeper::siEndAllVariableLiveRange(VARSET_VALA
// Notes:
// This overload exists for the case we are jitting code compiled in
// debug mode. When that happen we don't have variable liveness info
// as "BaiscBlock::bbLiveIn" or "BaiscBlock::bbLiveOut" and there is no
// as "BasicBlock::bbLiveIn" or "BasicBlock::bbLiveOut" and there is no
// tracked variable.
//
void CodeGenInterface::VariableLiveKeeper::siEndAllVariableLiveRange()
Expand Down
4 changes: 2 additions & 2 deletions src/coreclr/src/jit/codegeninterface.h
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
17 changes: 15 additions & 2 deletions src/coreclr/src/jit/ee_il_dll.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -835,8 +835,21 @@ void Compiler::eeDispVar(ICorDebugInfo::NativeVarInfo* var)
// Same parameters as ICorStaticInfo::setVars().
void Compiler::eeDispVars(CORINFO_METHOD_HANDLE ftn, ULONG32 cVars, ICorDebugInfo::NativeVarInfo* vars)
{
printf("*************** Variable debug info\n");
printf("%d live ranges\n", cVars);
// Estimate number of unique vars with debug info
//
ALLVARSET_TP uniqueVars(AllVarSetOps::MakeEmpty(this));
for (unsigned i = 0; i < cVars; i++)
{
// ignore "special vars" and out of bounds vars
if ((((int)vars[i].varNumber) >= 0) && (vars[i].varNumber < lclMAX_ALLSET_TRACKED))
{
AllVarSetOps::AddElemD(this, uniqueVars, vars[i].varNumber);
}
}

printf("; Variable debug info: %d live ranges, %d vars for method %s\n", cVars,
AllVarSetOps::Count(this, uniqueVars), info.compFullName);

for (unsigned i = 0; i < cVars; i++)
{
eeDispVar(&vars[i]);
Expand Down
27 changes: 27 additions & 0 deletions src/coreclr/src/jit/emit.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,33 @@ UNATIVE_OFFSET emitLocation::GetFuncletPrologOffset(emitter* emit) const
return emit->emitCurIGsize;
}

//------------------------------------------------------------------------
// IsPreviousInsNum: Returns true if the emitter is on the next instruction
// of the same group as this emitLocation, or the first instruction of
// an extension of this location's IG.
//
// Arguments:
// emit - an emitter* instance
//
bool emitLocation::IsPreviousInsNum(const emitter* emit) const
{
assert(Valid());

// Within the same IG?
if (ig == emit->emitCurIG)
{
return (emitGetInsNumFromCodePos(codePos) == emitGetInsNumFromCodePos(emit->emitCurOffset()) - 1);
}

// Spanning an IG boundary?
if (ig->igNext == emit->emitCurIG)
{
return (emitGetInsNumFromCodePos(codePos) == ig->igInsCnt) && (emit->emitCurIGinsCnt == 1);
}

return false;
}

#ifdef DEBUG
void emitLocation::Print(LONG compMethodID) const
{
Expand Down
4 changes: 3 additions & 1 deletion src/coreclr/src/jit/emit.h
Original file line number Diff line number Diff line change
Expand Up @@ -191,6 +191,8 @@ class emitLocation

UNATIVE_OFFSET GetFuncletPrologOffset(emitter* emit) const;

bool emitLocation::IsPreviousInsNum(const emitter* emit) const;

#ifdef DEBUG
void Print(LONG compMethodID) const;
#endif // DEBUG
Expand Down Expand Up @@ -2386,7 +2388,7 @@ inline unsigned emitGetInsOfsFromCodePos(unsigned codePos)
return (codePos >> 16);
}

inline unsigned emitter::emitCurOffset()
inline unsigned emitter::emitCurOffset() const
{
unsigned codePos = emitCurIGinsCnt + (emitCurIGsize << 16);

Expand Down
2 changes: 1 addition & 1 deletion src/coreclr/src/jit/emitpub.h
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ void emitFinishPrologEpilogGeneration();
/************************************************************************/

void* emitCurBlock();
unsigned emitCurOffset();
unsigned emitCurOffset() const;

UNATIVE_OFFSET emitCodeOffset(void* blockPtr, unsigned codeOffs);

Expand Down
9 changes: 7 additions & 2 deletions src/coreclr/src/jit/treelifeupdater.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -429,8 +429,13 @@ void TreeLifeUpdater<ForCodeGen>::UpdateLifeVar(GenTree* tree)
}

#ifdef USING_VARIABLE_LIVE_RANGE
// For each of the LclVarDsc that are reporting change, variable or fields
compiler->codeGen->getVariableLiveKeeper()->siStartOrCloseVariableLiveRanges(varDeltaSet, isBorn, isDying);
// For each of the LclVarDsc that are reporting change, variable or fields.
// If this is a spill, defer to the update done in genSpillVar.
if (!spill)
{
compiler->codeGen->getVariableLiveKeeper()->siStartOrCloseVariableLiveRanges(varDeltaSet, isBorn,
isDying);
}
#endif // USING_VARIABLE_LIVE_RANGE

#ifdef USING_SCOPE_INFO
Expand Down