Skip to content

Commit 2519072

Browse files
EgorBoEgorBo
committed
Cleanup
1 parent dd5b215 commit 2519072

File tree

2 files changed

+100
-65
lines changed

2 files changed

+100
-65
lines changed

‎src/coreclr/jit/loopcloning.cpp‎

Lines changed: 61 additions & 55 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,14 @@ XXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXX
2323
//
2424
void ArrIndex::Print(unsigned dim /* = -1 */)
2525
{
26-
printf("V%02d", arrLcl);
26+
// When we have only length, we deal with a single-dimensional array or span
27+
if (HasLength())
28+
{
29+
printf("Len: V%02d", GetLengthLcl());
30+
return;
31+
}
32+
33+
printf("ArrObj: V%02d", GetArrayObjLcl());
2734
for (unsigned i = 0; i < ((dim == (unsigned)-1) ? rank : dim); ++i)
2835
{
2936
printf("[V%02d]", indLcls.Get(i));
@@ -67,11 +74,18 @@ GenTree* LC_Array::ToGenTree(Compiler* comp, BasicBlock* bb)
6774
if (type == Jagged)
6875
{
6976
// Create a a[i][j][k].length type node.
70-
GenTree* arr = comp->gtNewLclvNode(arrIndex->arrLcl, comp->lvaTable[arrIndex->arrLcl].lvType);
71-
int rank = GetDimRank();
77+
int rank = GetDimRank();
78+
79+
if (arrIndex->HasLength())
80+
{
81+
assert(oper == ArrLen);
82+
assert(rank == 0);
83+
assert(arrIndex->HasLength());
84+
return comp->gtNewLclvNode(arrIndex->GetLengthLcl(), comp->lvaTable[arrIndex->GetLengthLcl()].lvType);
85+
}
7286

73-
// rank is always 0 for spans
74-
assert(!arrIndex->isSpan || (rank == 0));
87+
GenTree* arr =
88+
comp->gtNewLclvNode(arrIndex->GetArrayObjLcl(), comp->lvaTable[arrIndex->GetArrayObjLcl()].lvType);
7589

7690
for (int i = 0; i < rank; ++i)
7791
{
@@ -92,18 +106,7 @@ GenTree* LC_Array::ToGenTree(Compiler* comp, BasicBlock* bb)
92106
// If asked for arrlen invoke arr length operator.
93107
if (oper == ArrLen)
94108
{
95-
GenTree* arrLen;
96-
if (arrIndex->isSpan)
97-
{
98-
// For promoted spans, arr is already our length.
99-
assert(arr->OperIs(GT_LCL_VAR));
100-
assert(comp->lvaGetDesc(arr->AsLclVar()->GetLclNum())->IsSpanLength());
101-
arrLen = arr;
102-
}
103-
else
104-
{
105-
arrLen = comp->gtNewArrLen(TYP_INT, arr, OFFSETOF__CORINFO_Array__length, bb);
106-
}
109+
GenTree* arrLen = comp->gtNewArrLen(TYP_INT, arr, OFFSETOF__CORINFO_Array__length, bb);
107110

108111
// We already guaranteed (by a sequence of preceding checks) that the array length operator will not
109112
// throw an exception because we null checked the base array.
@@ -934,7 +937,7 @@ unsigned LC_ArrayDeref::Lcl()
934937
unsigned lvl = level;
935938
if (lvl == 0)
936939
{
937-
return array.arrIndex->arrLcl;
940+
return array.arrIndex->GetArrayObjLcl();
938941
}
939942
lvl--;
940943
return array.arrIndex->indLcls[lvl];
@@ -970,27 +973,17 @@ bool LC_ArrayDeref::HasChildren()
970973
//
971974
void LC_ArrayDeref::DeriveLevelConditions(JitExpandArrayStack<JitExpandArrayStack<LC_Condition>*>* conds)
972975
{
976+
assert(this->array.arrIndex->HasArrayObj());
977+
973978
if (level == 0)
974979
{
975-
if (this->array.arrIndex->isSpan)
976-
{
977-
// For promoted Spans we don't need the "array != null" check.
978-
// However, the current algorithm doesn't expect that this condition might not be
979-
// needed, we insert a dummy always-true condition.
980-
//
981-
(*conds)[level]->Push(
982-
LC_Condition(GT_NE, LC_Expr(LC_Ident::CreateConst(1)), LC_Expr(LC_Ident::CreateConst(0))));
983-
}
984-
else
985-
{
986-
// For level 0, just push (a != null).
987-
(*conds)[level]->Push(
988-
LC_Condition(GT_NE, LC_Expr(LC_Ident::CreateVar(Lcl())), LC_Expr(LC_Ident::CreateNull())));
989-
}
980+
// For level 0, just push (a != null).
981+
(*conds)[level]->Push(
982+
LC_Condition(GT_NE, LC_Expr(LC_Ident::CreateVar(Lcl())), LC_Expr(LC_Ident::CreateNull())));
990983
}
991984
else
992985
{
993-
assert(!this->array.arrIndex->isSpan);
986+
assert(this->array.arrIndex->HasArrayObj());
994987

995988
// Adjust for level0 having just 1 condition and push conditions (i >= 0) && (i < a.len).
996989
// We fold the two compares into one using unsigned compare, since we know a.len is non-negative.
@@ -1109,7 +1102,7 @@ bool Compiler::optDeriveLoopCloningConditions(FlowGraphNaturalLoop* loop, LoopCl
11091102
JitExpandArrayStack<LcOptInfo*>* optInfos = context->GetLoopOptInfo(loop->GetIndex());
11101103
assert(optInfos->Size() > 0);
11111104

1112-
bool spanInvolved = false;
1105+
bool spanMaybeInvolved = false;
11131106

11141107
// We only need to check for iteration behavior if we have array checks.
11151108
//
@@ -1122,7 +1115,7 @@ bool Compiler::optDeriveLoopCloningConditions(FlowGraphNaturalLoop* loop, LoopCl
11221115
{
11231116
case LcOptInfo::LcJaggedArray:
11241117
// Keep a note that we might be dealing with a Span
1125-
spanInvolved |= optInfo->AsLcJaggedArrayOptInfo()->arrIndex.isSpan;
1118+
spanMaybeInvolved |= optInfo->AsLcJaggedArrayOptInfo()->arrIndex.HasLength();
11261119
checkIterationBehavior = true;
11271120
break;
11281121

@@ -1205,7 +1198,7 @@ bool Compiler::optDeriveLoopCloningConditions(FlowGraphNaturalLoop* loop, LoopCl
12051198
return false;
12061199
}
12071200

1208-
if (spanInvolved && (stride > 1))
1201+
if (spanMaybeInvolved && (stride > 1))
12091202
{
12101203
// Span<T> can only be iterated with a stride of 1 since its Length
12111204
// can be INT32_MAX.
@@ -1514,6 +1507,14 @@ bool Compiler::optComputeDerefConditions(FlowGraphNaturalLoop* loop, LoopCloneCo
15141507
{
15151508
LC_Array& array = (*arrayDeref)[i];
15161509

1510+
if (array.arrIndex->HasLength())
1511+
{
1512+
// If we have only length, then we don't need to deref the array (we don't even have the array object)
1513+
// and the array is guaranteed to be single-dimensional.
1514+
maxRank = max(0, maxRank);
1515+
continue;
1516+
}
1517+
15171518
// First populate the array base variable.
15181519
LC_ArrayDeref* node = LC_ArrayDeref::Find(&arrayDerefNodes, array.arrIndex->arrLcl);
15191520
if (node == nullptr)
@@ -1954,7 +1955,6 @@ BasicBlock* Compiler::optInsertLoopChoiceConditions(LoopCloneContext* contex
19541955
BasicBlock* insertAfter)
19551956
{
19561957
JITDUMP("Inserting loop " FMT_LP " loop choice conditions\n", loop->GetIndex());
1957-
assert(context->HasBlockConditions(loop->GetIndex()));
19581958
assert(slowPreheader != nullptr);
19591959

19601960
if (context->HasBlockConditions(loop->GetIndex()))
@@ -2128,9 +2128,6 @@ void Compiler::optCloneLoop(FlowGraphNaturalLoop* loop, LoopCloneContext* contex
21282128
// ...
21292129
// slowPreheader --> slowHeader
21302130
//
2131-
// We should always have block conditions.
2132-
2133-
assert(context->HasBlockConditions(loop->GetIndex()));
21342131

21352132
// If any condition is false, go to slowPreheader (which branches or falls through to header of the slow loop).
21362133
BasicBlock* slowHeader = nullptr;
@@ -2278,24 +2275,24 @@ bool Compiler::optExtractArrIndex(GenTree* tree, ArrIndex* result, unsigned lhsN
22782275
return false;
22792276
}
22802277

2281-
bool isSpan = false;
2282-
unsigned arrLcl;
2278+
unsigned arrLcl = BAD_VAR_NUM;
2279+
unsigned arrLenLcl = BAD_VAR_NUM;
2280+
22832281
GenTree* arrLen = arrBndsChk->GetArrayLength();
2282+
if (!arrLen->TypeIs(TYP_INT))
2283+
{
2284+
return false;
2285+
}
2286+
22842287
if (arrLen->OperIsArrLength() && arrLen->gtGetOp1()->OperIs(GT_LCL_VAR))
22852288
{
22862289
// Case 1: Arrays (jagged or multi-dimensional), Strings
22872290
arrLcl = arrLen->gtGetOp1()->AsLclVarCommon()->GetLclNum();
22882291
}
22892292
else if (arrLen->OperIs(GT_LCL_VAR))
22902293
{
2291-
// Case 2: Promoted spans
2292-
arrLcl = arrLen->AsLclVarCommon()->GetLclNum();
2293-
if (!lvaGetDesc(arrLcl)->IsSpanLength())
2294-
{
2295-
return false;
2296-
}
2297-
isSpan = true;
2298-
assert(arrLen->TypeIs(TYP_INT));
2294+
// Case 2: Array's length (or Span) is stored in a local variable.
2295+
arrLenLcl = arrLen->AsLclVarCommon()->GetLclNum();
22992296
}
23002297
else
23012298
{
@@ -2313,7 +2310,7 @@ bool Compiler::optExtractArrIndex(GenTree* tree, ArrIndex* result, unsigned lhsN
23132310
{
23142311
result->arrLcl = arrLcl;
23152312
}
2316-
result->isSpan = isSpan;
2313+
result->arrLenLcl = arrLenLcl;
23172314
result->indLcls.Push(indLcl);
23182315
result->bndsChks.Push(tree);
23192316
result->useBlock = compCurBB;
@@ -2546,10 +2543,19 @@ Compiler::fgWalkResult Compiler::optCanOptimizeByLoopCloning(GenTree* tree, Loop
25462543
}
25472544
#endif
25482545

2549-
// Check that the array object local variable is invariant within the loop body.
2550-
if (!optIsStackLocalInvariant(info->loop, arrIndex.arrLcl))
2546+
// Check that the array object (or its length) local variable is invariant within the loop body.
2547+
if (!optIsStackLocalInvariant(info->loop,
2548+
arrIndex.HasArrayObj() ? arrIndex.GetArrayObjLcl() : arrIndex.GetLengthLcl()))
25512549
{
2552-
JITDUMP("V%02d is not loop invariant\n", arrIndex.arrLcl);
2550+
if (arrIndex.HasArrayObj())
2551+
{
2552+
JITDUMP("ArrObj V%02d is not loop invariant\n", arrIndex.GetArrayObjLcl());
2553+
}
2554+
else
2555+
{
2556+
assert(arrIndex.HasLength());
2557+
JITDUMP("ArrLen V%02d is not loop invariant\n", arrIndex.GetLengthLcl());
2558+
}
25532559
return WALK_SKIP_SUBTREES;
25542560
}
25552561

‎src/coreclr/jit/loopcloning.h‎

Lines changed: 39 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -190,23 +190,50 @@ class Compiler;
190190
*/
191191
struct ArrIndex
192192
{
193-
unsigned arrLcl; // The array base local num
194-
JitExpandArrayStack<unsigned> indLcls; // The indices local nums
195-
JitExpandArrayStack<GenTree*> bndsChks; // The bounds checks nodes along each dimension.
196-
unsigned rank; // Rank of the array
197-
BasicBlock* useBlock; // Block where the [] occurs
198-
bool isSpan;
193+
// We either have an array object or its length
194+
unsigned arrLcl; // The array base local num
195+
unsigned arrLenLcl; // The array length local num
196+
JitExpandArrayStack<unsigned> indLcls; // The indices local nums
197+
JitExpandArrayStack<GenTree*> bndsChks; // The bounds checks nodes along each dimension.
198+
unsigned rank; // Rank of the array
199+
BasicBlock* useBlock; // Block where the [] occurs
199200

200201
ArrIndex(CompAllocator alloc)
201202
: arrLcl(BAD_VAR_NUM)
203+
, arrLenLcl(BAD_VAR_NUM)
202204
, indLcls(alloc)
203205
, bndsChks(alloc)
204206
, rank(0)
205207
, useBlock(nullptr)
206-
, isSpan(false)
207208
{
208209
}
209210

211+
bool HasArrayObj() const
212+
{
213+
return arrLcl != BAD_VAR_NUM;
214+
}
215+
216+
bool HasLength() const
217+
{
218+
return arrLenLcl != BAD_VAR_NUM;
219+
}
220+
221+
unsigned GetArrayObjLcl() const
222+
{
223+
// arrLcl and arrLenLcl are mutually exclusive.
224+
assert(arrLenLcl == BAD_VAR_NUM);
225+
assert(arrLcl != BAD_VAR_NUM);
226+
return arrLcl;
227+
}
228+
229+
unsigned GetLengthLcl() const
230+
{
231+
// arrLcl and arrLenLcl are mutually exclusive.
232+
assert(arrLcl == BAD_VAR_NUM);
233+
assert(arrLenLcl != BAD_VAR_NUM);
234+
return arrLenLcl;
235+
}
236+
210237
#ifdef DEBUG
211238
void Print(unsigned dim = -1);
212239
void PrintBoundsCheckNodes(unsigned dim = -1);
@@ -292,7 +319,8 @@ struct LcMdArrayOptInfo : public LcOptInfo
292319
{
293320
index->indLcls.Push(arrElem->gtArrInds[i]->AsLclVarCommon()->GetLclNum());
294321
}
295-
index->arrLcl = arrElem->gtArrObj->AsLclVarCommon()->GetLclNum();
322+
index->arrLcl = arrElem->gtArrObj->AsLclVarCommon()->GetLclNum();
323+
index->arrLenLcl = BAD_VAR_NUM;
296324
}
297325
return index;
298326
}
@@ -449,7 +477,8 @@ struct LC_Array
449477
assert(type != Invalid && that.type != Invalid);
450478

451479
// Types match and the array base matches.
452-
if (type != that.type || arrIndex->arrLcl != that.arrIndex->arrLcl || oper != that.oper)
480+
if ((type != that.type) || (arrIndex->arrLcl != that.arrIndex->arrLcl) ||
481+
(arrIndex->arrLenLcl != that.arrIndex->arrLenLcl) || (oper != that.oper))
453482
{
454483
return false;
455484
}
@@ -869,7 +898,7 @@ struct LoopCloneContext
869898
// mark the slow path as "run rarely", since it really shouldn't execute (given the currently optimized loop
870899
// conditions) except under exceptional circumstances.
871900
//
872-
static constexpr weight_t fastPathWeightScaleFactor = 0.99;
901+
static constexpr weight_t fastPathWeightScaleFactor = 1.0;
873902
static constexpr weight_t slowPathWeightScaleFactor = 1.0 - fastPathWeightScaleFactor;
874903

875904
CompAllocator alloc; // The allocator

0 commit comments

Comments
 (0)