Skip to content

JIT: fix loop cloning type assumptions #128729

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.

Sign up for GitHub

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

Merged
AndyAyersMS merged 10 commits into from
Jun 4, 2026
Merged

JIT: fix loop cloning type assumptions #128729

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
ed35ce3
JIT: fix loop cloning type assumptions
AndyAyersMS May 28, 2026
fa2125b
Fix formatting
AndyAyersMS May 28, 2026
2b606dc
Use lvaTable types for Var and array base nodes
AndyAyersMS May 29, 2026
5af92ae
Only capture arrType on first dimension
AndyAyersMS May 29, 2026
cb53576
Revert genActualTypeIsInt checks to use lvaGetDesc
AndyAyersMS May 29, 2026
ec2a8e4
Fix formatting
AndyAyersMS May 29, 2026
b917c79
Use lvaTable type for IndirOfLocal too
AndyAyersMS May 29, 2026
070220e
Set arrType in MD array GetArrIndexForDim
AndyAyersMS Jun 2, 2026
515bd4a
Merge remote-tracking branch 'upstream/main' into fix-loop-cloning-types
AndyAyersMS Jun 2, 2026
03d7ec1
Create correctly typed null constants in loop cloning
AndyAyersMS Jun 2, 2026
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
5 changes: 3 additions & 2 deletions src/coreclr/jit/flowgraph.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6952,8 +6952,9 @@ bool NaturalLoopIterInfo::ArrLenLimit(Compiler* comp, ArrIndex* index)
// Check if we have a.length or a[i][j].length
if (limit->AsArrLen()->ArrRef()->OperIs(GT_LCL_VAR))
{
index->arrLcl = limit->AsArrLen()->ArrRef()->AsLclVarCommon()->GetLclNum();
index->rank = 0;
index->arrLcl = limit->AsArrLen()->ArrRef()->AsLclVarCommon()->GetLclNum();
index->arrType = limit->AsArrLen()->ArrRef()->TypeGet();
index->rank = 0;
return true;
}
// We have a[i].length, extract a[i] pattern.
Expand Down
45 changes: 26 additions & 19 deletions src/coreclr/jit/loopcloning.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -171,12 +171,12 @@ GenTree* LC_Ident::ToGenTree(Compiler* comp, BasicBlock* bb)
case SpanAccess:
return spanAccess.ToGenTree(comp);
case Null:
return comp->gtNewIconNode(0, TYP_REF);
return comp->gtNewIconNode(0, lclType);
case ClassHandle:
return comp->gtNewIconHandleNode((size_t)clsHnd, GTF_ICON_CLASS_HDL);
case IndirOfLocal:
{
GenTree* addr = comp->gtNewLclvNode(lclNum, TYP_REF);
GenTree* addr = comp->gtNewLclvNode(lclNum, comp->lvaTable[lclNum].lvType);
if (indirOffs == 0)
{
return comp->gtNewMethodTableLookup(addr);
Expand Down Expand Up @@ -247,6 +247,7 @@ GenTree* LC_Condition::ToGenTree(Compiler* comp, BasicBlock* bb, bool invert)
{
GenTree* op1Tree = op1.ToGenTree(comp, bb);
GenTree* op2Tree = op2.ToGenTree(comp, bb);

assert(genTypeSize(genActualType(op1Tree->TypeGet())) == genTypeSize(genActualType(op2Tree->TypeGet())));

GenTree* result = comp->gtNewOperNode(invert ? GenTree::ReverseRelop(oper) : oper, TYP_INT, op1Tree, op2Tree);
Expand Down Expand Up @@ -991,8 +992,8 @@ void LC_ArrayDeref::DeriveLevelConditions(JitExpandArrayStack<JitExpandArrayStac
if (level == 0)
{
// For level 0, just push (a != null).
(*conds)[level]->Push(
LC_Condition(GT_NE, LC_Expr(LC_Ident::CreateVar(Lcl())), LC_Expr(LC_Ident::CreateNull())));
(*conds)[level]->Push(LC_Condition(GT_NE, LC_Expr(LC_Ident::CreateVar(Lcl(), array.arrIndex->arrType)),
LC_Expr(LC_Ident::CreateNull(array.arrIndex->arrType))));
}
else
{
Expand All @@ -1002,7 +1003,7 @@ void LC_ArrayDeref::DeriveLevelConditions(JitExpandArrayStack<JitExpandArrayStac
LC_Array arrLen = array;
arrLen.oper = LC_Array::ArrLen;
arrLen.dim = level - 1;
(*conds)[level * 2 - 1]->Push(LC_Condition(GT_LT, LC_Expr(LC_Ident::CreateVar(Lcl())),
(*conds)[level * 2 - 1]->Push(LC_Condition(GT_LT, LC_Expr(LC_Ident::CreateVar(Lcl(), TYP_INT)),
LC_Expr(LC_Ident::CreateArrAccess(arrLen)), /*unsigned*/ true));

// Push condition (a[i] != null)
Expand Down Expand Up @@ -1138,10 +1139,11 @@ bool Compiler::optDeriveLoopCloningConditions(FlowGraphNaturalLoop* loop, LoopCl

case LcOptInfo::LcTypeTest:
{
LcTypeTestOptInfo* ttInfo = optInfo->AsLcTypeTestOptInfo();
LC_Ident objDeref = LC_Ident::CreateIndirOfLocal(ttInfo->lclNum, 0);
LC_Ident methodTable = LC_Ident::CreateClassHandle(ttInfo->clsHnd);
LC_Condition cond(GT_EQ, LC_Expr(objDeref), LC_Expr(methodTable));
LcTypeTestOptInfo* ttInfo = optInfo->AsLcTypeTestOptInfo();
LC_Ident objDeref =
LC_Ident::CreateIndirOfLocal(ttInfo->lclNum, 0, ttInfo->methodTableIndir->Addr()->TypeGet());
LC_Ident methodTable = LC_Ident::CreateClassHandle(ttInfo->clsHnd);
LC_Condition cond(GT_EQ, LC_Expr(objDeref), LC_Expr(methodTable));
context->EnsureObjDerefs(loop->GetIndex())->Push(objDeref);
context->EnsureConditions(loop->GetIndex())->Push(cond);
break;
Expand All @@ -1151,7 +1153,8 @@ bool Compiler::optDeriveLoopCloningConditions(FlowGraphNaturalLoop* loop, LoopCl
{
LcMethodAddrTestOptInfo* test = optInfo->AsLcMethodAddrTestOptInfo();
LC_Ident objDeref =
LC_Ident::CreateIndirOfLocal(test->delegateLclNum, eeGetEEInfo()->offsetOfDelegateFirstTarget);
LC_Ident::CreateIndirOfLocal(test->delegateLclNum, eeGetEEInfo()->offsetOfDelegateFirstTarget,
test->delegateAddressIndir->Addr()->TypeGet());
LC_Ident methAddr;
if (test->isSlot)
{
Expand Down Expand Up @@ -1273,7 +1276,7 @@ bool Compiler::optDeriveLoopCloningConditions(FlowGraphNaturalLoop* loop, LoopCl
JITDUMP("> NeedsZeroTripGuard: iter var V%02u not compatible with TYP_INT\n", initLcl);
return false;
}
initIdent = LC_Ident::CreateVar(initLcl);
initIdent = LC_Ident::CreateVar(initLcl, iterInfo->Iterator()->TypeGet());
}

LC_Ident limitIdent;
Expand All @@ -1295,7 +1298,7 @@ bool Compiler::optDeriveLoopCloningConditions(FlowGraphNaturalLoop* loop, LoopCl
JITDUMP("> NeedsZeroTripGuard: limit var V%02u not compatible with TYP_INT\n", limitLcl);
return false;
}
limitIdent = LC_Ident::CreateVar(limitLcl);
limitIdent = LC_Ident::CreateVar(limitLcl, iterInfo->Limit()->TypeGet());
}
else if (iterInfo->HasArrayLengthLimit)
{
Expand Down Expand Up @@ -1348,12 +1351,13 @@ bool Compiler::optDeriveLoopCloningConditions(FlowGraphNaturalLoop* loop, LoopCl
LC_Condition geZero;
if (isIncreasingLoop)
{
geZero = LC_Condition(GT_GE, LC_Expr(LC_Ident::CreateVar(initLcl)), LC_Expr(LC_Ident::CreateConst(0u)));
geZero = LC_Condition(GT_GE, LC_Expr(LC_Ident::CreateVar(initLcl, iterInfo->Iterator()->TypeGet())),
LC_Expr(LC_Ident::CreateConst(0u)));
}
else
{
// For decreasing loop, the init value needs to be checked against the array length
ident = LC_Ident::CreateVar(initLcl);
ident = LC_Ident::CreateVar(initLcl, iterInfo->Iterator()->TypeGet());
geZero = LC_Condition(GT_GE, LC_Expr(ident), LC_Expr(LC_Ident::CreateConst(0u)));
}
context->EnsureConditions(loop->GetIndex())->Push(geZero);
Expand Down Expand Up @@ -1388,12 +1392,13 @@ bool Compiler::optDeriveLoopCloningConditions(FlowGraphNaturalLoop* loop, LoopCl
if (isIncreasingLoop)
{
// For increasing loop, thelimit value needs to be checked against the array length
ident = LC_Ident::CreateVar(limitLcl);
ident = LC_Ident::CreateVar(limitLcl, iterInfo->Limit()->TypeGet());
geZero = LC_Condition(GT_GE, LC_Expr(ident), LC_Expr(LC_Ident::CreateConst(0u)));
}
else
{
geZero = LC_Condition(GT_GE, LC_Expr(LC_Ident::CreateVar(limitLcl)), LC_Expr(LC_Ident::CreateConst(0u)));
geZero = LC_Condition(GT_GE, LC_Expr(LC_Ident::CreateVar(limitLcl, iterInfo->Limit()->TypeGet())),
LC_Expr(LC_Ident::CreateConst(0u)));
}

context->EnsureConditions(loop->GetIndex())->Push(geZero);
Expand Down Expand Up @@ -1702,8 +1707,9 @@ bool Compiler::optComputeDerefConditions(FlowGraphNaturalLoop* loop, LoopCloneCo
// ObjDeref array has indir(lcl), we want lcl.
//
LC_Ident& mtIndirIdent = (*objDeref)[i];
LC_Ident ident = LC_Ident::CreateVar(mtIndirIdent.LclNum());
(*levelCond)[0]->Push(LC_Condition(GT_NE, LC_Expr(ident), LC_Expr(LC_Ident::CreateNull())));
LC_Ident ident = LC_Ident::CreateVar(mtIndirIdent.LclNum(), mtIndirIdent.lclType);
(*levelCond)[0]->Push(
LC_Condition(GT_NE, LC_Expr(ident), LC_Expr(LC_Ident::CreateNull(mtIndirIdent.lclType))));
}
}

Expand Down Expand Up @@ -2361,7 +2367,8 @@ bool Compiler::optExtractArrIndex(GenTree* tree, ArrIndex* result, unsigned lhsN

if (lhsNum == BAD_VAR_NUM)
{
result->arrLcl = arrLcl;
result->arrLcl = arrLcl;
result->arrType = arrBndsChk->GetArrayLength()->gtGetOp1()->TypeGet();
}
result->indLcls.Push(indLcl);
result->bndsChks.Push(tree);
Expand Down
29 changes: 20 additions & 9 deletions src/coreclr/jit/loopcloning.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -191,13 +191,15 @@ class Compiler;
struct ArrIndex
{
unsigned arrLcl; // The array base local num
var_types arrType; // The array base type at extraction time
JitExpandArrayStack<unsigned> indLcls; // The indices local nums
Comment thread
AndyAyersMS marked this conversation as resolved.
JitExpandArrayStack<GenTree*> bndsChks; // The bounds checks nodes along each dimension.
unsigned rank; // Rank of the array
BasicBlock* useBlock; // Block where the [] occurs

ArrIndex(CompAllocator alloc)
: arrLcl(BAD_VAR_NUM)
, arrType(TYP_UNDEF)
, indLcls(alloc)
, bndsChks(alloc)
, rank(0)
Expand Down Expand Up @@ -312,7 +314,8 @@ struct LcMdArrayOptInfo : public LcOptInfo
{
index->indLcls.Push(arrElem->gtArrInds[i]->AsLclVarCommon()->GetLclNum());
}
index->arrLcl = arrElem->gtArrObj->AsLclVarCommon()->GetLclNum();
index->arrLcl = arrElem->gtArrObj->AsLclVarCommon()->GetLclNum();
index->arrType = arrElem->gtArrObj->TypeGet();
}
return index;
}
Expand Down Expand Up @@ -484,7 +487,8 @@ struct LC_Array
assert(type != Invalid && that.type != Invalid);

// Types match and the array base matches.
if (type != that.type || arrIndex->arrLcl != that.arrIndex->arrLcl || oper != that.oper)
if (type != that.type || arrIndex->arrLcl != that.arrIndex->arrLcl ||
arrIndex->arrType != that.arrIndex->arrType || oper != that.oper)
{
return false;
}
Expand Down Expand Up @@ -592,15 +596,18 @@ struct LC_Ident

LC_Ident(IdentType type)
: type(type)
, lclType(TYP_UNDEF)
{
}

public:
// The type of this object
IdentType type;
var_types lclType;

LC_Ident()
: type(Invalid)
, lclType(TYP_UNDEF)
{
}

Expand All @@ -619,9 +626,9 @@ struct LC_Ident
case ClassHandle:
return (clsHnd == that.clsHnd);
case Var:
return (lclNum == that.lclNum);
return (lclNum == that.lclNum) && (lclType == that.lclType);
case IndirOfLocal:
return (lclNum == that.lclNum) && (indirOffs == that.indirOffs);
return (lclNum == that.lclNum) && (indirOffs == that.indirOffs) && (lclType == that.lclType);
case ArrAccess:
return (arrAccess == that.arrAccess);
case SpanAccess:
Expand Down Expand Up @@ -693,18 +700,20 @@ struct LC_Ident
// Convert this symbolic representation into a tree node.
GenTree* ToGenTree(Compiler* comp, BasicBlock* bb);

static LC_Ident CreateVar(unsigned lclNum)
static LC_Ident CreateVar(unsigned lclNum, var_types lclType)
{
LC_Ident id(Var);
id.lclNum = lclNum;
id.lclNum = lclNum;
id.lclType = lclType;
return id;
}

static LC_Ident CreateIndirOfLocal(unsigned lclNum, unsigned offs)
static LC_Ident CreateIndirOfLocal(unsigned lclNum, unsigned offs, var_types lclType)
{
LC_Ident id(IndirOfLocal);
id.lclNum = lclNum;
id.indirOffs = offs;
id.lclType = lclType;
return id;
}

Expand All @@ -729,9 +738,11 @@ struct LC_Ident
return id;
}

static LC_Ident CreateNull()
static LC_Ident CreateNull(var_types nullType = TYP_REF)
{
return LC_Ident(Null);
LC_Ident ident(Null);
ident.lclType = nullType;
return ident;
}

static LC_Ident CreateClassHandle(CORINFO_CLASS_HANDLE clsHnd)
Expand Down
Loading