Skip to content

Commit 0efeb9f

Browse files
author
Sergey Andreenko
authored
2 small CQ optimizations. (#37967)
* `REF(0)==INT(0)` for reg reuse. * Avoid field by field init when not profitable.
1 parent bc65d6b commit 0efeb9f

File tree

2 files changed

+28
-14
lines changed

2 files changed

+28
-14
lines changed

src/coreclr/src/jit/lsra.cpp

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2733,8 +2733,10 @@ bool LinearScan::isMatchingConstant(RegRecord* physRegRecord, RefPosition* refPo
27332733
switch (otherTreeNode->OperGet())
27342734
{
27352735
case GT_CNS_INT:
2736-
if ((refPosition->treeNode->AsIntCon()->IconValue() == otherTreeNode->AsIntCon()->IconValue()) &&
2737-
(varTypeGCtype(refPosition->treeNode) == varTypeGCtype(otherTreeNode)))
2736+
{
2737+
ssize_t v1 = refPosition->treeNode->AsIntCon()->IconValue();
2738+
ssize_t v2 = otherTreeNode->AsIntCon()->IconValue();
2739+
if ((v1 == v2) && (varTypeGCtype(refPosition->treeNode) == varTypeGCtype(otherTreeNode) || v1 == 0))
27382740
{
27392741
#ifdef TARGET_64BIT
27402742
// If the constant is negative, only reuse registers of the same type.
@@ -2743,14 +2745,14 @@ bool LinearScan::isMatchingConstant(RegRecord* physRegRecord, RefPosition* refPo
27432745
// This doesn't apply to a 32-bit system, on which long values occupy multiple registers.
27442746
// (We could sign-extend, but we would have to always sign-extend, because if we reuse more
27452747
// than once, we won't have access to the instruction that originally defines the constant).
2746-
if ((refPosition->treeNode->TypeGet() == otherTreeNode->TypeGet()) ||
2747-
(refPosition->treeNode->AsIntCon()->IconValue() >= 0))
2748+
if ((refPosition->treeNode->TypeGet() == otherTreeNode->TypeGet()) || (v1 >= 0))
27482749
#endif // TARGET_64BIT
27492750
{
27502751
return true;
27512752
}
27522753
}
27532754
break;
2755+
}
27542756
case GT_CNS_DBL:
27552757
{
27562758
// For floating point constants, the values must be identical, not simply compare

src/coreclr/src/jit/morph.cpp

Lines changed: 22 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -10600,8 +10600,20 @@ GenTree* Compiler::fgMorphCopyBlock(GenTree* tree)
1060010600
// If we passed the above checks, then we will check these two
1060110601
if (!requiresCopyBlock)
1060210602
{
10603+
// It is not always profitable to do field by field init for structs that are allocated to memory.
10604+
// A struct with 8 bool fields will require 8 moves instead of one if we do this transformation.
10605+
// A simple heuristic when field by field copy is prefered:
10606+
// - if fields can be enregistered;
10607+
// - if the struct has GCPtrs (block copy would be done via helper that is expensive);
10608+
// - if the struct has only one field.
10609+
bool dstFldIsProfitable =
10610+
((destLclVar != nullptr) &&
10611+
(!destLclVar->lvDoNotEnregister || destLclVar->HasGCPtr() || (destLclVar->lvFieldCnt == 1)));
10612+
bool srcFldIsProfitable =
10613+
((srcLclVar != nullptr) &&
10614+
(!srcLclVar->lvDoNotEnregister || srcLclVar->HasGCPtr() || (srcLclVar->lvFieldCnt == 1)));
1060310615
// Are both dest and src promoted structs?
10604-
if (destDoFldAsg && srcDoFldAsg)
10616+
if (destDoFldAsg && srcDoFldAsg && (dstFldIsProfitable || srcFldIsProfitable))
1060510617
{
1060610618
// Both structs should be of the same type, or have the same number of fields of the same type.
1060710619
// If not we will use a copy block.
@@ -10633,13 +10645,7 @@ GenTree* Compiler::fgMorphCopyBlock(GenTree* tree)
1063310645
}
1063410646
}
1063510647
}
10636-
// Are neither dest or src promoted structs?
10637-
else if (!destDoFldAsg && !srcDoFldAsg)
10638-
{
10639-
requiresCopyBlock = true; // Leave as a CopyBlock
10640-
JITDUMP(" with no promoted structs");
10641-
}
10642-
else if (destDoFldAsg)
10648+
else if (destDoFldAsg && dstFldIsProfitable)
1064310649
{
1064410650
// Match the following kinds of trees:
1064510651
// fgMorphTree BB01, stmt 9 (before)
@@ -10683,9 +10689,8 @@ GenTree* Compiler::fgMorphCopyBlock(GenTree* tree)
1068310689
}
1068410690
}
1068510691
}
10686-
else
10692+
else if (srcDoFldAsg && srcFldIsProfitable)
1068710693
{
10688-
assert(srcDoFldAsg);
1068910694
// Check for the symmetric case (which happens for the _pointer field of promoted spans):
1069010695
//
1069110696
// [000240] -----+------ /--* lclVar struct(P) V18 tmp9
@@ -10707,6 +10712,13 @@ GenTree* Compiler::fgMorphCopyBlock(GenTree* tree)
1070710712
}
1070810713
}
1070910714
}
10715+
// Are neither dest or src promoted structs?
10716+
else
10717+
{
10718+
assert(!(destDoFldAsg && dstFldIsProfitable) && !(srcDoFldAsg && srcFldIsProfitable));
10719+
requiresCopyBlock = true; // Leave as a CopyBlock
10720+
JITDUMP(" with no promoted structs");
10721+
}
1071010722
}
1071110723

1071210724
// If we require a copy block the set both of the field assign bools to false

0 commit comments

Comments
 (0)