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

Support loop cloning of byte array accesses #48894

Merged
merged 2 commits into from
Mar 2, 2021
Merged
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
67 changes: 43 additions & 24 deletions src/coreclr/jit/optimizer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -8453,24 +8453,38 @@ bool Compiler::optIdentifyLoopOptInfo(unsigned loopNum, LoopCloneContext* contex
//
// TODO-CQ: CLONE: After morph make sure this method extracts values before morph.
//
// STMT void(IL 0x007...0x00C)
// [000023] -A-XG+------ * ASG int
// [000022] D----+-N---- +--* LCL_VAR int V06 tmp1
// [000048] ---XG+------ \--* COMMA int
// [000041] ---X-+------ +--* ARR_BOUNDS_CHECK_Rng void
// [000020] -----+------ | +--* LCL_VAR int V04 loc0
// [000040] ---X-+------ | \--* ARR_LENGTH int
// [000019] -----+------ | \--* LCL_VAR ref V00 arg0
// [000021] a--XG+------ \--* IND int
// [000047] -----+------ \--* ADD byref
// [000038] -----+------ +--* LCL_VAR ref V00 arg0
// [000046] -----+------ \--* ADD long
// [000044] -----+------ +--* LSH long
// [000042] -----+------ | +--* CAST long < -int
// [000039] i----+------ | | \--* LCL_VAR int V04 loc0
// [000043] -----+-N---- | \--* CNS_INT long 2
// [000045] -----+------ \--* CNS_INT long 16 Fseq[#FirstElem]

// Example tree to pattern match:
//
// * COMMA int
// +--* ARR_BOUNDS_CHECK_Rng void
// | +--* LCL_VAR int V02 loc1
// | \--* ARR_LENGTH int
// | \--* LCL_VAR ref V00 arg0
// \--* IND int
// \--* ADD byref
// +--* LCL_VAR ref V00 arg0
// \--* ADD long
// +--* LSH long
// | +--* CAST long <- int
// | | \--* LCL_VAR int V02 loc1
// | \--* CNS_INT long 2
// \--* CNS_INT long 16 Fseq[#FirstElem]
//
// Note that byte arrays don't require the LSH to scale the index, so look like this:
//
// * COMMA ubyte
// +--* ARR_BOUNDS_CHECK_Rng void
// | +--* LCL_VAR int V03 loc2
// | \--* ARR_LENGTH int
// | \--* LCL_VAR ref V00 arg0
// \--* IND ubyte
// \--* ADD byref
// +--* LCL_VAR ref V00 arg0
// \--* ADD long
// +--* CAST long <- int
// | \--* LCL_VAR int V03 loc2
// \--* CNS_INT long 16 Fseq[#FirstElem]
//
bool Compiler::optExtractArrIndex(GenTree* tree, ArrIndex* result, unsigned lhsNum)
{
if (tree->gtOper != GT_COMMA)
Expand Down Expand Up @@ -8544,15 +8558,20 @@ bool Compiler::optExtractArrIndex(GenTree* tree, ArrIndex* result, unsigned lhsN
{
return false;
}
if (si->gtOper != GT_LSH)
GenTree* index;
if (si->gtOper == GT_LSH)
{
return false;
GenTree* scale = si->gtGetOp2();
index = si->gtGetOp1();
if (scale->gtOper != GT_CNS_INT)
{
return false;
}
}
GenTree* scale = si->gtGetOp2();
GenTree* index = si->gtGetOp1();
if (scale->gtOper != GT_CNS_INT)
else
{
return false;
// No scale (e.g., byte array).
index = si;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we need to add some assert to make sure the array is what we expect?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry, I missed this before merge. There's certainly some question about what additional things could/should be checked in this pattern matching. Types, for example. For this case, the index is checked (lower down) to be a lclvar that is the same as the array bounds check base, so we're good.

}
#ifdef TARGET_64BIT
if (index->gtOper != GT_CAST)
Expand Down