Skip to content

Commit

Permalink
JIT: Simplify formatting strings for various "reason" fields (#98434)
Browse files Browse the repository at this point in the history 
Add a `Compiler::printfAlloc` that makes it simpler to create more
insightful strings to be stored in `LclVarDsc` and other places.
  • Loading branch information
@jakobbotsch
jakobbotsch authored Feb 15, 2024
1 parent 96e5e6e commit 988958f
Show file tree
Hide file tree
Showing 7 changed files with 58 additions and 62 deletions.
25 changes: 25 additions & 0 deletions src/coreclr/jit/compiler.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10615,6 +10615,31 @@ const char* Compiler::devirtualizationDetailToString(CORINFO_DEVIRTUALIZATION_DE
return "undefined";
}
}

//------------------------------------------------------------------------------
// printfAlloc: printf a string and allocate the result in CMK_DebugOnly
// memory.
//
// Arguments:
// format - Format string
//
// Returns:
// Allocated string.
//
const char* Compiler::printfAlloc(const char* format, ...)
{
char str[512];
va_list args;
va_start(args, format);
int result = vsprintf_s(str, ArrLen(str), format, args);
va_end(args);
assert((result >= 0) && ((unsigned)result < ArrLen(str)));

char* resultStr = new (this, CMK_DebugOnly) char[result + 1];
memcpy(resultStr, str, (unsigned)result + 1);
return resultStr;
}

#endif // defined(DEBUG)

#if TRACK_ENREG_STATS
Expand Down
2 changes: 2 additions & 0 deletions src/coreclr/jit/compiler.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9988,6 +9988,8 @@ XXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXX

const char* devirtualizationDetailToString(CORINFO_DEVIRTUALIZATION_DETAIL detail);

const char* printfAlloc(const char* format, ...);

#endif // DEBUG

// clang-format off
Expand Down
17 changes: 4 additions & 13 deletions src/coreclr/jit/decomposelongs.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2171,20 +2171,11 @@ void DecomposeLongs::TryPromoteLongVar(unsigned lclNum)
// Grab the temp for the field local.
CLANG_FORMAT_COMMENT_ANCHOR;

#ifdef DEBUG
char buf[200];
sprintf_s(buf, sizeof(buf), "%s V%02u.%s (fldOffset=0x%x)", "field", lclNum, index == 0 ? "lo" : "hi",
index * 4);

// We need to copy 'buf' as lvaGrabTemp() below caches a copy to its argument.
size_t len = strlen(buf) + 1;
char* bufp = m_compiler->getAllocator(CMK_DebugOnly).allocate<char>(len);
strcpy_s(bufp, len, buf);
#endif

// Lifetime of field locals might span multiple BBs, so they are long lifetime temps.
unsigned fieldLclNum = m_compiler->lvaGrabTemp(false DEBUGARG(bufp));
varDsc = m_compiler->lvaGetDesc(lclNum);
unsigned fieldLclNum = m_compiler->lvaGrabTemp(
false DEBUGARG(m_compiler->printfAlloc("%s V%02u.%s (fldOffset=0x%x)", "field", lclNum,
index == 0 ? "lo" : "hi", index * 4)));
varDsc = m_compiler->lvaGetDesc(lclNum);

LclVarDsc* fieldVarDsc = m_compiler->lvaGetDesc(fieldLclNum);
fieldVarDsc->lvType = TYP_INT;
Expand Down
8 changes: 2 additions & 6 deletions src/coreclr/jit/lclvars.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2357,16 +2357,12 @@ void Compiler::StructPromotionHelper::PromoteStructVar(unsigned lclNum)
// Now grab the temp for the field local.

#ifdef DEBUG
char buf[200];
char fieldNameBuffer[128];
const char* fieldName =
compiler->eeGetFieldName(pFieldInfo->diagFldHnd, false, fieldNameBuffer, sizeof(fieldNameBuffer));
sprintf_s(buf, sizeof(buf), "field V%02u.%s (fldOffset=0x%x)", lclNum, fieldName, pFieldInfo->fldOffset);

// We need to copy 'buf' as lvaGrabTemp() below caches a copy to its argument.
size_t len = strlen(buf) + 1;
char* bufp = compiler->getAllocator(CMK_DebugOnly).allocate<char>(len);
strcpy_s(bufp, len, buf);
const char* bufp =
compiler->printfAlloc("field V%02u.%s (fldOffset=0x%x)", lclNum, fieldName, pFieldInfo->fldOffset);

if (index > 0)
{
Expand Down
9 changes: 2 additions & 7 deletions src/coreclr/jit/promotion.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1219,13 +1219,8 @@ class LocalsUseVisitor : public GenTreeVisitor<LocalsUseVisitor>
for (Replacement& rep : reps)
{
#ifdef DEBUG
char buf[32];
sprintf_s(buf, sizeof(buf), "V%02u.[%03u..%03u)", agg->LclNum, rep.Offset,
rep.Offset + genTypeSize(rep.AccessType));
size_t len = strlen(buf) + 1;
char* bufp = new (m_compiler, CMK_DebugOnly) char[len];
strcpy_s(bufp, len, buf);
rep.Description = bufp;
rep.Description = m_compiler->printfAlloc("V%02u.[%03u..%03u)", agg->LclNum, rep.Offset,
rep.Offset + genTypeSize(rep.AccessType));
#endif

rep.LclNum = m_compiler->lvaGrabTemp(false DEBUGARG(rep.Description));
Expand Down
42 changes: 18 additions & 24 deletions src/coreclr/jit/rangecheck.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,7 @@ bool RangeCheck::BetweenBounds(Range& range, GenTree* upper, int arrSize)
#ifdef DEBUG
if (m_pCompiler->verbose)
{
printf("%s BetweenBounds <%d, ", range.ToString(m_pCompiler->getAllocatorDebugOnly()), 0);
printf("%s BetweenBounds <%d, ", range.ToString(m_pCompiler), 0);
Compiler::printTreeID(upper);
printf(">\n");
}
Expand Down Expand Up @@ -359,7 +359,7 @@ void RangeCheck::OptimizeRangeCheck(BasicBlock* block, Statement* stmt, GenTree*
return;
}

JITDUMP("Range value %s\n", range.ToString(m_pCompiler->getAllocatorDebugOnly()));
JITDUMP("Range value %s\n", range.ToString(m_pCompiler));
m_pSearchPath->RemoveAll();
Widen(block, treeIndex, &range);

Expand Down Expand Up @@ -917,7 +917,7 @@ void RangeCheck::MergeEdgeAssertions(ValueNum normalLclVN, ASSERT_VALARG_TP asse
break;
}
JITDUMP("The range after edge merging:");
JITDUMP(pRange->ToString(m_pCompiler->getAllocatorDebugOnly()));
JITDUMP(pRange->ToString(m_pCompiler));
JITDUMP("\n");
}
}
Expand Down Expand Up @@ -1007,7 +1007,7 @@ Range RangeCheck::ComputeRangeForBinOp(BasicBlock* block, GenTreeOp* binop, bool
if (icon >= 0)
{
Range range(Limit(Limit::keConstant, 0), Limit(Limit::keConstant, icon));
JITDUMP("Limit range to %s\n", range.ToString(m_pCompiler->getAllocatorDebugOnly()));
JITDUMP("Limit range to %s\n", range.ToString(m_pCompiler));
return range;
}
// Generalized range computation not implemented for these operators
Expand Down Expand Up @@ -1068,32 +1068,28 @@ Range RangeCheck::ComputeRangeForBinOp(BasicBlock* block, GenTreeOp* binop, bool
if (binop->OperIs(GT_ADD))
{
r = RangeOps::Add(op1Range, op2Range);
JITDUMP("BinOp add ranges %s %s = %s\n", op1Range.ToString(m_pCompiler->getAllocatorDebugOnly()),
op2Range.ToString(m_pCompiler->getAllocatorDebugOnly()),
r.ToString(m_pCompiler->getAllocatorDebugOnly()));
JITDUMP("BinOp add ranges %s %s = %s\n", op1Range.ToString(m_pCompiler), op2Range.ToString(m_pCompiler),
r.ToString(m_pCompiler));
}
else if (binop->OperIs(GT_MUL))
{
r = RangeOps::Multiply(op1Range, op2Range);
JITDUMP("BinOp multiply ranges %s %s = %s\n", op1Range.ToString(m_pCompiler->getAllocatorDebugOnly()),
op2Range.ToString(m_pCompiler->getAllocatorDebugOnly()),
r.ToString(m_pCompiler->getAllocatorDebugOnly()));
JITDUMP("BinOp multiply ranges %s %s = %s\n", op1Range.ToString(m_pCompiler), op2Range.ToString(m_pCompiler),
r.ToString(m_pCompiler));
}
else if (binop->OperIs(GT_LSH))
{
// help the next step a bit, convert the LSH rhs to a multiply
Range convertedOp2Range = RangeOps::ConvertShiftToMultiply(op2Range);
r = RangeOps::Multiply(op1Range, convertedOp2Range);
JITDUMP("BinOp multiply ranges %s %s = %s\n", op1Range.ToString(m_pCompiler->getAllocatorDebugOnly()),
convertedOp2Range.ToString(m_pCompiler->getAllocatorDebugOnly()),
r.ToString(m_pCompiler->getAllocatorDebugOnly()));
JITDUMP("BinOp multiply ranges %s %s = %s\n", op1Range.ToString(m_pCompiler),
convertedOp2Range.ToString(m_pCompiler), r.ToString(m_pCompiler));
}
else if (binop->OperIs(GT_RSH))
{
r = RangeOps::ShiftRight(op1Range, op2Range);
JITDUMP("Right shift range: %s >> %s = %s\n", op1Range.ToString(m_pCompiler->getAllocatorDebugOnly()),
op2Range.ToString(m_pCompiler->getAllocatorDebugOnly()),
r.ToString(m_pCompiler->getAllocatorDebugOnly()));
JITDUMP("Right shift range: %s >> %s = %s\n", op1Range.ToString(m_pCompiler), op2Range.ToString(m_pCompiler),
r.ToString(m_pCompiler));
}
return r;
}
Expand Down Expand Up @@ -1265,9 +1261,8 @@ bool RangeCheck::DoesBinOpOverflow(BasicBlock* block, GenTreeOp* binop)
return true;
}

JITDUMP("Checking bin op overflow %s %s %s\n", GenTree::OpName(binop->OperGet()),
op1Range->ToString(m_pCompiler->getAllocatorDebugOnly()),
op2Range->ToString(m_pCompiler->getAllocatorDebugOnly()));
JITDUMP("Checking bin op overflow %s %s %s\n", GenTree::OpName(binop->OperGet()), op1Range->ToString(m_pCompiler),
op2Range->ToString(m_pCompiler));

if (binop->OperIs(GT_ADD))
{
Expand Down Expand Up @@ -1482,16 +1477,15 @@ Range RangeCheck::ComputeRange(BasicBlock* block, GenTree* expr, bool monIncreas
assert(!argRange.LowerLimit().IsUndef());
assert(!argRange.UpperLimit().IsUndef());
MergeAssertion(block, use.GetNode(), &argRange DEBUGARG(indent + 1));
JITDUMP("Merging ranges %s %s:", range.ToString(m_pCompiler->getAllocatorDebugOnly()),
argRange.ToString(m_pCompiler->getAllocatorDebugOnly()));
JITDUMP("Merging ranges %s %s:", range.ToString(m_pCompiler), argRange.ToString(m_pCompiler));
range = RangeOps::Merge(range, argRange, monIncreasing);
JITDUMP("%s\n", range.ToString(m_pCompiler->getAllocatorDebugOnly()));
JITDUMP("%s\n", range.ToString(m_pCompiler));
}
}
else if (varTypeIsSmall(expr))
{
range = GetRangeFromType(expr->TypeGet());
JITDUMP("%s\n", range.ToString(m_pCompiler->getAllocatorDebugOnly()));
JITDUMP("%s\n", range.ToString(m_pCompiler));
}
else if (expr->OperIs(GT_COMMA))
{
Expand Down Expand Up @@ -1546,7 +1540,7 @@ Range RangeCheck::GetRange(BasicBlock* block, GenTree* expr, bool monIncreasing
{
Indent(indent);
JITDUMP(" %s Range [%06d] => %s\n", (pRange == nullptr) ? "Computed" : "Cached", Compiler::dspTreeID(expr),
range.ToString(m_pCompiler->getAllocatorDebugOnly()));
range.ToString(m_pCompiler));
Indent(indent);
JITDUMP("}\n", expr);
}
Expand Down
17 changes: 5 additions & 12 deletions src/coreclr/jit/rangecheck.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -209,10 +209,8 @@ struct Limit
return false;
}
#ifdef DEBUG
const char* ToString(CompAllocator alloc)
const char* ToString(Compiler* comp)
{
unsigned size = 64;
char* buf = alloc.allocate<char>(size);
switch (type)
{
case keUndef:
Expand All @@ -225,12 +223,10 @@ struct Limit
return "Dependent";

case keBinOpArray:
sprintf_s(buf, size, FMT_VN " + %d", vn, cns);
return buf;
return comp->printfAlloc(FMT_VN " + %d", vn, cns);

case keConstant:
sprintf_s(buf, size, "%d", cns);
return buf;
return comp->printfAlloc("%d", cns);
}
unreached();
}
Expand Down Expand Up @@ -265,12 +261,9 @@ struct Range
}

#ifdef DEBUG
char* ToString(CompAllocator alloc)
const char* ToString(Compiler* comp)
{
size_t size = 64;
char* buf = alloc.allocate<char>(size);
sprintf_s(buf, size, "<%s, %s>", lLimit.ToString(alloc), uLimit.ToString(alloc));
return buf;
return comp->printfAlloc("<%s, %s>", lLimit.ToString(comp), uLimit.ToString(comp));
}
#endif
};
Expand Down

0 comments on commit 988958f

Please sign in to comment.