Skip to content

Commit bb346e2

Browse files
EgorBoEgorBo
committed
cleanup
1 parent 4ca1691 commit bb346e2

File tree

3 files changed

+121
-28
lines changed

3 files changed

+121
-28
lines changed

‎src/coreclr/jit/importercalls.cpp

+2
Original file line numberDiff line numberDiff line change
@@ -3671,6 +3671,8 @@ GenTree* Compiler::impIntrinsic(CORINFO_CLASS_HANDLE clsHnd,
36713671
case NI_System_Span_get_Item:
36723672
case NI_System_ReadOnlySpan_get_Item:
36733673
{
3674+
optMethodFlags |= OMF_HAS_ARRAYREF;
3675+
36743676
// Have index, stack pointer-to Span<T> s on the stack. Expand to:
36753677
//
36763678
// For Span<T>

‎src/coreclr/jit/loopcloning.cpp

+81-21
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,8 +74,19 @@ 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+
}
86+
87+
GenTree* arr =
88+
comp->gtNewLclvNode(arrIndex->GetArrayObjLcl(), comp->lvaTable[arrIndex->GetArrayObjLcl()].lvType);
89+
7290
for (int i = 0; i < rank; ++i)
7391
{
7492
GenTree* idx = comp->gtNewLclvNode(arrIndex->indLcls[i], comp->lvaTable[arrIndex->indLcls[i]].lvType);
@@ -919,7 +937,7 @@ unsigned LC_ArrayDeref::Lcl()
919937
unsigned lvl = level;
920938
if (lvl == 0)
921939
{
922-
return array.arrIndex->arrLcl;
940+
return array.arrIndex->GetArrayObjLcl();
923941
}
924942
lvl--;
925943
return array.arrIndex->indLcls[lvl];
@@ -955,6 +973,8 @@ bool LC_ArrayDeref::HasChildren()
955973
//
956974
void LC_ArrayDeref::DeriveLevelConditions(JitExpandArrayStack<JitExpandArrayStack<LC_Condition>*>* conds)
957975
{
976+
assert(this->array.arrIndex->HasArrayObj());
977+
958978
if (level == 0)
959979
{
960980
// For level 0, just push (a != null).
@@ -1080,6 +1100,8 @@ bool Compiler::optDeriveLoopCloningConditions(FlowGraphNaturalLoop* loop, LoopCl
10801100
JitExpandArrayStack<LcOptInfo*>* optInfos = context->GetLoopOptInfo(loop->GetIndex());
10811101
assert(optInfos->Size() > 0);
10821102

1103+
bool canBeSpan = false;
1104+
10831105
// We only need to check for iteration behavior if we have array checks.
10841106
//
10851107
bool checkIterationBehavior = false;
@@ -1090,6 +1112,11 @@ bool Compiler::optDeriveLoopCloningConditions(FlowGraphNaturalLoop* loop, LoopCl
10901112
switch (optInfo->GetOptType())
10911113
{
10921114
case LcOptInfo::LcJaggedArray:
1115+
// Keep a note that we *might* be dealing with a Span
1116+
canBeSpan |= optInfo->AsLcJaggedArrayOptInfo()->arrIndex.HasLength();
1117+
checkIterationBehavior = true;
1118+
break;
1119+
10931120
case LcOptInfo::LcMdArray:
10941121
checkIterationBehavior = true;
10951122
break;
@@ -1161,13 +1188,19 @@ bool Compiler::optDeriveLoopCloningConditions(FlowGraphNaturalLoop* loop, LoopCl
11611188
// is beyond the limit.
11621189
int stride = abs(iterInfo->IterConst());
11631190

1164-
if (stride >= 58)
1191+
static_assert_no_msg(INT32_MAX >= CORINFO_Array_MaxLength);
1192+
if (stride >= (INT32_MAX - CORINFO_Array_MaxLength))
11651193
{
11661194
// Array.MaxLength can have maximum of 0X7FFFFFC7 elements, so make sure
1167-
// the stride increment doesn't overflow or underflow the index. Hence,
1168-
// the maximum stride limit is set to
1169-
// (int.MaxValue - (Array.MaxLength - 1) + 1), which is
1170-
// (0X7fffffff - 0x7fffffc7 + 2) = 0x3a or 58.
1195+
// the stride increment doesn't overflow or underflow the index.
1196+
return false;
1197+
}
1198+
1199+
// We don't know exactly whether we might be dealing with a Span<T> or not,
1200+
// but if we suspect we are, we need to be careful about the stride:
1201+
// As Span<>.Length can be INT32_MAX unlike arrays.
1202+
if (canBeSpan && (stride > 1))
1203+
{
11711204
return false;
11721205
}
11731206

@@ -1473,6 +1506,14 @@ bool Compiler::optComputeDerefConditions(FlowGraphNaturalLoop* loop, LoopCloneCo
14731506
{
14741507
LC_Array& array = (*arrayDeref)[i];
14751508

1509+
if (array.arrIndex->HasLength())
1510+
{
1511+
// If we have only length, then we don't need to deref the array (we don't even have the array object)
1512+
// and the array is guaranteed to be single-dimensional.
1513+
maxRank = max(0, maxRank);
1514+
continue;
1515+
}
1516+
14761517
// First populate the array base variable.
14771518
LC_ArrayDeref* node = LC_ArrayDeref::Find(&arrayDerefNodes, array.arrIndex->arrLcl);
14781519
if (node == nullptr)
@@ -1913,7 +1954,6 @@ BasicBlock* Compiler::optInsertLoopChoiceConditions(LoopCloneContext* contex
19131954
BasicBlock* insertAfter)
19141955
{
19151956
JITDUMP("Inserting loop " FMT_LP " loop choice conditions\n", loop->GetIndex());
1916-
assert(context->HasBlockConditions(loop->GetIndex()));
19171957
assert(slowPreheader != nullptr);
19181958

19191959
if (context->HasBlockConditions(loop->GetIndex()))
@@ -2087,9 +2127,6 @@ void Compiler::optCloneLoop(FlowGraphNaturalLoop* loop, LoopCloneContext* contex
20872127
// ...
20882128
// slowPreheader --> slowHeader
20892129
//
2090-
// We should always have block conditions.
2091-
2092-
assert(context->HasBlockConditions(loop->GetIndex()));
20932130

20942131
// If any condition is false, go to slowPreheader (which branches or falls through to header of the slow loop).
20952132
BasicBlock* slowHeader = nullptr;
@@ -2237,17 +2274,30 @@ bool Compiler::optExtractArrIndex(GenTree* tree, ArrIndex* result, unsigned lhsN
22372274
return false;
22382275
}
22392276

2240-
// For span we may see the array length is a local var or local field or constant.
2241-
// We won't try and extract those.
2242-
if (arrBndsChk->GetArrayLength()->OperIs(GT_LCL_VAR, GT_LCL_FLD, GT_CNS_INT))
2277+
unsigned arrLcl = BAD_VAR_NUM;
2278+
unsigned arrLenLcl = BAD_VAR_NUM;
2279+
2280+
GenTree* arrLen = arrBndsChk->GetArrayLength();
2281+
if (!arrLen->TypeIs(TYP_INT))
22432282
{
22442283
return false;
22452284
}
2246-
if (arrBndsChk->GetArrayLength()->gtGetOp1()->gtOper != GT_LCL_VAR)
2285+
2286+
if (arrLen->OperIsArrLength() && arrLen->gtGetOp1()->OperIs(GT_LCL_VAR))
2287+
{
2288+
// Case 1: Arrays (jagged or multi-dimensional), Strings
2289+
arrLcl = arrLen->gtGetOp1()->AsLclVarCommon()->GetLclNum();
2290+
}
2291+
else if (arrLen->OperIs(GT_LCL_VAR))
2292+
{
2293+
// Case 2: Array's length (or Span) is stored in a local variable.
2294+
arrLenLcl = arrLen->AsLclVarCommon()->GetLclNum();
2295+
}
2296+
else
22472297
{
22482298
return false;
22492299
}
2250-
unsigned arrLcl = arrBndsChk->GetArrayLength()->gtGetOp1()->AsLclVarCommon()->GetLclNum();
2300+
22512301
if (lhsNum != BAD_VAR_NUM && arrLcl != lhsNum)
22522302
{
22532303
return false;
@@ -2259,6 +2309,7 @@ bool Compiler::optExtractArrIndex(GenTree* tree, ArrIndex* result, unsigned lhsN
22592309
{
22602310
result->arrLcl = arrLcl;
22612311
}
2312+
result->arrLenLcl = arrLenLcl;
22622313
result->indLcls.Push(indLcl);
22632314
result->bndsChks.Push(tree);
22642315
result->useBlock = compCurBB;
@@ -2491,10 +2542,19 @@ Compiler::fgWalkResult Compiler::optCanOptimizeByLoopCloning(GenTree* tree, Loop
24912542
}
24922543
#endif
24932544

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

‎src/coreclr/jit/loopcloning.h

+38-7
Original file line numberDiff line numberDiff line change
@@ -190,21 +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
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
198200

199201
ArrIndex(CompAllocator alloc)
200202
: arrLcl(BAD_VAR_NUM)
203+
, arrLenLcl(BAD_VAR_NUM)
201204
, indLcls(alloc)
202205
, bndsChks(alloc)
203206
, rank(0)
204207
, useBlock(nullptr)
205208
{
206209
}
207210

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+
208237
#ifdef DEBUG
209238
void Print(unsigned dim = -1);
210239
void PrintBoundsCheckNodes(unsigned dim = -1);
@@ -290,7 +319,8 @@ struct LcMdArrayOptInfo : public LcOptInfo
290319
{
291320
index->indLcls.Push(arrElem->gtArrInds[i]->AsLclVarCommon()->GetLclNum());
292321
}
293-
index->arrLcl = arrElem->gtArrObj->AsLclVarCommon()->GetLclNum();
322+
index->arrLcl = arrElem->gtArrObj->AsLclVarCommon()->GetLclNum();
323+
index->arrLenLcl = BAD_VAR_NUM;
294324
}
295325
return index;
296326
}
@@ -447,7 +477,8 @@ struct LC_Array
447477
assert(type != Invalid && that.type != Invalid);
448478

449479
// Types match and the array base matches.
450-
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))
451482
{
452483
return false;
453484
}

0 commit comments

Comments
 (0)