From 6d73f6e1d9f1fe2eb073803b51b0cf8de1b8e9aa Mon Sep 17 00:00:00 2001 From: Mike Danes Date: Wed, 7 Jun 2017 14:42:30 +0300 Subject: [PATCH] Improve long relop lowering Commit migrated from https://github.com/dotnet/coreclr/commit/7657f216b8e09fff9b4969efea78370c096f42fe --- src/coreclr/src/jit/lower.cpp | 83 +++++++++++++++++++++++++++++++++++++++---- 1 file changed, 77 insertions(+), 6 deletions(-) diff --git a/src/coreclr/src/jit/lower.cpp b/src/coreclr/src/jit/lower.cpp index 8c09014..572a2a1 100644 --- a/src/coreclr/src/jit/lower.cpp +++ b/src/coreclr/src/jit/lower.cpp @@ -2034,10 +2034,20 @@ void Lowering::LowerCompare(GenTree* cmp) if (cmp->OperIs(GT_EQ, GT_NE)) { // - // Transform (x EQ|NE y) into (((x.lo SUB y.lo) OR (x.hi SUB y.hi)) EQ|NE 0). If y is 0 then this can + // Transform (x EQ|NE y) into (((x.lo XOR y.lo) OR (x.hi XOR y.hi)) EQ|NE 0). If y is 0 then this can // be reduced to just ((x.lo OR x.hi) EQ|NE 0). The OR is expected to set the condition flags so we // don't need to generate a redundant compare against 0, we only generate a SETCC|JCC instruction. // + // XOR is used rather than SUB because it is commutative and thus allows swapping the operands when + // the first happens to be a constant. Usually only the second compare operand is a constant but it's + // still possible to have a constant on the left side. For example, when src1 is a uint->ulong cast + // then hiSrc1 would be 0. + // + + if (loSrc1->OperIs(GT_CNS_INT)) + { + std::swap(loSrc1, loSrc2); + } if (loSrc2->IsIntegralConst(0)) { @@ -2046,10 +2056,15 @@ void Lowering::LowerCompare(GenTree* cmp) } else { - loCmp = comp->gtNewOperNode(GT_SUB, TYP_INT, loSrc1, loSrc2); + loCmp = comp->gtNewOperNode(GT_XOR, TYP_INT, loSrc1, loSrc2); BlockRange().InsertBefore(cmp, loCmp); } + if (hiSrc1->OperIs(GT_CNS_INT)) + { + std::swap(hiSrc1, hiSrc2); + } + if (hiSrc2->IsIntegralConst(0)) { BlockRange().Remove(hiSrc2); @@ -2057,7 +2072,7 @@ void Lowering::LowerCompare(GenTree* cmp) } else { - hiCmp = comp->gtNewOperNode(GT_SUB, TYP_INT, hiSrc1, hiSrc2); + hiCmp = comp->gtNewOperNode(GT_XOR, TYP_INT, hiSrc1, hiSrc2); BlockRange().InsertBefore(cmp, hiCmp); } @@ -2085,12 +2100,42 @@ void Lowering::LowerCompare(GenTree* cmp) // like the one we get for LT|GE compares, this can be achieved by swapping the compare: // (x LE|GT y) becomes (y GE|LT x) // + // Having to swap operands is problematic when the second operand is a constant. The constant + // moves to the first operand where it cannot be contained and thus needs a register. This can + // be avoided by changing the constant such that LE|GT becomes LT|GE: + // (x LE|GT 41) becomes (x LT|GE 42) + // if (cmp->OperIs(GT_LE, GT_GT)) { - std::swap(loSrc1, loSrc2); - std::swap(hiSrc1, hiSrc2); - condition = GenTree::SwapRelop(condition); + bool mustSwap = true; + + if (loSrc2->OperIs(GT_CNS_INT) && hiSrc2->OperIs(GT_CNS_INT)) + { + uint32_t loValue = static_cast(loSrc2->AsIntCon()->IconValue()); + uint32_t hiValue = static_cast(hiSrc2->AsIntCon()->IconValue()); + uint64_t value = static_cast(loValue) | (static_cast(hiValue) << 32); + uint64_t maxValue = cmp->IsUnsigned() ? UINT64_MAX : INT64_MAX; + + if (value != maxValue) + { + value++; + loValue = value & UINT32_MAX; + hiValue = (value >> 32) & UINT32_MAX; + loSrc2->AsIntCon()->SetIconValue(loValue); + hiSrc2->AsIntCon()->SetIconValue(hiValue); + + condition = cmp->OperIs(GT_LE) ? GT_LT : GT_GE; + mustSwap = false; + } + } + + if (mustSwap) + { + std::swap(loSrc1, loSrc2); + std::swap(hiSrc1, hiSrc2); + condition = GenTree::SwapRelop(condition); + } } assert((condition == GT_LT) || (condition == GT_GE)); @@ -2099,6 +2144,18 @@ void Lowering::LowerCompare(GenTree* cmp) { BlockRange().Remove(loSrc2); + // Very conservative dead code removal... but it helps. + + if (loSrc1->OperIs(GT_CNS_INT, GT_LCL_VAR, GT_LCL_FLD)) + { + BlockRange().Remove(loSrc1); + + if (loSrc1->OperIs(GT_LCL_VAR, GT_LCL_FLD)) + { + comp->lvaDecRefCnts(m_block, loSrc1); + } + } + hiCmp = comp->gtNewOperNode(GT_CMP, TYP_VOID, hiSrc1, hiSrc2); BlockRange().InsertBefore(cmp, hiCmp); } @@ -2107,6 +2164,20 @@ void Lowering::LowerCompare(GenTree* cmp) loCmp = comp->gtNewOperNode(GT_CMP, TYP_VOID, loSrc1, loSrc2); hiCmp = comp->gtNewOperNode(GT_SUB_HI, TYP_INT, hiSrc1, hiSrc2); BlockRange().InsertBefore(cmp, loCmp, hiCmp); + + // + // Try to move the first SUB_HI operands right in front of it, this allows using + // a single temporary register instead of 2 (one for CMP and one for SUB_HI). Do + // this only for locals as they won't change condition flags. Note that we could + // move constants (except 0 which generates XOR reg, reg) but it's extremly rare + // to have a constant as the first operand. + // + + if (hiSrc1->OperIs(GT_LCL_VAR, GT_LCL_FLD)) + { + BlockRange().Remove(hiSrc1); + BlockRange().InsertBefore(hiCmp, hiSrc1); + } } } -- 2.7.4