From e8f9ae6ddccee50f8fb555b96859f1fe35b2f944 Mon Sep 17 00:00:00 2001 From: Alan Hayward Date: Tue, 22 Mar 2022 13:09:34 +0000 Subject: [PATCH 01/26] Add lowering support for conditional nodes --- src/coreclr/jit/compiler.h | 28 +++++++ src/coreclr/jit/compiler.hpp | 21 +++++ src/coreclr/jit/gentree.cpp | 123 +++++++++++++++++++++++++++- src/coreclr/jit/gentree.h | 10 +++ src/coreclr/jit/lir.cpp | 4 +- src/coreclr/jit/liveness.cpp | 10 ++- src/coreclr/jit/lower.cpp | 135 ++++++++++++++++++++++--------- src/coreclr/jit/lower.h | 4 +- src/coreclr/jit/lowerarmarch.cpp | 8 ++ src/coreclr/jit/lowerxarch.cpp | 20 +++-- src/coreclr/jit/lsraarm64.cpp | 18 +++++ 11 files changed, 328 insertions(+), 53 deletions(-) diff --git a/src/coreclr/jit/compiler.h b/src/coreclr/jit/compiler.h index a69148d93d5ef2..b26460f2adfb0b 100644 --- a/src/coreclr/jit/compiler.h +++ b/src/coreclr/jit/compiler.h @@ -10984,6 +10984,34 @@ class GenTreeVisitor break; #endif // defined(FEATURE_SIMD) || defined(FEATURE_HW_INTRINSICS) + case GT_SELECT: + case GT_CEQ: + case GT_CNE: + case GT_CLT: + case GT_CLE: + case GT_CGE: + case GT_CGT: + { + GenTreeConditional* const conditional = node->AsConditional(); + + result = WalkTree(&conditional->gtCond, conditional); + if (result == fgWalkResult::WALK_ABORT) + { + return result; + } + result = WalkTree(&conditional->gtOp1, conditional); + if (result == fgWalkResult::WALK_ABORT) + { + return result; + } + result = WalkTree(&conditional->gtOp2, conditional); + if (result == fgWalkResult::WALK_ABORT) + { + return result; + } + break; + } + // Binary nodes default: { diff --git a/src/coreclr/jit/compiler.hpp b/src/coreclr/jit/compiler.hpp index d353807493f5a1..eec2ccfe73d15e 100644 --- a/src/coreclr/jit/compiler.hpp +++ b/src/coreclr/jit/compiler.hpp @@ -4393,6 +4393,27 @@ void GenTree::VisitOperands(TVisitor visitor) return; } + case GT_SELECT: + case GT_CEQ: + case GT_CNE: + case GT_CLT: + case GT_CLE: + case GT_CGE: + case GT_CGT: + { + GenTreeConditional* const cond = this->AsConditional(); + if (visitor(cond->gtCond) == VisitResult::Abort) + { + return; + } + if (visitor(cond->gtOp1) == VisitResult::Abort) + { + return; + } + visitor(cond->gtOp2); + return; + } + // Binary nodes default: assert(this->OperIsBinary()); diff --git a/src/coreclr/jit/gentree.cpp b/src/coreclr/jit/gentree.cpp index 65809969a120b2..864d7227b5f421 100644 --- a/src/coreclr/jit/gentree.cpp +++ b/src/coreclr/jit/gentree.cpp @@ -5790,6 +5790,28 @@ unsigned Compiler::gtSetEvalOrder(GenTree* tree) costSz += tree->AsStoreDynBlk()->gtDynamicSize->GetCostSz(); break; + case GT_SELECT: + case GT_CEQ: + case GT_CNE: + case GT_CLT: + case GT_CLE: + case GT_CGE: + case GT_CGT: + level = gtSetEvalOrder(tree->AsConditional()->gtCond); + costEx = tree->AsConditional()->gtCond->GetCostEx(); + costSz = tree->AsConditional()->gtCond->GetCostSz(); + + lvl2 = gtSetEvalOrder(tree->AsConditional()->gtOp1); + level = max(level, lvl2); + costEx += tree->AsConditional()->gtOp1->GetCostEx(); + costSz += tree->AsConditional()->gtOp1->GetCostSz(); + + lvl2 = gtSetEvalOrder(tree->AsConditional()->gtOp2); + level = max(level, lvl2); + costEx += tree->AsConditional()->gtOp2->GetCostEx(); + costSz += tree->AsConditional()->gtOp2->GetCostSz(); + break; + default: JITDUMP("unexpected operator in this tree:\n"); DISPTREE(tree); @@ -6182,6 +6204,33 @@ bool GenTree::TryGetUse(GenTree* operand, GenTree*** pUse) return false; } + case GT_SELECT: + case GT_CEQ: + case GT_CNE: + case GT_CLT: + case GT_CLE: + case GT_CGE: + case GT_CGT: + { + GenTreeConditional* const conditional = this->AsConditional(); + if (operand == conditional->gtCond) + { + *pUse = &conditional->gtCond; + return true; + } + if (operand == conditional->gtOp1) + { + *pUse = &conditional->gtOp1; + return true; + } + if (operand == conditional->gtOp2) + { + *pUse = &conditional->gtOp2; + return true; + } + return false; + } + // Binary nodes default: assert(this->OperIsBinary()); @@ -8710,6 +8759,19 @@ GenTree* Compiler::gtCloneExpr( gtCloneExpr(tree->AsStoreDynBlk()->gtDynamicSize, addFlags, deepVarNum, deepVarVal)); break; + case GT_SELECT: + case GT_CEQ: + case GT_CNE: + case GT_CLT: + case GT_CLE: + case GT_CGE: + case GT_CGT: + copy = new (this, oper) + GenTreeConditional(oper, tree->TypeGet(), + gtCloneExpr(tree->AsConditional()->gtCond, addFlags, deepVarNum, deepVarVal), + gtCloneExpr(tree->AsConditional()->gtOp1, addFlags, deepVarNum, deepVarVal), + gtCloneExpr(tree->AsConditional()->gtOp2, addFlags, deepVarNum, deepVarVal)); + break; default: #ifdef DEBUG gtDispTree(tree); @@ -9369,6 +9431,18 @@ GenTreeUseEdgeIterator::GenTreeUseEdgeIterator(GenTree* node) AdvanceCall(); return; + case GT_SELECT: + case GT_CEQ: + case GT_CNE: + case GT_CLT: + case GT_CLE: + case GT_CGE: + case GT_CGT: + m_edge = &m_node->AsConditional()->gtCond; + assert(*m_edge != nullptr); + m_advance = &GenTreeUseEdgeIterator::AdvanceConditional; + return; + // Binary nodes default: assert(m_node->OperIsBinary()); @@ -9501,6 +9575,29 @@ void GenTreeUseEdgeIterator::AdvancePhi() } } +//------------------------------------------------------------------------ +// GenTreeUseEdgeIterator::AdvanceConditional: produces the next operand of a conditional node and advances the state. +// +void GenTreeUseEdgeIterator::AdvanceConditional() +{ + GenTreeConditional* const conditional = m_node->AsConditional(); + switch (m_state) + { + case 0: + m_edge = &conditional->gtOp1; + m_state = 1; + break; + case 1: + m_edge = &conditional->gtOp2; + m_advance = &GenTreeUseEdgeIterator::Terminate; + break; + default: + unreached(); + } + + assert(*m_edge != nullptr); +} + //------------------------------------------------------------------------ // GenTreeUseEdgeIterator::AdvanceBinOp: produces the next operand of a binary node and advances the state. // @@ -10372,6 +10469,13 @@ void Compiler::gtDispNode(GenTree* tree, IndentStack* indentStack, _In_ _In_opt_ case GT_GT: case GT_TEST_EQ: case GT_TEST_NE: + case GT_SELECT: + case GT_CEQ: + case GT_CNE: + case GT_CLT: + case GT_CLE: + case GT_CGE: + case GT_CGT: if (tree->gtFlags & GTF_RELOP_NAN_UN) { printf("N"); @@ -12017,6 +12121,23 @@ void Compiler::gtDispTree(GenTree* tree, } break; + case GT_SELECT: + case GT_CEQ: + case GT_CNE: + case GT_CLT: + case GT_CLE: + case GT_CGE: + case GT_CGT: + gtDispCommonEndLine(tree); + + if (!topOnly) + { + gtDispChild(tree->AsConditional()->gtCond, indentStack, IIArc, childMsg, topOnly); + gtDispChild(tree->AsConditional()->gtOp1, indentStack, IIArc, childMsg, topOnly); + gtDispChild(tree->AsConditional()->gtOp2, indentStack, IIArcBottom, childMsg, topOnly); + } + break; + default: printf(" :"); printf(""); // null string means flush @@ -12390,7 +12511,7 @@ void Compiler::gtDispLIRNode(GenTree* node, const char* prefixMsg /* = nullptr * IndentInfo operandArc = IIArcTop; for (GenTree* operand : node->Operands()) { - if (!operand->IsValue()) + if (!operand->IsValue() && (operand->gtFlags & GTF_SET_FLAGS) == 0) { // Either of these situations may happen with calls. continue; diff --git a/src/coreclr/jit/gentree.h b/src/coreclr/jit/gentree.h index e9d512e16c6d18..44e6fe9f32e41b 100644 --- a/src/coreclr/jit/gentree.h +++ b/src/coreclr/jit/gentree.h @@ -1375,6 +1375,16 @@ struct GenTree return OperIsConditionalCompare(OperGet()); } + static bool OperIsCC(genTreeOps gtOper) + { + return (gtOper == GT_JCC) || (gtOper == GT_SETCC); + } + + bool OperIsCC() const + { + return OperIsCC(OperGet()); + } + static bool OperIsShift(genTreeOps gtOper) { return (gtOper == GT_LSH) || (gtOper == GT_RSH) || (gtOper == GT_RSZ); diff --git a/src/coreclr/jit/lir.cpp b/src/coreclr/jit/lir.cpp index 5adef46eff7930..8a5801d0647105 100644 --- a/src/coreclr/jit/lir.cpp +++ b/src/coreclr/jit/lir.cpp @@ -1588,9 +1588,9 @@ bool LIR::Range::CheckLIR(Compiler* compiler, bool checkUnusedValues) const // Stack arguments do not produce a value, but they are considered children of the call. // It may be useful to remove these from being call operands, but that may also impact // other code that relies on being able to reach all the operands from a call node. - // The argument of a JTRUE doesn't produce a value (just sets a flag). + // The argument of a JTRUE or conditional doesn't produce a value (just sets a flag). assert(((node->OperGet() == GT_CALL) && def->OperIs(GT_PUTARG_STK)) || - ((node->OperGet() == GT_JTRUE) && (def->TypeGet() == TYP_VOID) && + ((node->OperGet() == GT_JTRUE || node->OperIsConditional()) && (def->TypeGet() == TYP_VOID) && ((def->gtFlags & GTF_SET_FLAGS) != 0))); continue; } diff --git a/src/coreclr/jit/liveness.cpp b/src/coreclr/jit/liveness.cpp index ddbdb586bc0159..4b8062838d2af3 100644 --- a/src/coreclr/jit/liveness.cpp +++ b/src/coreclr/jit/liveness.cpp @@ -2148,7 +2148,15 @@ bool Compiler::fgTryRemoveNonLocal(GenTree* node, LIR::Range* blockRange) DISPNODE(node); node->VisitOperands([](GenTree* operand) -> GenTree::VisitResult { - operand->SetUnusedValue(); + if (!operand->IsValue()) + { + // Child sets flags, which are consumed by the parent. + assert(operand->gtSetFlags()); + } + else + { + operand->SetUnusedValue(); + } return GenTree::VisitResult::Continue; }); diff --git a/src/coreclr/jit/lower.cpp b/src/coreclr/jit/lower.cpp index db81ecb0f7d301..dfce01d3e91b82 100644 --- a/src/coreclr/jit/lower.cpp +++ b/src/coreclr/jit/lower.cpp @@ -238,6 +238,16 @@ GenTree* Lowering::LowerNode(GenTree* node) case GT_JTRUE: return LowerJTrue(node->AsOp()); + case GT_SELECT: + case GT_CEQ: + case GT_CNE: + case GT_CLT: + case GT_CLE: + case GT_CGE: + case GT_CGT: + ContainCheckConditional(node->AsConditional()); + break; + case GT_JMP: LowerJmpMethod(node); break; @@ -2836,7 +2846,8 @@ GenTree* Lowering::OptimizeConstCompare(GenTree* cmp) if ((op2Value == 1) && cmp->OperIs(GT_EQ)) { if (andOp2->IsIntegralConst(1) && (genActualType(op1) == cmp->TypeGet()) && - BlockRange().TryGetUse(cmp, &cmpUse) && !cmpUse.User()->OperIs(GT_JTRUE)) + BlockRange().TryGetUse(cmp, &cmpUse) && !cmpUse.User()->OperIs(GT_JTRUE) && + !cmpUse.User()->OperIsConditional()) { GenTree* next = cmp->gtNext; @@ -3095,49 +3106,53 @@ GenTree* Lowering::LowerCompare(GenTree* cmp) GenTree* Lowering::LowerJTrue(GenTreeOp* jtrue) { #if defined(TARGET_ARM64) || defined(TARGET_LOONGARCH64) - GenTree* relop = jtrue->gtGetOp1(); - GenTree* relopOp2 = relop->AsOp()->gtGetOp2(); + GenTree* relop = jtrue->gtGetOp1(); - if ((relop->gtNext == jtrue) && relopOp2->IsCnsIntOrI()) + if (relop->OperIsCompare()) { - bool useJCMP = false; - GenTreeFlags flags = GTF_EMPTY; + GenTree* relopOp2 = relop->AsOp()->gtGetOp2(); -#if defined(TARGET_LOONGARCH64) - if (relop->OperIs(GT_EQ, GT_NE)) + if ((relop->gtNext == jtrue) && relopOp2->IsCnsIntOrI()) { - // Codegen will use beq or bne. - flags = relop->OperIs(GT_EQ) ? GTF_JCMP_EQ : GTF_EMPTY; - useJCMP = true; - } + bool useJCMP = false; + GenTreeFlags flags = GTF_EMPTY; + +#if defined(TARGET_LOONGARCH64) + if (relop->OperIs(GT_EQ, GT_NE)) + { + // Codegen will use beq or bne. + flags = relop->OperIs(GT_EQ) ? GTF_JCMP_EQ : GTF_EMPTY; + useJCMP = true; + } #else // TARGET_ARM64 - if (relop->OperIs(GT_EQ, GT_NE) && relopOp2->IsIntegralConst(0)) - { - // Codegen will use cbz or cbnz in codegen which do not affect the flag register - flags = relop->OperIs(GT_EQ) ? GTF_JCMP_EQ : GTF_EMPTY; - useJCMP = true; - } - else if (relop->OperIs(GT_TEST_EQ, GT_TEST_NE) && isPow2(relopOp2->AsIntCon()->IconValue())) - { - // Codegen will use tbz or tbnz in codegen which do not affect the flag register - flags = GTF_JCMP_TST | (relop->OperIs(GT_TEST_EQ) ? GTF_JCMP_EQ : GTF_EMPTY); - useJCMP = true; - } + if (relop->OperIs(GT_EQ, GT_NE) && relopOp2->IsIntegralConst(0)) + { + // Codegen will use cbz or cbnz in codegen which do not affect the flag register + flags = relop->OperIs(GT_EQ) ? GTF_JCMP_EQ : GTF_EMPTY; + useJCMP = true; + } + else if (relop->OperIs(GT_TEST_EQ, GT_TEST_NE) && isPow2(relopOp2->AsIntCon()->IconValue())) + { + // Codegen will use tbz or tbnz in codegen which do not affect the flag register + flags = GTF_JCMP_TST | (relop->OperIs(GT_TEST_EQ) ? GTF_JCMP_EQ : GTF_EMPTY); + useJCMP = true; + } #endif // TARGET_ARM64 - if (useJCMP) - { - relop->SetOper(GT_JCMP); - relop->gtFlags &= ~(GTF_JCMP_TST | GTF_JCMP_EQ); - relop->gtFlags |= flags; - relop->gtType = TYP_VOID; + if (useJCMP) + { + relop->SetOper(GT_JCMP); + relop->gtFlags &= ~(GTF_JCMP_TST | GTF_JCMP_EQ); + relop->gtFlags |= flags; + relop->gtType = TYP_VOID; - relopOp2->SetContained(); + relopOp2->SetContained(); - BlockRange().Remove(jtrue); + BlockRange().Remove(jtrue); - assert(relop->gtNext == nullptr); - return nullptr; + assert(relop->gtNext == nullptr); + return nullptr; + } } } #endif // TARGET_ARM64 || TARGET_LOONGARCH64 @@ -3164,7 +3179,7 @@ GenTree* Lowering::LowerJTrue(GenTreeOp* jtrue) // It's the caller's responsibility to change `node` such that it only // sets the condition flags, without producing a boolean value. // -GenTreeCC* Lowering::LowerNodeCC(GenTree* node, GenCondition condition) +GenTree* Lowering::LowerNodeCC(GenTree* node, GenCondition condition) { // Skip over a chain of EQ/NE(x, 0) relops. This may be present either // because `node` is not a relop and so it cannot be used directly by a @@ -3198,7 +3213,7 @@ GenTreeCC* Lowering::LowerNodeCC(GenTree* node, GenCondition condition) } } - GenTreeCC* cc = nullptr; + GenTree* cc = nullptr; // Next may be null if `node` is not used. In that case we don't need to generate a SETCC node. if (next != nullptr) @@ -3215,8 +3230,8 @@ GenTreeCC* Lowering::LowerNodeCC(GenTree* node, GenCondition condition) assert(relop->OperIsCompare()); next->ChangeOper(GT_JCC); - cc = next->AsCC(); - cc->gtCondition = condition; + cc = next; + cc->AsCC()->gtCondition = condition; } } else @@ -3227,8 +3242,18 @@ GenTreeCC* Lowering::LowerNodeCC(GenTree* node, GenCondition condition) if (BlockRange().TryGetUse(relop, &use)) { - cc = new (comp, GT_SETCC) GenTreeCC(GT_SETCC, condition, TYP_INT); - BlockRange().InsertAfter(node, cc); + if (use.User()->OperIsConditional()) + { + // Don't replace if the use is a conditional (Ideally GTF_SET_FLAGS + // would have been set on the node). + cc = (GenTreeCC*)node; + node->gtType = TYP_VOID; + } + else + { + cc = new (comp, GT_SETCC) GenTreeCC(GT_SETCC, condition, TYP_INT); + BlockRange().InsertAfter(node, cc); + } use.ReplaceWith(cc); } } @@ -6814,6 +6839,16 @@ void Lowering::ContainCheckNode(GenTree* node) ContainCheckJTrue(node->AsOp()); break; + case GT_SELECT: + case GT_CEQ: + case GT_CNE: + case GT_CLT: + case GT_CLE: + case GT_CGE: + case GT_CGT: + ContainCheckConditional(node->AsConditional()); + break; + case GT_ADD: case GT_SUB: #if !defined(TARGET_64BIT) @@ -7004,6 +7039,26 @@ void Lowering::ContainCheckJTrue(GenTreeOp* node) cmp->gtFlags |= GTF_SET_FLAGS; } +//------------------------------------------------------------------------ +// ContainCheckConditional : determine whether the source of a conditional should be contained. +// +// Arguments: +// node - pointer to the node +// +void Lowering::ContainCheckConditional(GenTreeConditional* node) +{ + // The compare does not need to be generated into a register. + GenTree* cmp = node->gtCond; + assert(cmp->OperIsCompare() || cmp->OperIsConditionalCompare()); + cmp->gtType = TYP_VOID; + cmp->gtFlags |= GTF_SET_FLAGS; + + if (node->gtOper != GT_SELECT) + { + CheckImmedAndMakeContained(node, node->gtOp2); + } +} + //------------------------------------------------------------------------ // ContainCheckBitCast: determine whether the source of a BITCAST should be contained. // diff --git a/src/coreclr/jit/lower.h b/src/coreclr/jit/lower.h index b8b5686133efc2..720be68cd3abc9 100644 --- a/src/coreclr/jit/lower.h +++ b/src/coreclr/jit/lower.h @@ -85,7 +85,7 @@ class Lowering final : public Phase void ContainCheckLclHeap(GenTreeOp* node); void ContainCheckRet(GenTreeUnOp* ret); void ContainCheckJTrue(GenTreeOp* node); - + void ContainCheckConditional(GenTreeConditional* node); void ContainCheckBitCast(GenTree* node); void ContainCheckCallOperands(GenTreeCall* call); void ContainCheckIndir(GenTreeIndir* indirNode); @@ -133,7 +133,7 @@ class Lowering final : public Phase GenTree* OptimizeConstCompare(GenTree* cmp); GenTree* LowerCompare(GenTree* cmp); GenTree* LowerJTrue(GenTreeOp* jtrue); - GenTreeCC* LowerNodeCC(GenTree* node, GenCondition condition); + GenTree* LowerNodeCC(GenTree* node, GenCondition condition); void LowerJmpMethod(GenTree* jmp); void LowerRet(GenTreeUnOp* ret); void LowerStoreLocCommon(GenTreeLclVarCommon* lclVar); diff --git a/src/coreclr/jit/lowerarmarch.cpp b/src/coreclr/jit/lowerarmarch.cpp index 1e2420b37a953b..a8fca93bdf5046 100644 --- a/src/coreclr/jit/lowerarmarch.cpp +++ b/src/coreclr/jit/lowerarmarch.cpp @@ -136,6 +136,14 @@ bool Lowering::IsContainableImmed(GenTree* parentNode, GenTree* childNode) const if (immVal == 0) return true; break; + + case GT_CEQ: + case GT_CNE: + case GT_CLT: + case GT_CLE: + case GT_CGE: + case GT_CGT: + return emitter::emitIns_valid_imm_for_ccmp(immVal); #endif default: diff --git a/src/coreclr/jit/lowerxarch.cpp b/src/coreclr/jit/lowerxarch.cpp index 5f2e384daa7ce7..002e5aea8dd015 100644 --- a/src/coreclr/jit/lowerxarch.cpp +++ b/src/coreclr/jit/lowerxarch.cpp @@ -808,15 +808,21 @@ void Lowering::LowerCast(GenTree* tree) // void Lowering::LowerHWIntrinsicCC(GenTreeHWIntrinsic* node, NamedIntrinsic newIntrinsicId, GenCondition condition) { - GenTreeCC* cc = LowerNodeCC(node, condition); + GenTree* cc = LowerNodeCC(node, condition); + + if (!cc->OperIsCC()) + { + return; + } assert(HWIntrinsicInfo::lookupNumArgs(newIntrinsicId) == 2); node->ChangeHWIntrinsicId(newIntrinsicId); node->gtType = TYP_VOID; node->ClearUnusedValue(); - bool swapOperands = false; - bool canSwapOperands = false; + bool swapOperands = false; + bool canSwapOperands = false; + GenCondition ccCondition = cc->AsCC()->gtCondition; switch (newIntrinsicId) { @@ -836,20 +842,20 @@ void Lowering::LowerHWIntrinsicCC(GenTreeHWIntrinsic* node, NamedIntrinsic newIn // containment. // - Allow swapping for containment purposes only if this doesn't result in a non-"preferred" // condition being generated. - if ((cc != nullptr) && cc->gtCondition.PreferSwap()) + if ((cc != nullptr) && ccCondition.PreferSwap()) { swapOperands = true; } else { - canSwapOperands = (cc == nullptr) || !GenCondition::Swap(cc->gtCondition).PreferSwap(); + canSwapOperands = (cc == nullptr) || !GenCondition::Swap(ccCondition).PreferSwap(); } break; case NI_SSE41_PTEST: case NI_AVX_PTEST: // If we need the Carry flag then we can't swap operands. - canSwapOperands = (cc == nullptr) || cc->gtCondition.Is(GenCondition::EQ, GenCondition::NE); + canSwapOperands = (cc == nullptr) || ccCondition.Is(GenCondition::EQ, GenCondition::NE); break; default: @@ -875,7 +881,7 @@ void Lowering::LowerHWIntrinsicCC(GenTreeHWIntrinsic* node, NamedIntrinsic newIn if (cc != nullptr) { - cc->gtCondition = GenCondition::Swap(cc->gtCondition); + ccCondition = GenCondition::Swap(ccCondition); } } } diff --git a/src/coreclr/jit/lsraarm64.cpp b/src/coreclr/jit/lsraarm64.cpp index 21134d2dab42a5..d92504e2b3ac1a 100644 --- a/src/coreclr/jit/lsraarm64.cpp +++ b/src/coreclr/jit/lsraarm64.cpp @@ -792,6 +792,24 @@ int LinearScan::BuildNode(GenTree* tree) BuildDef(tree); break; + case GT_SELECT: + assert(dstCount == 1); + FALLTHROUGH; + case GT_CEQ: + case GT_CNE: + case GT_CLT: + case GT_CLE: + case GT_CGE: + case GT_CGT: + // Don't build a use for the conditional. + srcCount = BuildOperandUses(tree->AsConditional()->gtOp1); + srcCount += BuildOperandUses(tree->AsConditional()->gtOp2); + if (tree->TypeGet() != TYP_VOID) + { + BuildDef(tree, dstCandidates); + } + break; + } // end switch (tree->OperGet()) if (tree->IsUnusedValue() && (dstCount != 0)) From e9c047432bdac893506d9f0778fa8f7072a6912d Mon Sep 17 00:00:00 2001 From: Alan Hayward Date: Fri, 8 Jul 2022 11:36:29 +0100 Subject: [PATCH 02/26] Contain conditionals --- src/coreclr/jit/codegenarm64.cpp | 29 +++++++++++- src/coreclr/jit/codegenlinear.cpp | 13 +++++ src/coreclr/jit/gentree.cpp | 10 +--- src/coreclr/jit/gentree.h | 2 +- src/coreclr/jit/lir.cpp | 4 +- src/coreclr/jit/liveness.cpp | 10 +--- src/coreclr/jit/lower.cpp | 79 +++++++++++++++---------------- src/coreclr/jit/lsraarm64.cpp | 9 ++-- src/coreclr/jit/lsrabuild.cpp | 13 +++++ 9 files changed, 100 insertions(+), 69 deletions(-) diff --git a/src/coreclr/jit/codegenarm64.cpp b/src/coreclr/jit/codegenarm64.cpp index 13328168ac551b..3202e7d8a938b0 100644 --- a/src/coreclr/jit/codegenarm64.cpp +++ b/src/coreclr/jit/codegenarm64.cpp @@ -4432,6 +4432,26 @@ void CodeGen::genCodeForConditional(GenTreeConditional* tree) assert(!op1->isUsedFromMemory()); assert(genTypeSize(op1Type) == genTypeSize(op2Type)); + if (opcond->isContained()) + { + // Generate the code for the condition + if (opcond->OperIsCompare()) + { + genCodeForCompare(opcond->AsOp()); + } + else + { + assert(opcond->OperIsConditionalCompare()); + genCodeForConditional(opcond->AsConditional()); + } + } + else + { + // Get the result of the condition into the condition flags. + emit->emitIns_R_I(INS_cmp, EA_ATTR(genTypeSize(opcond)), opcond->GetRegNum(), 1); + cond = INS_COND_EQ; + } + regNumber targetReg = tree->GetRegNum(); regNumber srcReg1 = genConsumeReg(op1); @@ -4446,8 +4466,6 @@ void CodeGen::genCodeForConditional(GenTreeConditional* tree) assert(!varTypeIsFloating(op2Type)); // We don't support swapping op1 and op2 to generate cmp reg, imm. assert(!op1->isContainedIntOrIImmed()); - // This should not be generating into a register. - assert(targetReg == REG_NA); // For the ccmp flags, get the condition of the compare. insCflags cflags = InsCflagsForCcmp(InsCondForCompareOp(tree)); @@ -4462,6 +4480,13 @@ void CodeGen::genCodeForConditional(GenTreeConditional* tree) regNumber srcReg2 = genConsumeReg(op2); emit->emitIns_R_R_FLAGS_COND(INS_ccmp, cmpSize, srcReg1, srcReg2, cflags, cond); } + + // Are we evaluating this into a register? + if (targetReg != REG_NA) + { + inst_SETCC(GenCondition::FromRelop(tree), tree->TypeGet(), targetReg); + genProduceReg(tree); + } } } diff --git a/src/coreclr/jit/codegenlinear.cpp b/src/coreclr/jit/codegenlinear.cpp index 8045344441020f..bbceba031e6cbe 100644 --- a/src/coreclr/jit/codegenlinear.cpp +++ b/src/coreclr/jit/codegenlinear.cpp @@ -1629,6 +1629,19 @@ void CodeGen::genConsumeRegs(GenTree* tree) assert(cast->isContained()); genConsumeAddress(cast->CastOp()); } + else if (tree->OperIsCompare()) + { + // Compares may be contained by a conditional. + genConsumeRegs(tree->gtGetOp1()); + genConsumeRegs(tree->gtGetOp2()); + } + else if (tree->OperIsConditionalCompare()) + { + // Conditional compares should always be contained. + genConsumeRegs(tree->AsConditional()->gtCond); + genConsumeRegs(tree->gtGetOp1()); + genConsumeRegs(tree->gtGetOp2()); + } #endif else if (tree->OperIsLocalRead()) { diff --git a/src/coreclr/jit/gentree.cpp b/src/coreclr/jit/gentree.cpp index 864d7227b5f421..ccefcbd8f7b097 100644 --- a/src/coreclr/jit/gentree.cpp +++ b/src/coreclr/jit/gentree.cpp @@ -12511,7 +12511,7 @@ void Compiler::gtDispLIRNode(GenTree* node, const char* prefixMsg /* = nullptr * IndentInfo operandArc = IIArcTop; for (GenTree* operand : node->Operands()) { - if (!operand->IsValue() && (operand->gtFlags & GTF_SET_FLAGS) == 0) + if (!operand->IsValue()) { // Either of these situations may happen with calls. continue; @@ -16840,14 +16840,6 @@ bool GenTree::isContained() const assert(!isMarkedContained); } - // these actually produce a register (the flags reg, we just don't model it) - // and are a separate instruction from the branch that consumes the result. - // They can only produce a result if the child is a SIMD equality comparison. - else if (OperIsCompare()) - { - assert(isMarkedContained == false); - } - // if it's contained it can't be unused. if (isMarkedContained) { diff --git a/src/coreclr/jit/gentree.h b/src/coreclr/jit/gentree.h index 44e6fe9f32e41b..d8ea5a832468e3 100644 --- a/src/coreclr/jit/gentree.h +++ b/src/coreclr/jit/gentree.h @@ -8234,7 +8234,7 @@ struct GenCondition { assert(relop->OperIsCompare() || relop->OperIsConditionalCompare()); - if (varTypeIsFloating(relop->gtGetOp1())) + if (relop->OperIsCompare() && varTypeIsFloating(relop->gtGetOp1())) { return FromFloatRelop(relop); } diff --git a/src/coreclr/jit/lir.cpp b/src/coreclr/jit/lir.cpp index 8a5801d0647105..5adef46eff7930 100644 --- a/src/coreclr/jit/lir.cpp +++ b/src/coreclr/jit/lir.cpp @@ -1588,9 +1588,9 @@ bool LIR::Range::CheckLIR(Compiler* compiler, bool checkUnusedValues) const // Stack arguments do not produce a value, but they are considered children of the call. // It may be useful to remove these from being call operands, but that may also impact // other code that relies on being able to reach all the operands from a call node. - // The argument of a JTRUE or conditional doesn't produce a value (just sets a flag). + // The argument of a JTRUE doesn't produce a value (just sets a flag). assert(((node->OperGet() == GT_CALL) && def->OperIs(GT_PUTARG_STK)) || - ((node->OperGet() == GT_JTRUE || node->OperIsConditional()) && (def->TypeGet() == TYP_VOID) && + ((node->OperGet() == GT_JTRUE) && (def->TypeGet() == TYP_VOID) && ((def->gtFlags & GTF_SET_FLAGS) != 0))); continue; } diff --git a/src/coreclr/jit/liveness.cpp b/src/coreclr/jit/liveness.cpp index 4b8062838d2af3..ddbdb586bc0159 100644 --- a/src/coreclr/jit/liveness.cpp +++ b/src/coreclr/jit/liveness.cpp @@ -2148,15 +2148,7 @@ bool Compiler::fgTryRemoveNonLocal(GenTree* node, LIR::Range* blockRange) DISPNODE(node); node->VisitOperands([](GenTree* operand) -> GenTree::VisitResult { - if (!operand->IsValue()) - { - // Child sets flags, which are consumed by the parent. - assert(operand->gtSetFlags()); - } - else - { - operand->SetUnusedValue(); - } + operand->SetUnusedValue(); return GenTree::VisitResult::Continue; }); diff --git a/src/coreclr/jit/lower.cpp b/src/coreclr/jit/lower.cpp index dfce01d3e91b82..8f2bad2ec25cef 100644 --- a/src/coreclr/jit/lower.cpp +++ b/src/coreclr/jit/lower.cpp @@ -3106,53 +3106,49 @@ GenTree* Lowering::LowerCompare(GenTree* cmp) GenTree* Lowering::LowerJTrue(GenTreeOp* jtrue) { #if defined(TARGET_ARM64) || defined(TARGET_LOONGARCH64) - GenTree* relop = jtrue->gtGetOp1(); + GenTree* relop = jtrue->gtGetOp1(); + GenTree* relopOp2 = relop->AsOp()->gtGetOp2(); - if (relop->OperIsCompare()) + if ((relop->gtNext == jtrue) && relopOp2->IsCnsIntOrI()) { - GenTree* relopOp2 = relop->AsOp()->gtGetOp2(); - - if ((relop->gtNext == jtrue) && relopOp2->IsCnsIntOrI()) - { - bool useJCMP = false; - GenTreeFlags flags = GTF_EMPTY; + bool useJCMP = false; + GenTreeFlags flags = GTF_EMPTY; #if defined(TARGET_LOONGARCH64) - if (relop->OperIs(GT_EQ, GT_NE)) - { - // Codegen will use beq or bne. - flags = relop->OperIs(GT_EQ) ? GTF_JCMP_EQ : GTF_EMPTY; - useJCMP = true; - } + if (relop->OperIs(GT_EQ, GT_NE)) + { + // Codegen will use beq or bne. + flags = relop->OperIs(GT_EQ) ? GTF_JCMP_EQ : GTF_EMPTY; + useJCMP = true; + } #else // TARGET_ARM64 - if (relop->OperIs(GT_EQ, GT_NE) && relopOp2->IsIntegralConst(0)) - { - // Codegen will use cbz or cbnz in codegen which do not affect the flag register - flags = relop->OperIs(GT_EQ) ? GTF_JCMP_EQ : GTF_EMPTY; - useJCMP = true; - } - else if (relop->OperIs(GT_TEST_EQ, GT_TEST_NE) && isPow2(relopOp2->AsIntCon()->IconValue())) - { - // Codegen will use tbz or tbnz in codegen which do not affect the flag register - flags = GTF_JCMP_TST | (relop->OperIs(GT_TEST_EQ) ? GTF_JCMP_EQ : GTF_EMPTY); - useJCMP = true; - } + if (relop->OperIs(GT_EQ, GT_NE) && relopOp2->IsIntegralConst(0)) + { + // Codegen will use cbz or cbnz in codegen which do not affect the flag register + flags = relop->OperIs(GT_EQ) ? GTF_JCMP_EQ : GTF_EMPTY; + useJCMP = true; + } + else if (relop->OperIs(GT_TEST_EQ, GT_TEST_NE) && isPow2(relopOp2->AsIntCon()->IconValue())) + { + // Codegen will use tbz or tbnz in codegen which do not affect the flag register + flags = GTF_JCMP_TST | (relop->OperIs(GT_TEST_EQ) ? GTF_JCMP_EQ : GTF_EMPTY); + useJCMP = true; + } #endif // TARGET_ARM64 - if (useJCMP) - { - relop->SetOper(GT_JCMP); - relop->gtFlags &= ~(GTF_JCMP_TST | GTF_JCMP_EQ); - relop->gtFlags |= flags; - relop->gtType = TYP_VOID; + if (useJCMP) + { + relop->SetOper(GT_JCMP); + relop->gtFlags &= ~(GTF_JCMP_TST | GTF_JCMP_EQ); + relop->gtFlags |= flags; + relop->gtType = TYP_VOID; - relopOp2->SetContained(); + relopOp2->SetContained(); - BlockRange().Remove(jtrue); + BlockRange().Remove(jtrue); - assert(relop->gtNext == nullptr); - return nullptr; - } + assert(relop->gtNext == nullptr); + return nullptr; } } #endif // TARGET_ARM64 || TARGET_LOONGARCH64 @@ -7047,12 +7043,15 @@ void Lowering::ContainCheckJTrue(GenTreeOp* node) // void Lowering::ContainCheckConditional(GenTreeConditional* node) { - // The compare does not need to be generated into a register. + // Check if the compare does not need to be generated into a register. GenTree* cmp = node->gtCond; assert(cmp->OperIsCompare() || cmp->OperIsConditionalCompare()); - cmp->gtType = TYP_VOID; - cmp->gtFlags |= GTF_SET_FLAGS; + if (IsSafeToContainMem(node, cmp)) + { + cmp->SetContained(); + } + // Check if an immediate can be contained. if (node->gtOper != GT_SELECT) { CheckImmedAndMakeContained(node, node->gtOp2); diff --git a/src/coreclr/jit/lsraarm64.cpp b/src/coreclr/jit/lsraarm64.cpp index d92504e2b3ac1a..940249bbe8043c 100644 --- a/src/coreclr/jit/lsraarm64.cpp +++ b/src/coreclr/jit/lsraarm64.cpp @@ -801,13 +801,10 @@ int LinearScan::BuildNode(GenTree* tree) case GT_CLE: case GT_CGE: case GT_CGT: - // Don't build a use for the conditional. - srcCount = BuildOperandUses(tree->AsConditional()->gtOp1); + srcCount = BuildOperandUses(tree->AsConditional()->gtCond); + srcCount += BuildOperandUses(tree->AsConditional()->gtOp1); srcCount += BuildOperandUses(tree->AsConditional()->gtOp2); - if (tree->TypeGet() != TYP_VOID) - { - BuildDef(tree, dstCandidates); - } + BuildDef(tree, dstCandidates); break; } // end switch (tree->OperGet()) diff --git a/src/coreclr/jit/lsrabuild.cpp b/src/coreclr/jit/lsrabuild.cpp index 9f01df462e978b..e848218f24ed91 100644 --- a/src/coreclr/jit/lsrabuild.cpp +++ b/src/coreclr/jit/lsrabuild.cpp @@ -3153,6 +3153,19 @@ int LinearScan::BuildOperandUses(GenTree* node, regMaskTP candidates) // GT_CAST and GT_LSH for ADD with sign/zero extension return BuildOperandUses(node->gtGetOp1(), candidates); } + if (node->OperIsCompare()) + { + // Compares may be contained by a conditional. + return BuildBinaryUses(node->AsOp(), candidates); + } + if (node->OperIsConditionalCompare()) + { + // Conditional compares should always be contained. + int uses = BuildOperandUses(node->AsConditional()->gtCond, candidates); + uses += BuildOperandUses(node->AsConditional()->gtOp1, candidates); + uses += BuildOperandUses(node->AsConditional()->gtOp2, candidates); + return uses; + } #endif return 0; From 97386b31e1c8297738d6f1d1142d4f58dedd0c58 Mon Sep 17 00:00:00 2001 From: Alan Hayward Date: Mon, 11 Jul 2022 12:10:49 +0100 Subject: [PATCH 03/26] Fix formatting --- src/coreclr/jit/lower.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/coreclr/jit/lower.cpp b/src/coreclr/jit/lower.cpp index 8f2bad2ec25cef..c985f3f5c9321e 100644 --- a/src/coreclr/jit/lower.cpp +++ b/src/coreclr/jit/lower.cpp @@ -7048,7 +7048,7 @@ void Lowering::ContainCheckConditional(GenTreeConditional* node) assert(cmp->OperIsCompare() || cmp->OperIsConditionalCompare()); if (IsSafeToContainMem(node, cmp)) { - cmp->SetContained(); + cmp->SetContained(); } // Check if an immediate can be contained. From 1bcd61a43f847ba89789f22ec55a165b8e8aeed8 Mon Sep 17 00:00:00 2001 From: Alan Hayward Date: Tue, 12 Jul 2022 14:26:17 +0100 Subject: [PATCH 04/26] Use AND and CMP nodes --- src/coreclr/jit/codegen.h | 4 +- src/coreclr/jit/codegenarm64.cpp | 182 +++++++++++++++++++++-------- src/coreclr/jit/codegenarmarch.cpp | 10 +- src/coreclr/jit/lower.cpp | 75 +++++++++--- src/coreclr/jit/lower.h | 3 +- src/coreclr/jit/lowerarmarch.cpp | 11 ++ src/coreclr/jit/lsrabuild.cpp | 12 +- 7 files changed, 222 insertions(+), 75 deletions(-) diff --git a/src/coreclr/jit/codegen.h b/src/coreclr/jit/codegen.h index fc9aae292d5e96..8ee61a5b8309b5 100644 --- a/src/coreclr/jit/codegen.h +++ b/src/coreclr/jit/codegen.h @@ -1047,7 +1047,9 @@ XXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXX void genCkfinite(GenTree* treeNode); void genCodeForCompare(GenTreeOp* tree); #ifdef TARGET_ARM64 - void genCodeForConditional(GenTreeConditional* tree); + void genCodeForConditionalCompare(GenTreeOp* tree, insCond cond); + void genCodeForContainedCompareChain(GenTreeOp* tree, bool *inchain, insCond *prevcond); + void genCodeForSelect(GenTreeConditional* tree); #endif void genIntrinsic(GenTree* treeNode); void genPutArgStk(GenTreePutArgStk* treeNode); diff --git a/src/coreclr/jit/codegenarm64.cpp b/src/coreclr/jit/codegenarm64.cpp index 3202e7d8a938b0..848ae8ec8cb3e2 100644 --- a/src/coreclr/jit/codegenarm64.cpp +++ b/src/coreclr/jit/codegenarm64.cpp @@ -4412,84 +4412,168 @@ void CodeGen::genCodeForCompare(GenTreeOp* tree) } //------------------------------------------------------------------------ -// genCodeForCompare: Produce code for a GT_CEQ/GT_CNE node. +// genCodeForConditionalCompare: Produce code for a compare that's dependent on a previous compare. // // Arguments: -// tree - the node +// tree - a compare node (GT_EQ etc) +// cond - the condition of the previous generated compare. // -void CodeGen::genCodeForConditional(GenTreeConditional* tree) +void CodeGen::genCodeForConditionalCompare(GenTreeOp* tree, insCond cond) { emitter* emit = GetEmitter(); - GenTree* opcond = tree->gtCond; - GenTree* op1 = tree->gtOp1; - GenTree* op2 = tree->gtOp2; - var_types op1Type = genActualType(op1->TypeGet()); - var_types op2Type = genActualType(op2->TypeGet()); - emitAttr cmpSize = EA_ATTR(genTypeSize(op1Type)); - insCond cond = InsCondForCompareOp(opcond); + GenTree* op1 = tree->gtGetOp1(); + GenTree* op2 = tree->gtGetOp2(); + var_types op1Type = genActualType(op1->TypeGet()); + var_types op2Type = genActualType(op2->TypeGet()); + emitAttr cmpSize = EA_ATTR(genTypeSize(op1Type)); + regNumber targetReg = tree->GetRegNum(); + regNumber srcReg1 = genConsumeReg(op1); - assert(!op1->isUsedFromMemory()); - assert(genTypeSize(op1Type) == genTypeSize(op2Type)); + // No float support or swapping op1 and op2 to generate cmp reg, imm. + assert(!varTypeIsFloating(op2Type)); + assert(!op1->isContainedIntOrIImmed()); - if (opcond->isContained()) + // For the ccmp flags, invert the condition of the compare. + insCflags cflags = InsCflagsForCcmp(InsCondForCompareOp(tree)); + + if (op2->isContainedIntOrIImmed()) { - // Generate the code for the condition - if (opcond->OperIsCompare()) - { - genCodeForCompare(opcond->AsOp()); - } - else - { - assert(opcond->OperIsConditionalCompare()); - genCodeForConditional(opcond->AsConditional()); - } + GenTreeIntConCommon* intConst = op2->AsIntConCommon(); + emit->emitIns_R_I_FLAGS_COND(INS_ccmp, cmpSize, srcReg1, (int)intConst->IconValue(), cflags, cond); } else { - // Get the result of the condition into the condition flags. - emit->emitIns_R_I(INS_cmp, EA_ATTR(genTypeSize(opcond)), opcond->GetRegNum(), 1); - cond = INS_COND_EQ; + regNumber srcReg2 = genConsumeReg(op2); + emit->emitIns_R_R_FLAGS_COND(INS_ccmp, cmpSize, srcReg1, srcReg2, cflags, cond); } - regNumber targetReg = tree->GetRegNum(); - regNumber srcReg1 = genConsumeReg(op1); - - if (tree->OperIs(GT_SELECT)) + // Are we evaluating this into a register? + if (targetReg != REG_NA) { - regNumber srcReg2 = genConsumeReg(op2); - emit->emitIns_R_R_R_COND(INS_csel, cmpSize, targetReg, srcReg1, srcReg2, cond); - regSet.verifyRegUsed(targetReg); + inst_SETCC(GenCondition::FromRelop(tree), tree->TypeGet(), targetReg); + genProduceReg(tree); } - else - { - assert(!varTypeIsFloating(op2Type)); - // We don't support swapping op1 and op2 to generate cmp reg, imm. - assert(!op1->isContainedIntOrIImmed()); +} - // For the ccmp flags, get the condition of the compare. - insCflags cflags = InsCflagsForCcmp(InsCondForCompareOp(tree)); +//------------------------------------------------------------------------ +// genCodeForContainedCompareChain: Produce code for a chain of conditional compares. +// +// Only generates for contained nodes. Nodes that are not contained are assumed to be +// generated as part of standard tree generation. +// +// Arguments: +// tree - the node. Either a compare or a tree of compares connected by ANDs. +// inchain - whether a contained chain is in progress. +// prev - If a chain is in progress, the condition of the previous compare. +// Return: +// The last compare node generated. +// +void CodeGen::genCodeForContainedCompareChain(GenTreeOp* tree, bool *inchain, insCond *prevcond) +{ + if (tree->OperIs(GT_AND)) + { + GenTreeOp* op1 = tree->gtGetOp1()->AsOp(); + GenTreeOp* op2 = tree->gtGetOp2()->AsOp(); - if (op2->isContainedIntOrIImmed()) + if (!tree->isContained()) { - GenTreeIntConCommon* intConst = op2->AsIntConCommon(); - emit->emitIns_R_I_FLAGS_COND(INS_ccmp, cmpSize, srcReg1, (int)intConst->IconValue(), cflags, cond); + // An And that is not contained should not have any contained children. + assert(!op1->isContained() && !op2->isContained()); + *inchain = false; } else { - regNumber srcReg2 = genConsumeReg(op2); - emit->emitIns_R_R_FLAGS_COND(INS_ccmp, cmpSize, srcReg1, srcReg2, cflags, cond); + // An And can only be contained if Op2 is contained. + assert(op2->isContained()); + + // If Op1 is contained, generate into flags. Otherwise, move the result into flags. + if (op1->isContained()) + { + genCodeForContainedCompareChain(op1, inchain, prevcond); + assert(*inchain); + } + else + { + emitter* emit = GetEmitter(); + emit->emitIns_R_I(INS_cmp, EA_ATTR(genTypeSize(op1)), op1->GetRegNum(), 1); + *prevcond = INS_COND_EQ; + *inchain = true; + } + + //Generate Op2 based on Op1. + genCodeForContainedCompareChain(op2, inchain, prevcond); + assert(*inchain); } + } + else + { + assert(tree->OperIsCompare()); + if (tree->isContained()) + { + // Generate the compare, putting the result in the flags register. + if (!*inchain) + { + // First item in a chain. Use a standard compare. + genCodeForCompare(tree); + } + else + { + // Within the chain. Use a conditional compare (which is + // dependent on the previous emitted compare). + genCodeForConditionalCompare(tree, *prevcond); + } - // Are we evaluating this into a register? - if (targetReg != REG_NA) + *inchain = true; + *prevcond = InsCondForCompareOp(tree); + } + else { - inst_SETCC(GenCondition::FromRelop(tree), tree->TypeGet(), targetReg); - genProduceReg(tree); + *inchain = false; } } } +//------------------------------------------------------------------------ +// genCodeForSelect: Produce code for a GT_SELECT node. +// +// Arguments: +// tree - the node +// +void CodeGen::genCodeForSelect(GenTreeConditional* tree) +{ + emitter* emit = GetEmitter(); + + GenTree* opcond = tree->gtCond; + GenTree* op1 = tree->gtOp1; + GenTree* op2 = tree->gtOp2; + var_types op1Type = genActualType(op1->TypeGet()); + var_types op2Type = genActualType(op2->TypeGet()); + emitAttr cmpSize = EA_ATTR(genTypeSize(op1Type)); + + assert(!op1->isUsedFromMemory()); + assert(genTypeSize(op1Type) == genTypeSize(op2Type)); + + // Generate the condition. + bool chain = false; + insCond cond = INS_COND_EQ; // Dummy value. + genCodeForContainedCompareChain(opcond->AsOp(), &chain, &cond); + assert(chain == opcond->isContained()); + if (!opcond->isContained()) + { + // Node has been generated into a register - move into flags. + emit->emitIns_R_I(INS_cmp, EA_ATTR(genTypeSize(opcond)), opcond->GetRegNum(), 0); + cond = INS_COND_NE; + } + + regNumber targetReg = tree->GetRegNum(); + regNumber srcReg1 = genConsumeReg(op1); + regNumber srcReg2 = genConsumeReg(op2); + + emit->emitIns_R_R_R_COND(INS_csel, cmpSize, targetReg, srcReg1, srcReg2, cond); + regSet.verifyRegUsed(targetReg); +} + //------------------------------------------------------------------------ // genCodeForJumpCompare: Generates code for jmpCompare statement. // diff --git a/src/coreclr/jit/codegenarmarch.cpp b/src/coreclr/jit/codegenarmarch.cpp index 69c85482637811..3081e27a09580a 100644 --- a/src/coreclr/jit/codegenarmarch.cpp +++ b/src/coreclr/jit/codegenarmarch.cpp @@ -212,9 +212,15 @@ void CodeGen::genCodeForTreeNode(GenTree* treeNode) genCodeForDivMod(treeNode->AsOp()); break; + case GT_AND: +#if !defined(TARGET_64BIT) + // An And that is not contained should not have any contained children. + assert(!treeNode->AsOp()->gtOp1->isContained() && !treeNode->AsOp()->gtOp2->isContained()); + FALLTHROUGH; +#endif + case GT_OR: case GT_XOR: - case GT_AND: case GT_AND_NOT: assert(varTypeIsIntegralOrI(treeNode)); @@ -375,7 +381,7 @@ void CodeGen::genCodeForTreeNode(GenTree* treeNode) case GT_CLE: case GT_CGE: case GT_CGT: - genCodeForConditional(treeNode->AsConditional()); + genCodeForSelect(treeNode->AsConditional()); break; #endif diff --git a/src/coreclr/jit/lower.cpp b/src/coreclr/jit/lower.cpp index c985f3f5c9321e..7118993314a57f 100644 --- a/src/coreclr/jit/lower.cpp +++ b/src/coreclr/jit/lower.cpp @@ -245,7 +245,7 @@ GenTree* Lowering::LowerNode(GenTree* node) case GT_CLE: case GT_CGE: case GT_CGT: - ContainCheckConditional(node->AsConditional()); + ContainCheckSelect(node->AsConditional()); break; case GT_JMP: @@ -6842,7 +6842,7 @@ void Lowering::ContainCheckNode(GenTree* node) case GT_CLE: case GT_CGE: case GT_CGT: - ContainCheckConditional(node->AsConditional()); + ContainCheckSelect(node->AsConditional()); break; case GT_ADD: @@ -7036,25 +7036,72 @@ void Lowering::ContainCheckJTrue(GenTreeOp* node) } //------------------------------------------------------------------------ -// ContainCheckConditional : determine whether the source of a conditional should be contained. +// ContainCheckCompareChain : determine whether a chain of compares should be contained by the given parent. +// +// Work backwards through the chain until it is no longer valid or containable. // // Arguments: -// node - pointer to the node +// tree - the node. Either a compare or a tree of compares connected by ANDs. +// parent - the parent of the tree. +// earliest_valid - Returns the earliest valid compare node in the chain (if any). +// +// Return Value: +// True if the chain is valid // -void Lowering::ContainCheckConditional(GenTreeConditional* node) +bool Lowering::ContainCheckCompareChain(GenTree* tree, GenTree* parent, GenTree** earliest_valid) { - // Check if the compare does not need to be generated into a register. - GenTree* cmp = node->gtCond; - assert(cmp->OperIsCompare() || cmp->OperIsConditionalCompare()); - if (IsSafeToContainMem(node, cmp)) + if (tree->OperIs(GT_AND)) { - cmp->SetContained(); + // To ensure ordering at code generation, Op1 and the parent can + // only be contained if Op2 is contained. + if (ContainCheckCompareChain(tree->AsOp()->gtGetOp2(), tree, earliest_valid)) + { + ContainCheckCompareChain(tree->AsOp()->gtGetOp1(), tree, earliest_valid); + tree->SetContained(); + return true; + } } - - // Check if an immediate can be contained. - if (node->gtOper != GT_SELECT) + else if (tree->OperIsCompare()) { - CheckImmedAndMakeContained(node, node->gtOp2); + // Can the compare be contained. + if(IsSafeToContainMem(parent, tree)) + { + tree->AsOp()->SetContained(); + + // Ensure the children of the compare are contained correctly. + tree->AsOp()->gtGetOp1()->ClearContained(); + tree->AsOp()->gtGetOp2()->ClearContained(); + ContainCheckCompare(tree->AsOp()); + *earliest_valid = tree; + return true; + } + } + + //Not a valid compare chain + return false; +} + +//------------------------------------------------------------------------ +// ContainCheckSelect : determine whether the source of a select should be contained. +// +// Arguments: +// node - pointer to the node +// +void Lowering::ContainCheckSelect(GenTreeConditional* node) +{ + // Check if the compare does not need to be generated into a register. + GenTree* earliest_valid = nullptr; + ContainCheckCompareChain(node->gtCond, node, &earliest_valid); + + if (earliest_valid != nullptr) + { + // The earliest node in the chain will be generated as a standard compare. + // Temporary set as uncontained in order to correct its children. + earliest_valid->AsOp()->ClearContained(); + earliest_valid->AsOp()->gtGetOp1()->ClearContained(); + earliest_valid->AsOp()->gtGetOp2()->ClearContained(); + ContainCheckCompare(earliest_valid->AsOp()); + earliest_valid->AsOp()->SetContained(); } } diff --git a/src/coreclr/jit/lower.h b/src/coreclr/jit/lower.h index 720be68cd3abc9..240f86d0305995 100644 --- a/src/coreclr/jit/lower.h +++ b/src/coreclr/jit/lower.h @@ -85,7 +85,8 @@ class Lowering final : public Phase void ContainCheckLclHeap(GenTreeOp* node); void ContainCheckRet(GenTreeUnOp* ret); void ContainCheckJTrue(GenTreeOp* node); - void ContainCheckConditional(GenTreeConditional* node); + bool ContainCheckCompareChain(GenTree* tree, GenTree* parent, GenTree** earliest_valid); + void ContainCheckSelect(GenTreeConditional* node); void ContainCheckBitCast(GenTree* node); void ContainCheckCallOperands(GenTreeCall* call); void ContainCheckIndir(GenTreeIndir* indirNode); diff --git a/src/coreclr/jit/lowerarmarch.cpp b/src/coreclr/jit/lowerarmarch.cpp index a8fca93bdf5046..0e53996dcee11e 100644 --- a/src/coreclr/jit/lowerarmarch.cpp +++ b/src/coreclr/jit/lowerarmarch.cpp @@ -104,6 +104,12 @@ bool Lowering::IsContainableImmed(GenTree* parentNode, GenTree* childNode) const case GT_LE: case GT_GE: case GT_GT: + if (parentNode->isContained()) + { + // Contained node will be generated as a ccmp. + return emitter::emitIns_valid_imm_for_ccmp(immVal); + } + FALLTHROUGH; case GT_CMP: case GT_BOUNDS_CHECK: return emitter::emitIns_valid_imm_for_cmp(immVal, size); @@ -112,6 +118,11 @@ bool Lowering::IsContainableImmed(GenTree* parentNode, GenTree* childNode) const case GT_XOR: case GT_TEST_EQ: case GT_TEST_NE: + if (parentNode->isContained()) + { + // Contained node will be generated as a ccmp. + return emitter::emitIns_valid_imm_for_ccmp(immVal); + } return emitter::emitIns_valid_imm_for_alu(immVal, size); case GT_JCMP: assert(((parentNode->gtFlags & GTF_JCMP_TST) == 0) ? (immVal == 0) : isPow2(immVal)); diff --git a/src/coreclr/jit/lsrabuild.cpp b/src/coreclr/jit/lsrabuild.cpp index e848218f24ed91..f48bfc285ed9ab 100644 --- a/src/coreclr/jit/lsrabuild.cpp +++ b/src/coreclr/jit/lsrabuild.cpp @@ -3142,9 +3142,10 @@ int LinearScan::BuildOperandUses(GenTree* node, regMaskTP candidates) } #endif // FEATURE_HW_INTRINSICS #ifdef TARGET_ARM64 - if (node->OperIs(GT_MUL)) + if (node->OperIs(GT_MUL) || node->OperIsCompare() || node->OperIs(GT_AND)) { - // Can be contained for MultiplyAdd on arm64 + // Can be contained for MultiplyAdd on arm64. + // Compare and And may be contained due to If Conversion. return BuildBinaryUses(node->AsOp(), candidates); } if (node->OperIs(GT_NEG, GT_CAST, GT_LSH)) @@ -3153,14 +3154,9 @@ int LinearScan::BuildOperandUses(GenTree* node, regMaskTP candidates) // GT_CAST and GT_LSH for ADD with sign/zero extension return BuildOperandUses(node->gtGetOp1(), candidates); } - if (node->OperIsCompare()) - { - // Compares may be contained by a conditional. - return BuildBinaryUses(node->AsOp(), candidates); - } if (node->OperIsConditionalCompare()) { - // Conditional compares should always be contained. + // A conditional compare may be contained due to If Conversion. int uses = BuildOperandUses(node->AsConditional()->gtCond, candidates); uses += BuildOperandUses(node->AsConditional()->gtOp1, candidates); uses += BuildOperandUses(node->AsConditional()->gtOp2, candidates); From 7884eca89a00818ec11b9b603aa2957f3f2f558f Mon Sep 17 00:00:00 2001 From: Alan Hayward Date: Mon, 18 Jul 2022 17:08:25 +0100 Subject: [PATCH 05/26] Fix formatting --- src/coreclr/jit/codegen.h | 2 +- src/coreclr/jit/codegenarm64.cpp | 12 ++++++------ src/coreclr/jit/lower.cpp | 4 ++-- 3 files changed, 9 insertions(+), 9 deletions(-) diff --git a/src/coreclr/jit/codegen.h b/src/coreclr/jit/codegen.h index 8ee61a5b8309b5..d232acc5c75762 100644 --- a/src/coreclr/jit/codegen.h +++ b/src/coreclr/jit/codegen.h @@ -1048,7 +1048,7 @@ XXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXX void genCodeForCompare(GenTreeOp* tree); #ifdef TARGET_ARM64 void genCodeForConditionalCompare(GenTreeOp* tree, insCond cond); - void genCodeForContainedCompareChain(GenTreeOp* tree, bool *inchain, insCond *prevcond); + void genCodeForContainedCompareChain(GenTreeOp* tree, bool* inchain, insCond* prevcond); void genCodeForSelect(GenTreeConditional* tree); #endif void genIntrinsic(GenTree* treeNode); diff --git a/src/coreclr/jit/codegenarm64.cpp b/src/coreclr/jit/codegenarm64.cpp index 848ae8ec8cb3e2..773ab19e42e7b6 100644 --- a/src/coreclr/jit/codegenarm64.cpp +++ b/src/coreclr/jit/codegenarm64.cpp @@ -4469,7 +4469,7 @@ void CodeGen::genCodeForConditionalCompare(GenTreeOp* tree, insCond cond) // Return: // The last compare node generated. // -void CodeGen::genCodeForContainedCompareChain(GenTreeOp* tree, bool *inchain, insCond *prevcond) +void CodeGen::genCodeForContainedCompareChain(GenTreeOp* tree, bool* inchain, insCond* prevcond) { if (tree->OperIs(GT_AND)) { @@ -4498,10 +4498,10 @@ void CodeGen::genCodeForContainedCompareChain(GenTreeOp* tree, bool *inchain, in emitter* emit = GetEmitter(); emit->emitIns_R_I(INS_cmp, EA_ATTR(genTypeSize(op1)), op1->GetRegNum(), 1); *prevcond = INS_COND_EQ; - *inchain = true; + *inchain = true; } - //Generate Op2 based on Op1. + // Generate Op2 based on Op1. genCodeForContainedCompareChain(op2, inchain, prevcond); assert(*inchain); } @@ -4524,7 +4524,7 @@ void CodeGen::genCodeForContainedCompareChain(GenTreeOp* tree, bool *inchain, in genCodeForConditionalCompare(tree, *prevcond); } - *inchain = true; + *inchain = true; *prevcond = InsCondForCompareOp(tree); } else @@ -4555,8 +4555,8 @@ void CodeGen::genCodeForSelect(GenTreeConditional* tree) assert(genTypeSize(op1Type) == genTypeSize(op2Type)); // Generate the condition. - bool chain = false; - insCond cond = INS_COND_EQ; // Dummy value. + bool chain = false; + insCond cond = INS_COND_EQ; // Dummy value. genCodeForContainedCompareChain(opcond->AsOp(), &chain, &cond); assert(chain == opcond->isContained()); if (!opcond->isContained()) diff --git a/src/coreclr/jit/lower.cpp b/src/coreclr/jit/lower.cpp index 7118993314a57f..28ca530e3333d7 100644 --- a/src/coreclr/jit/lower.cpp +++ b/src/coreclr/jit/lower.cpp @@ -7064,7 +7064,7 @@ bool Lowering::ContainCheckCompareChain(GenTree* tree, GenTree* parent, GenTree* else if (tree->OperIsCompare()) { // Can the compare be contained. - if(IsSafeToContainMem(parent, tree)) + if (IsSafeToContainMem(parent, tree)) { tree->AsOp()->SetContained(); @@ -7077,7 +7077,7 @@ bool Lowering::ContainCheckCompareChain(GenTree* tree, GenTree* parent, GenTree* } } - //Not a valid compare chain + // Not a valid compare chain return false; } From 6d36f9d6fcb9714da2b6e334c9733e120e8037d6 Mon Sep 17 00:00:00 2001 From: Alan Hayward Date: Mon, 18 Jul 2022 17:39:00 +0100 Subject: [PATCH 06/26] Remove LowerNodeCC changes Change-Id: Icdf1828905c4a3f210b06e047ef3c0ac913d71f7 CustomizedGitHooks: yes --- src/coreclr/jit/lower.cpp | 22 ++++++---------------- src/coreclr/jit/lower.h | 2 +- src/coreclr/jit/lowerxarch.cpp | 20 +++++++------------- 3 files changed, 14 insertions(+), 30 deletions(-) diff --git a/src/coreclr/jit/lower.cpp b/src/coreclr/jit/lower.cpp index 28ca530e3333d7..710785ea15075a 100644 --- a/src/coreclr/jit/lower.cpp +++ b/src/coreclr/jit/lower.cpp @@ -3175,7 +3175,7 @@ GenTree* Lowering::LowerJTrue(GenTreeOp* jtrue) // It's the caller's responsibility to change `node` such that it only // sets the condition flags, without producing a boolean value. // -GenTree* Lowering::LowerNodeCC(GenTree* node, GenCondition condition) +GenTreeCC* Lowering::LowerNodeCC(GenTree* node, GenCondition condition) { // Skip over a chain of EQ/NE(x, 0) relops. This may be present either // because `node` is not a relop and so it cannot be used directly by a @@ -3209,7 +3209,7 @@ GenTree* Lowering::LowerNodeCC(GenTree* node, GenCondition condition) } } - GenTree* cc = nullptr; + GenTreeCC* cc = nullptr; // Next may be null if `node` is not used. In that case we don't need to generate a SETCC node. if (next != nullptr) @@ -3226,8 +3226,8 @@ GenTree* Lowering::LowerNodeCC(GenTree* node, GenCondition condition) assert(relop->OperIsCompare()); next->ChangeOper(GT_JCC); - cc = next; - cc->AsCC()->gtCondition = condition; + cc = next->AsCC(); + cc->gtCondition = condition; } } else @@ -3238,18 +3238,8 @@ GenTree* Lowering::LowerNodeCC(GenTree* node, GenCondition condition) if (BlockRange().TryGetUse(relop, &use)) { - if (use.User()->OperIsConditional()) - { - // Don't replace if the use is a conditional (Ideally GTF_SET_FLAGS - // would have been set on the node). - cc = (GenTreeCC*)node; - node->gtType = TYP_VOID; - } - else - { - cc = new (comp, GT_SETCC) GenTreeCC(GT_SETCC, condition, TYP_INT); - BlockRange().InsertAfter(node, cc); - } + cc = new (comp, GT_SETCC) GenTreeCC(GT_SETCC, condition, TYP_INT); + BlockRange().InsertAfter(node, cc); use.ReplaceWith(cc); } } diff --git a/src/coreclr/jit/lower.h b/src/coreclr/jit/lower.h index 240f86d0305995..f578767530b71f 100644 --- a/src/coreclr/jit/lower.h +++ b/src/coreclr/jit/lower.h @@ -134,7 +134,7 @@ class Lowering final : public Phase GenTree* OptimizeConstCompare(GenTree* cmp); GenTree* LowerCompare(GenTree* cmp); GenTree* LowerJTrue(GenTreeOp* jtrue); - GenTree* LowerNodeCC(GenTree* node, GenCondition condition); + GenTreeCC* LowerNodeCC(GenTree* node, GenCondition condition); void LowerJmpMethod(GenTree* jmp); void LowerRet(GenTreeUnOp* ret); void LowerStoreLocCommon(GenTreeLclVarCommon* lclVar); diff --git a/src/coreclr/jit/lowerxarch.cpp b/src/coreclr/jit/lowerxarch.cpp index 002e5aea8dd015..5f2e384daa7ce7 100644 --- a/src/coreclr/jit/lowerxarch.cpp +++ b/src/coreclr/jit/lowerxarch.cpp @@ -808,21 +808,15 @@ void Lowering::LowerCast(GenTree* tree) // void Lowering::LowerHWIntrinsicCC(GenTreeHWIntrinsic* node, NamedIntrinsic newIntrinsicId, GenCondition condition) { - GenTree* cc = LowerNodeCC(node, condition); - - if (!cc->OperIsCC()) - { - return; - } + GenTreeCC* cc = LowerNodeCC(node, condition); assert(HWIntrinsicInfo::lookupNumArgs(newIntrinsicId) == 2); node->ChangeHWIntrinsicId(newIntrinsicId); node->gtType = TYP_VOID; node->ClearUnusedValue(); - bool swapOperands = false; - bool canSwapOperands = false; - GenCondition ccCondition = cc->AsCC()->gtCondition; + bool swapOperands = false; + bool canSwapOperands = false; switch (newIntrinsicId) { @@ -842,20 +836,20 @@ void Lowering::LowerHWIntrinsicCC(GenTreeHWIntrinsic* node, NamedIntrinsic newIn // containment. // - Allow swapping for containment purposes only if this doesn't result in a non-"preferred" // condition being generated. - if ((cc != nullptr) && ccCondition.PreferSwap()) + if ((cc != nullptr) && cc->gtCondition.PreferSwap()) { swapOperands = true; } else { - canSwapOperands = (cc == nullptr) || !GenCondition::Swap(ccCondition).PreferSwap(); + canSwapOperands = (cc == nullptr) || !GenCondition::Swap(cc->gtCondition).PreferSwap(); } break; case NI_SSE41_PTEST: case NI_AVX_PTEST: // If we need the Carry flag then we can't swap operands. - canSwapOperands = (cc == nullptr) || ccCondition.Is(GenCondition::EQ, GenCondition::NE); + canSwapOperands = (cc == nullptr) || cc->gtCondition.Is(GenCondition::EQ, GenCondition::NE); break; default: @@ -881,7 +875,7 @@ void Lowering::LowerHWIntrinsicCC(GenTreeHWIntrinsic* node, NamedIntrinsic newIn if (cc != nullptr) { - ccCondition = GenCondition::Swap(ccCondition); + cc->gtCondition = GenCondition::Swap(cc->gtCondition); } } } From 1138faed14a265125ffe7b3263af18895992b9db Mon Sep 17 00:00:00 2001 From: Alan Hayward Date: Tue, 19 Jul 2022 13:34:39 +0100 Subject: [PATCH 07/26] Remove asserts & fix variable names --- src/coreclr/jit/codegenarm64.cpp | 26 +++++++++++++------------- src/coreclr/jit/codegenarmarch.cpp | 8 +------- 2 files changed, 14 insertions(+), 20 deletions(-) diff --git a/src/coreclr/jit/codegenarm64.cpp b/src/coreclr/jit/codegenarm64.cpp index 773ab19e42e7b6..b920c36a79e6c1 100644 --- a/src/coreclr/jit/codegenarm64.cpp +++ b/src/coreclr/jit/codegenarm64.cpp @@ -4464,12 +4464,12 @@ void CodeGen::genCodeForConditionalCompare(GenTreeOp* tree, insCond cond) // // Arguments: // tree - the node. Either a compare or a tree of compares connected by ANDs. -// inchain - whether a contained chain is in progress. +// inChain - whether a contained chain is in progress. // prev - If a chain is in progress, the condition of the previous compare. // Return: // The last compare node generated. // -void CodeGen::genCodeForContainedCompareChain(GenTreeOp* tree, bool* inchain, insCond* prevcond) +void CodeGen::genCodeForContainedCompareChain(GenTreeOp* tree, bool* inChain, insCond* prevcond) { if (tree->OperIs(GT_AND)) { @@ -4480,7 +4480,7 @@ void CodeGen::genCodeForContainedCompareChain(GenTreeOp* tree, bool* inchain, in { // An And that is not contained should not have any contained children. assert(!op1->isContained() && !op2->isContained()); - *inchain = false; + *inChain = false; } else { @@ -4490,20 +4490,20 @@ void CodeGen::genCodeForContainedCompareChain(GenTreeOp* tree, bool* inchain, in // If Op1 is contained, generate into flags. Otherwise, move the result into flags. if (op1->isContained()) { - genCodeForContainedCompareChain(op1, inchain, prevcond); - assert(*inchain); + genCodeForContainedCompareChain(op1, inChain, prevcond); + assert(*inChain); } else { emitter* emit = GetEmitter(); - emit->emitIns_R_I(INS_cmp, EA_ATTR(genTypeSize(op1)), op1->GetRegNum(), 1); - *prevcond = INS_COND_EQ; - *inchain = true; + emit->emitIns_R_I(INS_cmp, EA_ATTR(genTypeSize(op1)), op1->GetRegNum(), 0); + *prevcond = INS_COND_NE; + *inChain = true; } // Generate Op2 based on Op1. - genCodeForContainedCompareChain(op2, inchain, prevcond); - assert(*inchain); + genCodeForContainedCompareChain(op2, inChain, prevcond); + assert(*inChain); } } else @@ -4512,7 +4512,7 @@ void CodeGen::genCodeForContainedCompareChain(GenTreeOp* tree, bool* inchain, in if (tree->isContained()) { // Generate the compare, putting the result in the flags register. - if (!*inchain) + if (!*inChain) { // First item in a chain. Use a standard compare. genCodeForCompare(tree); @@ -4524,12 +4524,12 @@ void CodeGen::genCodeForContainedCompareChain(GenTreeOp* tree, bool* inchain, in genCodeForConditionalCompare(tree, *prevcond); } - *inchain = true; + *inChain = true; *prevcond = InsCondForCompareOp(tree); } else { - *inchain = false; + *inChain = false; } } } diff --git a/src/coreclr/jit/codegenarmarch.cpp b/src/coreclr/jit/codegenarmarch.cpp index 3081e27a09580a..13749cbfa3a1bc 100644 --- a/src/coreclr/jit/codegenarmarch.cpp +++ b/src/coreclr/jit/codegenarmarch.cpp @@ -212,15 +212,9 @@ void CodeGen::genCodeForTreeNode(GenTree* treeNode) genCodeForDivMod(treeNode->AsOp()); break; - case GT_AND: -#if !defined(TARGET_64BIT) - // An And that is not contained should not have any contained children. - assert(!treeNode->AsOp()->gtOp1->isContained() && !treeNode->AsOp()->gtOp2->isContained()); - FALLTHROUGH; -#endif - case GT_OR: case GT_XOR: + case GT_AND: case GT_AND_NOT: assert(varTypeIsIntegralOrI(treeNode)); From d98f9d084d37d79d5f5021d1bf89751ba3827905 Mon Sep 17 00:00:00 2001 From: Alan Hayward Date: Tue, 19 Jul 2022 14:58:47 +0100 Subject: [PATCH 08/26] Better contain checks for conditional compares --- src/coreclr/jit/lower.cpp | 7 +++---- src/coreclr/jit/lower.h | 3 +++ src/coreclr/jit/lowerarmarch.cpp | 30 ++++++++++++++++++++++++------ 3 files changed, 30 insertions(+), 10 deletions(-) diff --git a/src/coreclr/jit/lower.cpp b/src/coreclr/jit/lower.cpp index 710785ea15075a..915a5e1bc7fbc2 100644 --- a/src/coreclr/jit/lower.cpp +++ b/src/coreclr/jit/lower.cpp @@ -7058,10 +7058,12 @@ bool Lowering::ContainCheckCompareChain(GenTree* tree, GenTree* parent, GenTree* { tree->AsOp()->SetContained(); +#ifdef TARGET_ARM64 // Ensure the children of the compare are contained correctly. tree->AsOp()->gtGetOp1()->ClearContained(); tree->AsOp()->gtGetOp2()->ClearContained(); - ContainCheckCompare(tree->AsOp()); + ContainCheckConditionalCompare(tree->AsOp()); +#endif *earliest_valid = tree; return true; } @@ -7086,12 +7088,9 @@ void Lowering::ContainCheckSelect(GenTreeConditional* node) if (earliest_valid != nullptr) { // The earliest node in the chain will be generated as a standard compare. - // Temporary set as uncontained in order to correct its children. - earliest_valid->AsOp()->ClearContained(); earliest_valid->AsOp()->gtGetOp1()->ClearContained(); earliest_valid->AsOp()->gtGetOp2()->ClearContained(); ContainCheckCompare(earliest_valid->AsOp()); - earliest_valid->AsOp()->SetContained(); } } diff --git a/src/coreclr/jit/lower.h b/src/coreclr/jit/lower.h index f578767530b71f..4a98e779a18a0b 100644 --- a/src/coreclr/jit/lower.h +++ b/src/coreclr/jit/lower.h @@ -85,6 +85,9 @@ class Lowering final : public Phase void ContainCheckLclHeap(GenTreeOp* node); void ContainCheckRet(GenTreeUnOp* ret); void ContainCheckJTrue(GenTreeOp* node); +#ifdef TARGET_ARM64 + void ContainCheckConditionalCompare(GenTreeOp* cmp); +#endif bool ContainCheckCompareChain(GenTree* tree, GenTree* parent, GenTree** earliest_valid); void ContainCheckSelect(GenTreeConditional* node); void ContainCheckBitCast(GenTree* node); diff --git a/src/coreclr/jit/lowerarmarch.cpp b/src/coreclr/jit/lowerarmarch.cpp index 0e53996dcee11e..3afd5f04755a2f 100644 --- a/src/coreclr/jit/lowerarmarch.cpp +++ b/src/coreclr/jit/lowerarmarch.cpp @@ -104,12 +104,6 @@ bool Lowering::IsContainableImmed(GenTree* parentNode, GenTree* childNode) const case GT_LE: case GT_GE: case GT_GT: - if (parentNode->isContained()) - { - // Contained node will be generated as a ccmp. - return emitter::emitIns_valid_imm_for_ccmp(immVal); - } - FALLTHROUGH; case GT_CMP: case GT_BOUNDS_CHECK: return emitter::emitIns_valid_imm_for_cmp(immVal, size); @@ -2061,6 +2055,30 @@ void Lowering::ContainCheckCompare(GenTreeOp* cmp) CheckImmedAndMakeContained(cmp, cmp->gtOp2); } +//------------------------------------------------------------------------ +// ContainCheckConditionalCompare: determine whether the source of a compare within a compare chain should be contained. +// +// Arguments: +// node - pointer to the node +// +#ifdef TARGET_ARM64 +void Lowering::ContainCheckConditionalCompare(GenTreeOp* cmp) +{ + GenTree* op2 = cmp->gtOp2; + + if (!varTypeIsFloating(cmp->TypeGet()) && op2->IsCnsIntOrI() && + !op2->AsIntCon()->ImmedValNeedsReloc(comp)) + { + target_ssize_t immVal = (target_ssize_t)op2->AsIntCon()->gtIconVal; + + if (emitter::emitIns_valid_imm_for_ccmp(immVal)) + { + MakeSrcContained(cmp, op2); + } + } +} +#endif + //------------------------------------------------------------------------ // ContainCheckBoundsChk: determine whether any source of a bounds check node should be contained. // From 8ef999426a0ec132e053754ea6c605be38e19602 Mon Sep 17 00:00:00 2001 From: Alan Hayward Date: Wed, 20 Jul 2022 11:36:11 +0100 Subject: [PATCH 09/26] Simpler contained conditions codegen --- src/coreclr/jit/codegenarm64.cpp | 95 ++++++++++++++------------------ src/coreclr/jit/lowerarmarch.cpp | 3 +- 2 files changed, 42 insertions(+), 56 deletions(-) diff --git a/src/coreclr/jit/codegenarm64.cpp b/src/coreclr/jit/codegenarm64.cpp index b920c36a79e6c1..3c3e6ef71da228 100644 --- a/src/coreclr/jit/codegenarm64.cpp +++ b/src/coreclr/jit/codegenarm64.cpp @@ -4434,6 +4434,9 @@ void CodeGen::genCodeForConditionalCompare(GenTreeOp* tree, insCond cond) assert(!varTypeIsFloating(op2Type)); assert(!op1->isContainedIntOrIImmed()); + // Should only be called on contained nodes. + assert(targetReg == REG_NA); + // For the ccmp flags, invert the condition of the compare. insCflags cflags = InsCflagsForCcmp(InsCondForCompareOp(tree)); @@ -4447,13 +4450,6 @@ void CodeGen::genCodeForConditionalCompare(GenTreeOp* tree, insCond cond) regNumber srcReg2 = genConsumeReg(op2); emit->emitIns_R_R_FLAGS_COND(INS_ccmp, cmpSize, srcReg1, srcReg2, cflags, cond); } - - // Are we evaluating this into a register? - if (targetReg != REG_NA) - { - inst_SETCC(GenCondition::FromRelop(tree), tree->TypeGet(), targetReg); - genProduceReg(tree); - } } //------------------------------------------------------------------------ @@ -4471,66 +4467,53 @@ void CodeGen::genCodeForConditionalCompare(GenTreeOp* tree, insCond cond) // void CodeGen::genCodeForContainedCompareChain(GenTreeOp* tree, bool* inChain, insCond* prevcond) { + assert(tree->isContained()); + if (tree->OperIs(GT_AND)) { GenTreeOp* op1 = tree->gtGetOp1()->AsOp(); GenTreeOp* op2 = tree->gtGetOp2()->AsOp(); - if (!tree->isContained()) + assert(op2->isContained()); + + // If Op1 is contained, generate into flags. Otherwise, move the result into flags. + if (op1->isContained()) { - // An And that is not contained should not have any contained children. - assert(!op1->isContained() && !op2->isContained()); - *inChain = false; + genCodeForContainedCompareChain(op1, inChain, prevcond); + assert(*inChain); } else { - // An And can only be contained if Op2 is contained. - assert(op2->isContained()); - - // If Op1 is contained, generate into flags. Otherwise, move the result into flags. - if (op1->isContained()) - { - genCodeForContainedCompareChain(op1, inChain, prevcond); - assert(*inChain); - } - else - { - emitter* emit = GetEmitter(); - emit->emitIns_R_I(INS_cmp, EA_ATTR(genTypeSize(op1)), op1->GetRegNum(), 0); - *prevcond = INS_COND_NE; - *inChain = true; - } - - // Generate Op2 based on Op1. - genCodeForContainedCompareChain(op2, inChain, prevcond); - assert(*inChain); + emitter* emit = GetEmitter(); + genConsumeReg(op1); + emit->emitIns_R_I(INS_cmp, EA_ATTR(genTypeSize(op1)), op1->GetRegNum(), 0); + *prevcond = INS_COND_NE; + *inChain = true; } + + // Generate Op2 based on Op1. + genCodeForContainedCompareChain(op2, inChain, prevcond); + assert(*inChain); } else { assert(tree->OperIsCompare()); - if (tree->isContained()) - { - // Generate the compare, putting the result in the flags register. - if (!*inChain) - { - // First item in a chain. Use a standard compare. - genCodeForCompare(tree); - } - else - { - // Within the chain. Use a conditional compare (which is - // dependent on the previous emitted compare). - genCodeForConditionalCompare(tree, *prevcond); - } - *inChain = true; - *prevcond = InsCondForCompareOp(tree); + // Generate the compare, putting the result in the flags register. + if (!*inChain) + { + // First item in a chain. Use a standard compare. + genCodeForCompare(tree); } else { - *inChain = false; + // Within the chain. Use a conditional compare (which is + // dependent on the previous emitted compare). + genCodeForConditionalCompare(tree, *prevcond); } + + *inChain = true; + *prevcond = InsCondForCompareOp(tree); } } @@ -4554,14 +4537,18 @@ void CodeGen::genCodeForSelect(GenTreeConditional* tree) assert(!op1->isUsedFromMemory()); assert(genTypeSize(op1Type) == genTypeSize(op2Type)); - // Generate the condition. - bool chain = false; insCond cond = INS_COND_EQ; // Dummy value. - genCodeForContainedCompareChain(opcond->AsOp(), &chain, &cond); - assert(chain == opcond->isContained()); - if (!opcond->isContained()) + if (opcond->isContained()) + { + // Generate the contained condition. + bool chain = false; + genCodeForContainedCompareChain(opcond->AsOp(), &chain, &cond); + assert(chain); + } + else { - // Node has been generated into a register - move into flags. + // Condition has been generated into a register - move it into flags. + genConsumeReg(opcond); emit->emitIns_R_I(INS_cmp, EA_ATTR(genTypeSize(opcond)), opcond->GetRegNum(), 0); cond = INS_COND_NE; } diff --git a/src/coreclr/jit/lowerarmarch.cpp b/src/coreclr/jit/lowerarmarch.cpp index 3afd5f04755a2f..33125f8e8051c9 100644 --- a/src/coreclr/jit/lowerarmarch.cpp +++ b/src/coreclr/jit/lowerarmarch.cpp @@ -2066,8 +2066,7 @@ void Lowering::ContainCheckConditionalCompare(GenTreeOp* cmp) { GenTree* op2 = cmp->gtOp2; - if (!varTypeIsFloating(cmp->TypeGet()) && op2->IsCnsIntOrI() && - !op2->AsIntCon()->ImmedValNeedsReloc(comp)) + if (!varTypeIsFloating(cmp->TypeGet()) && op2->IsCnsIntOrI() && !op2->AsIntCon()->ImmedValNeedsReloc(comp)) { target_ssize_t immVal = (target_ssize_t)op2->AsIntCon()->gtIconVal; From 62028941d6f17ccd8e7c627246289093247a3ca9 Mon Sep 17 00:00:00 2001 From: Alan Hayward Date: Mon, 18 Jul 2022 16:08:31 +0100 Subject: [PATCH 10/26] Remove Conditional Compare nodes --- src/coreclr/jit/codegenarm64.cpp | 64 ++++++++++-------------------- src/coreclr/jit/codegenarmarch.cpp | 6 --- src/coreclr/jit/codegenlinear.cpp | 18 +-------- src/coreclr/jit/compiler.h | 6 --- src/coreclr/jit/compiler.hpp | 6 --- src/coreclr/jit/gentree.cpp | 36 ----------------- src/coreclr/jit/gentree.h | 39 ++++-------------- src/coreclr/jit/gtlist.h | 6 --- src/coreclr/jit/gtstructs.h | 2 +- src/coreclr/jit/lower.cpp | 12 ------ src/coreclr/jit/lowerarmarch.cpp | 8 ---- src/coreclr/jit/lsraarm64.cpp | 7 ---- src/coreclr/jit/lsrabuild.cpp | 8 ---- 13 files changed, 29 insertions(+), 189 deletions(-) diff --git a/src/coreclr/jit/codegenarm64.cpp b/src/coreclr/jit/codegenarm64.cpp index 3c3e6ef71da228..7c136f579a8296 100644 --- a/src/coreclr/jit/codegenarm64.cpp +++ b/src/coreclr/jit/codegenarm64.cpp @@ -10539,51 +10539,27 @@ void CodeGen::genCodeForCond(GenTreeOp* tree) // insCond CodeGen::InsCondForCompareOp(GenTree* tree) { - assert(tree->OperIsCompare() || tree->OperIsConditionalCompare()); - - if (tree->OperIsCompare()) - { - switch (tree->AsOp()->OperGet()) - { - case GT_EQ: - case GT_TEST_EQ: - return INS_COND_EQ; - case GT_NE: - case GT_TEST_NE: - return INS_COND_NE; - case GT_GE: - return INS_COND_GE; - case GT_GT: - return INS_COND_GT; - case GT_LT: - return INS_COND_LT; - case GT_LE: - return INS_COND_LE; - default: - assert(false && "Invalid condition"); - return INS_COND_EQ; - } - } - else + assert(tree->OperIsCompare()); + + switch (tree->AsOp()->OperGet()) { - switch (tree->AsConditional()->OperGet()) - { - case GT_CEQ: - return INS_COND_EQ; - case GT_CNE: - return INS_COND_NE; - case GT_CGE: - return INS_COND_GE; - case GT_CGT: - return INS_COND_GT; - case GT_CLT: - return INS_COND_LT; - case GT_CLE: - return INS_COND_LE; - default: - assert(false && "Invalid condition"); - return INS_COND_EQ; - } + case GT_EQ: + case GT_TEST_EQ: + return INS_COND_EQ; + case GT_NE: + case GT_TEST_NE: + return INS_COND_NE; + case GT_GE: + return INS_COND_GE; + case GT_GT: + return INS_COND_GT; + case GT_LT: + return INS_COND_LT; + case GT_LE: + return INS_COND_LE; + default: + assert(false && "Invalid condition"); + return INS_COND_EQ; } } diff --git a/src/coreclr/jit/codegenarmarch.cpp b/src/coreclr/jit/codegenarmarch.cpp index 13749cbfa3a1bc..564fce1e05fafd 100644 --- a/src/coreclr/jit/codegenarmarch.cpp +++ b/src/coreclr/jit/codegenarmarch.cpp @@ -369,12 +369,6 @@ void CodeGen::genCodeForTreeNode(GenTree* treeNode) #ifdef TARGET_ARM64 case GT_SELECT: - case GT_CEQ: - case GT_CNE: - case GT_CLT: - case GT_CLE: - case GT_CGE: - case GT_CGT: genCodeForSelect(treeNode->AsConditional()); break; #endif diff --git a/src/coreclr/jit/codegenlinear.cpp b/src/coreclr/jit/codegenlinear.cpp index bbceba031e6cbe..2f15f91208e32c 100644 --- a/src/coreclr/jit/codegenlinear.cpp +++ b/src/coreclr/jit/codegenlinear.cpp @@ -1635,13 +1635,6 @@ void CodeGen::genConsumeRegs(GenTree* tree) genConsumeRegs(tree->gtGetOp1()); genConsumeRegs(tree->gtGetOp2()); } - else if (tree->OperIsConditionalCompare()) - { - // Conditional compares should always be contained. - genConsumeRegs(tree->AsConditional()->gtCond); - genConsumeRegs(tree->gtGetOp1()); - genConsumeRegs(tree->gtGetOp2()); - } #endif else if (tree->OperIsLocalRead()) { @@ -2643,16 +2636,7 @@ void CodeGen::genCodeForJumpTrue(GenTreeOp* jtrue) assert(compiler->compCurBB->bbJumpKind == BBJ_COND); assert(jtrue->OperIs(GT_JTRUE)); - GenTreeOp* relop; - if (jtrue->gtGetOp1()->OperIsCompare()) - { - relop = jtrue->gtGetOp1()->AsOp(); - } - else - { - assert(jtrue->gtGetOp1()->OperIsConditionalCompare()); - relop = jtrue->gtGetOp1()->AsConditional(); - } + GenTreeOp* relop = jtrue->gtGetOp1()->AsOp(); GenCondition condition = GenCondition::FromRelop(relop); if (condition.PreferSwap()) diff --git a/src/coreclr/jit/compiler.h b/src/coreclr/jit/compiler.h index b26460f2adfb0b..deb8d95072aba6 100644 --- a/src/coreclr/jit/compiler.h +++ b/src/coreclr/jit/compiler.h @@ -10985,12 +10985,6 @@ class GenTreeVisitor #endif // defined(FEATURE_SIMD) || defined(FEATURE_HW_INTRINSICS) case GT_SELECT: - case GT_CEQ: - case GT_CNE: - case GT_CLT: - case GT_CLE: - case GT_CGE: - case GT_CGT: { GenTreeConditional* const conditional = node->AsConditional(); diff --git a/src/coreclr/jit/compiler.hpp b/src/coreclr/jit/compiler.hpp index eec2ccfe73d15e..37d8b979280a60 100644 --- a/src/coreclr/jit/compiler.hpp +++ b/src/coreclr/jit/compiler.hpp @@ -4394,12 +4394,6 @@ void GenTree::VisitOperands(TVisitor visitor) } case GT_SELECT: - case GT_CEQ: - case GT_CNE: - case GT_CLT: - case GT_CLE: - case GT_CGE: - case GT_CGT: { GenTreeConditional* const cond = this->AsConditional(); if (visitor(cond->gtCond) == VisitResult::Abort) diff --git a/src/coreclr/jit/gentree.cpp b/src/coreclr/jit/gentree.cpp index ccefcbd8f7b097..b214781d6642c7 100644 --- a/src/coreclr/jit/gentree.cpp +++ b/src/coreclr/jit/gentree.cpp @@ -5791,12 +5791,6 @@ unsigned Compiler::gtSetEvalOrder(GenTree* tree) break; case GT_SELECT: - case GT_CEQ: - case GT_CNE: - case GT_CLT: - case GT_CLE: - case GT_CGE: - case GT_CGT: level = gtSetEvalOrder(tree->AsConditional()->gtCond); costEx = tree->AsConditional()->gtCond->GetCostEx(); costSz = tree->AsConditional()->gtCond->GetCostSz(); @@ -6205,12 +6199,6 @@ bool GenTree::TryGetUse(GenTree* operand, GenTree*** pUse) } case GT_SELECT: - case GT_CEQ: - case GT_CNE: - case GT_CLT: - case GT_CLE: - case GT_CGE: - case GT_CGT: { GenTreeConditional* const conditional = this->AsConditional(); if (operand == conditional->gtCond) @@ -8760,12 +8748,6 @@ GenTree* Compiler::gtCloneExpr( break; case GT_SELECT: - case GT_CEQ: - case GT_CNE: - case GT_CLT: - case GT_CLE: - case GT_CGE: - case GT_CGT: copy = new (this, oper) GenTreeConditional(oper, tree->TypeGet(), gtCloneExpr(tree->AsConditional()->gtCond, addFlags, deepVarNum, deepVarVal), @@ -9432,12 +9414,6 @@ GenTreeUseEdgeIterator::GenTreeUseEdgeIterator(GenTree* node) return; case GT_SELECT: - case GT_CEQ: - case GT_CNE: - case GT_CLT: - case GT_CLE: - case GT_CGE: - case GT_CGT: m_edge = &m_node->AsConditional()->gtCond; assert(*m_edge != nullptr); m_advance = &GenTreeUseEdgeIterator::AdvanceConditional; @@ -10470,12 +10446,6 @@ void Compiler::gtDispNode(GenTree* tree, IndentStack* indentStack, _In_ _In_opt_ case GT_TEST_EQ: case GT_TEST_NE: case GT_SELECT: - case GT_CEQ: - case GT_CNE: - case GT_CLT: - case GT_CLE: - case GT_CGE: - case GT_CGT: if (tree->gtFlags & GTF_RELOP_NAN_UN) { printf("N"); @@ -12122,12 +12092,6 @@ void Compiler::gtDispTree(GenTree* tree, break; case GT_SELECT: - case GT_CEQ: - case GT_CNE: - case GT_CLT: - case GT_CLE: - case GT_CGE: - case GT_CGT: gtDispCommonEndLine(tree); if (!topOnly) diff --git a/src/coreclr/jit/gentree.h b/src/coreclr/jit/gentree.h index d8ea5a832468e3..832f04c33ebb2f 100644 --- a/src/coreclr/jit/gentree.h +++ b/src/coreclr/jit/gentree.h @@ -1088,7 +1088,7 @@ struct GenTree if (gtType == TYP_VOID) { // These are the only operators which can produce either VOID or non-VOID results. - assert(OperIs(GT_NOP, GT_CALL, GT_COMMA) || OperIsCompare() || OperIsConditionalCompare() || OperIsLong() || + assert(OperIs(GT_NOP, GT_CALL, GT_COMMA) || OperIsCompare() || OperIsLong() || OperIsSimdOrHWintrinsic() || IsCnsVec()); return false; } @@ -1355,8 +1355,7 @@ struct GenTree static bool OperIsConditional(genTreeOps gtOper) { - static_assert_no_msg(AreContiguous(GT_SELECT, GT_CEQ, GT_CNE, GT_CLT, GT_CLE, GT_CGE, GT_CGT)); - return (GT_SELECT <= gtOper) && (gtOper <= GT_CGT); + return (GT_SELECT == gtOper); } bool OperIsConditional() const @@ -1364,17 +1363,6 @@ struct GenTree return OperIsConditional(OperGet()); } - static bool OperIsConditionalCompare(genTreeOps gtOper) - { - static_assert_no_msg(AreContiguous(GT_CEQ, GT_CNE, GT_CLT, GT_CLE, GT_CGE, GT_CGT)); - return (GT_CEQ <= gtOper) && (gtOper <= GT_CGT); - } - - bool OperIsConditionalCompare() const - { - return OperIsConditionalCompare(OperGet()); - } - static bool OperIsCC(genTreeOps gtOper) { return (gtOper == GT_JCC) || (gtOper == GT_SETCC); @@ -8232,9 +8220,9 @@ struct GenCondition static GenCondition FromRelop(GenTree* relop) { - assert(relop->OperIsCompare() || relop->OperIsConditionalCompare()); + assert(relop->OperIsCompare()); - if (relop->OperIsCompare() && varTypeIsFloating(relop->gtGetOp1())) + if (varTypeIsFloating(relop->gtGetOp1())) { return FromFloatRelop(relop); } @@ -8269,30 +8257,17 @@ struct GenCondition static GenCondition FromIntegralRelop(GenTree* relop) { - if (relop->OperIsConditionalCompare()) - { - assert(!varTypeIsFloating(relop->AsConditional()->gtOp1) && - !varTypeIsFloating(relop->AsConditional()->gtOp2)); - } - else - { - assert(!varTypeIsFloating(relop->gtGetOp1()) && !varTypeIsFloating(relop->gtGetOp2())); - } - + assert(!varTypeIsFloating(relop->gtGetOp1()) && !varTypeIsFloating(relop->gtGetOp2())); return FromIntegralRelop(relop->OperGet(), relop->IsUnsigned()); } static GenCondition FromIntegralRelop(genTreeOps oper, bool isUnsigned) { - assert(GenTree::OperIsCompare(oper) || GenTree::OperIsConditionalCompare(oper)); + assert(GenTree::OperIsCompare(oper)); // GT_TEST_EQ/NE are special, they need to be mapped as GT_EQ/NE unsigned code; - if (oper >= GT_CEQ) - { - code = oper - GT_CEQ; - } - else if (oper >= GT_TEST_EQ) + if (oper >= GT_TEST_EQ) { code = oper - GT_TEST_EQ; } diff --git a/src/coreclr/jit/gtlist.h b/src/coreclr/jit/gtlist.h index fe8b5c89d3ad14..56ea06b2a061bd 100644 --- a/src/coreclr/jit/gtlist.h +++ b/src/coreclr/jit/gtlist.h @@ -147,12 +147,6 @@ GTNODE(TEST_EQ , GenTreeOp ,0,GTK_BINOP|DBK_NOTHIR) GTNODE(TEST_NE , GenTreeOp ,0,GTK_BINOP|DBK_NOTHIR) GTNODE(SELECT , GenTreeConditional ,0,GTK_SPECIAL) -GTNODE(CEQ , GenTreeConditional ,0,GTK_SPECIAL) -GTNODE(CNE , GenTreeConditional ,0,GTK_SPECIAL) -GTNODE(CLT , GenTreeConditional ,0,GTK_SPECIAL) -GTNODE(CLE , GenTreeConditional ,0,GTK_SPECIAL) -GTNODE(CGE , GenTreeConditional ,0,GTK_SPECIAL) -GTNODE(CGT , GenTreeConditional ,0,GTK_SPECIAL) GTNODE(COMMA , GenTreeOp ,0,GTK_BINOP|DBK_NOTLIR) GTNODE(QMARK , GenTreeQmark ,0,GTK_BINOP|GTK_EXOP|DBK_NOTLIR) diff --git a/src/coreclr/jit/gtstructs.h b/src/coreclr/jit/gtstructs.h index f3e89080c390ea..7d50adbca39f97 100644 --- a/src/coreclr/jit/gtstructs.h +++ b/src/coreclr/jit/gtstructs.h @@ -101,7 +101,7 @@ GTSTRUCT_1(PhiArg , GT_PHI_ARG) GTSTRUCT_1(Phi , GT_PHI) GTSTRUCT_1(StoreInd , GT_STOREIND) GTSTRUCT_N(Indir , GT_STOREIND, GT_IND, GT_NULLCHECK, GT_BLK, GT_STORE_BLK, GT_OBJ, GT_STORE_OBJ, GT_STORE_DYN_BLK) -GTSTRUCT_N(Conditional , GT_SELECT, GT_CEQ, GT_CNE, GT_CLT, GT_CLE, GT_CGE, GT_CGT) +GTSTRUCT_N(Conditional , GT_SELECT) #if FEATURE_ARG_SPLIT GTSTRUCT_2_SPECIAL(PutArgStk, GT_PUTARG_STK, GT_PUTARG_SPLIT) GTSTRUCT_1(PutArgSplit , GT_PUTARG_SPLIT) diff --git a/src/coreclr/jit/lower.cpp b/src/coreclr/jit/lower.cpp index 915a5e1bc7fbc2..55c8d29d4b2a80 100644 --- a/src/coreclr/jit/lower.cpp +++ b/src/coreclr/jit/lower.cpp @@ -239,12 +239,6 @@ GenTree* Lowering::LowerNode(GenTree* node) return LowerJTrue(node->AsOp()); case GT_SELECT: - case GT_CEQ: - case GT_CNE: - case GT_CLT: - case GT_CLE: - case GT_CGE: - case GT_CGT: ContainCheckSelect(node->AsConditional()); break; @@ -6826,12 +6820,6 @@ void Lowering::ContainCheckNode(GenTree* node) break; case GT_SELECT: - case GT_CEQ: - case GT_CNE: - case GT_CLT: - case GT_CLE: - case GT_CGE: - case GT_CGT: ContainCheckSelect(node->AsConditional()); break; diff --git a/src/coreclr/jit/lowerarmarch.cpp b/src/coreclr/jit/lowerarmarch.cpp index 33125f8e8051c9..42a8e2fc48544f 100644 --- a/src/coreclr/jit/lowerarmarch.cpp +++ b/src/coreclr/jit/lowerarmarch.cpp @@ -141,14 +141,6 @@ bool Lowering::IsContainableImmed(GenTree* parentNode, GenTree* childNode) const if (immVal == 0) return true; break; - - case GT_CEQ: - case GT_CNE: - case GT_CLT: - case GT_CLE: - case GT_CGE: - case GT_CGT: - return emitter::emitIns_valid_imm_for_ccmp(immVal); #endif default: diff --git a/src/coreclr/jit/lsraarm64.cpp b/src/coreclr/jit/lsraarm64.cpp index 940249bbe8043c..b2c815bb1e396e 100644 --- a/src/coreclr/jit/lsraarm64.cpp +++ b/src/coreclr/jit/lsraarm64.cpp @@ -794,13 +794,6 @@ int LinearScan::BuildNode(GenTree* tree) case GT_SELECT: assert(dstCount == 1); - FALLTHROUGH; - case GT_CEQ: - case GT_CNE: - case GT_CLT: - case GT_CLE: - case GT_CGE: - case GT_CGT: srcCount = BuildOperandUses(tree->AsConditional()->gtCond); srcCount += BuildOperandUses(tree->AsConditional()->gtOp1); srcCount += BuildOperandUses(tree->AsConditional()->gtOp2); diff --git a/src/coreclr/jit/lsrabuild.cpp b/src/coreclr/jit/lsrabuild.cpp index f48bfc285ed9ab..58889895249e12 100644 --- a/src/coreclr/jit/lsrabuild.cpp +++ b/src/coreclr/jit/lsrabuild.cpp @@ -3154,14 +3154,6 @@ int LinearScan::BuildOperandUses(GenTree* node, regMaskTP candidates) // GT_CAST and GT_LSH for ADD with sign/zero extension return BuildOperandUses(node->gtGetOp1(), candidates); } - if (node->OperIsConditionalCompare()) - { - // A conditional compare may be contained due to If Conversion. - int uses = BuildOperandUses(node->AsConditional()->gtCond, candidates); - uses += BuildOperandUses(node->AsConditional()->gtOp1, candidates); - uses += BuildOperandUses(node->AsConditional()->gtOp2, candidates); - return uses; - } #endif return 0; From c0aba377c724454bd176740619863856dabee18f Mon Sep 17 00:00:00 2001 From: Alan Hayward Date: Wed, 20 Jul 2022 13:30:37 +0100 Subject: [PATCH 11/26] Minor cleanups --- src/coreclr/jit/lower.cpp | 22 +++++++++++----------- src/coreclr/jit/lowerarmarch.cpp | 8 ++------ 2 files changed, 13 insertions(+), 17 deletions(-) diff --git a/src/coreclr/jit/lower.cpp b/src/coreclr/jit/lower.cpp index 55c8d29d4b2a80..a265f791f240a2 100644 --- a/src/coreclr/jit/lower.cpp +++ b/src/coreclr/jit/lower.cpp @@ -7021,20 +7021,20 @@ void Lowering::ContainCheckJTrue(GenTreeOp* node) // Arguments: // tree - the node. Either a compare or a tree of compares connected by ANDs. // parent - the parent of the tree. -// earliest_valid - Returns the earliest valid compare node in the chain (if any). +// earliestValid - Returns the earliest valid compare node in the chain (if any). // // Return Value: // True if the chain is valid // -bool Lowering::ContainCheckCompareChain(GenTree* tree, GenTree* parent, GenTree** earliest_valid) +bool Lowering::ContainCheckCompareChain(GenTree* tree, GenTree* parent, GenTree** earliestValid) { if (tree->OperIs(GT_AND)) { // To ensure ordering at code generation, Op1 and the parent can // only be contained if Op2 is contained. - if (ContainCheckCompareChain(tree->AsOp()->gtGetOp2(), tree, earliest_valid)) + if (ContainCheckCompareChain(tree->AsOp()->gtGetOp2(), tree, earliestValid)) { - ContainCheckCompareChain(tree->AsOp()->gtGetOp1(), tree, earliest_valid); + ContainCheckCompareChain(tree->AsOp()->gtGetOp1(), tree, earliestValid); tree->SetContained(); return true; } @@ -7052,7 +7052,7 @@ bool Lowering::ContainCheckCompareChain(GenTree* tree, GenTree* parent, GenTree* tree->AsOp()->gtGetOp2()->ClearContained(); ContainCheckConditionalCompare(tree->AsOp()); #endif - *earliest_valid = tree; + *earliestValid = tree; return true; } } @@ -7070,15 +7070,15 @@ bool Lowering::ContainCheckCompareChain(GenTree* tree, GenTree* parent, GenTree* void Lowering::ContainCheckSelect(GenTreeConditional* node) { // Check if the compare does not need to be generated into a register. - GenTree* earliest_valid = nullptr; - ContainCheckCompareChain(node->gtCond, node, &earliest_valid); + GenTree* earliestValid = nullptr; + ContainCheckCompareChain(node->gtCond, node, &earliestValid); - if (earliest_valid != nullptr) + if (earliestValid != nullptr) { // The earliest node in the chain will be generated as a standard compare. - earliest_valid->AsOp()->gtGetOp1()->ClearContained(); - earliest_valid->AsOp()->gtGetOp2()->ClearContained(); - ContainCheckCompare(earliest_valid->AsOp()); + earliestValid->AsOp()->gtGetOp1()->ClearContained(); + earliestValid->AsOp()->gtGetOp2()->ClearContained(); + ContainCheckCompare(earliestValid->AsOp()); } } diff --git a/src/coreclr/jit/lowerarmarch.cpp b/src/coreclr/jit/lowerarmarch.cpp index 42a8e2fc48544f..77aac81b1929f2 100644 --- a/src/coreclr/jit/lowerarmarch.cpp +++ b/src/coreclr/jit/lowerarmarch.cpp @@ -112,11 +112,6 @@ bool Lowering::IsContainableImmed(GenTree* parentNode, GenTree* childNode) const case GT_XOR: case GT_TEST_EQ: case GT_TEST_NE: - if (parentNode->isContained()) - { - // Contained node will be generated as a ccmp. - return emitter::emitIns_valid_imm_for_ccmp(immVal); - } return emitter::emitIns_valid_imm_for_alu(immVal, size); case GT_JCMP: assert(((parentNode->gtFlags & GTF_JCMP_TST) == 0) ? (immVal == 0) : isPow2(immVal)); @@ -2056,9 +2051,10 @@ void Lowering::ContainCheckCompare(GenTreeOp* cmp) #ifdef TARGET_ARM64 void Lowering::ContainCheckConditionalCompare(GenTreeOp* cmp) { + assert(cmp->OperIsCompare()); GenTree* op2 = cmp->gtOp2; - if (!varTypeIsFloating(cmp->TypeGet()) && op2->IsCnsIntOrI() && !op2->AsIntCon()->ImmedValNeedsReloc(comp)) + if (op2->IsCnsIntOrI() && !op2->AsIntCon()->ImmedValNeedsReloc(comp)) { target_ssize_t immVal = (target_ssize_t)op2->AsIntCon()->gtIconVal; From 99958816a650c1a1349e5d27d30afb2fef2d0b81 Mon Sep 17 00:00:00 2001 From: Alan Hayward Date: Tue, 26 Jul 2022 11:21:40 +0100 Subject: [PATCH 12/26] Generate AND compare chains --- src/coreclr/jit/codegen.h | 2 +- src/coreclr/jit/codegenarm64.cpp | 83 ++++++++++--- src/coreclr/jit/codegenarmarch.cpp | 2 + src/coreclr/jit/codegenlinear.cpp | 6 +- src/coreclr/jit/gentree.h | 10 +- src/coreclr/jit/lower.cpp | 186 +++++++++++++++++++++++++---- src/coreclr/jit/lower.h | 4 +- src/coreclr/jit/lowerarmarch.cpp | 4 + 8 files changed, 249 insertions(+), 48 deletions(-) diff --git a/src/coreclr/jit/codegen.h b/src/coreclr/jit/codegen.h index d232acc5c75762..4f93d4a34bc3d7 100644 --- a/src/coreclr/jit/codegen.h +++ b/src/coreclr/jit/codegen.h @@ -1048,7 +1048,7 @@ XXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXX void genCodeForCompare(GenTreeOp* tree); #ifdef TARGET_ARM64 void genCodeForConditionalCompare(GenTreeOp* tree, insCond cond); - void genCodeForContainedCompareChain(GenTreeOp* tree, bool* inchain, insCond* prevcond); + void genCodeForContainedCompareChain(GenTree* tree, bool* inchain, insCond* prevcond); void genCodeForSelect(GenTreeConditional* tree); #endif void genIntrinsic(GenTree* treeNode); diff --git a/src/coreclr/jit/codegenarm64.cpp b/src/coreclr/jit/codegenarm64.cpp index 7c136f579a8296..cf11834f092a11 100644 --- a/src/coreclr/jit/codegenarm64.cpp +++ b/src/coreclr/jit/codegenarm64.cpp @@ -2513,6 +2513,9 @@ void CodeGen::genCodeForBinary(GenTreeOp* tree) GenTree* op1 = tree->gtGetOp1(); GenTree* op2 = tree->gtGetOp2(); + // The arithmetic node must be sitting in a register (since it's not contained) + assert(targetReg != REG_NA); + // Handles combined operations: 'madd', 'msub' if (op2->OperIs(GT_MUL) && op2->isContained()) { @@ -2552,6 +2555,58 @@ void CodeGen::genCodeForBinary(GenTreeOp* tree) return; } + if (tree->OperIs(GT_AND) && op2->isContainedAndNotIntOrIImmed()) + { + insCond cond = INS_COND_EQ; // Dummy value. + bool chain = false; + + if (op1->isContained()) + { + // Generate Op1 into flags. + genCodeForContainedCompareChain(op1, &chain, &cond); + assert(chain); + } + else + { + // Op1 is not contained, move it from a register into flags. + emit->emitIns_R_I(INS_cmp, EA_ATTR(genTypeSize(op1)), op1->GetRegNum(), 0); + cond = INS_COND_NE; + chain = true; + } + // Gen Op2 into flags. + genCodeForContainedCompareChain(op2, &chain, &cond); + assert(chain); + + // Move the result from flags into a register. + genTreeOps opCond = GT_EQ; + switch (cond) + { + case INS_COND_EQ: + opCond = GT_EQ; + break; + case INS_COND_NE: + opCond = GT_NE; + break; + case INS_COND_GE: + opCond = GT_GE; + break; + case INS_COND_LT: + opCond = GT_LT; + break; + case INS_COND_GT: + opCond = GT_GT; + break; + case INS_COND_LE: + opCond = GT_LE; + break; + default: + assert(!"Unexpected cond"); + } + inst_SETCC(GenCondition::FromIntegralRelop(opCond, false), tree->TypeGet(), targetReg); + genProduceReg(tree); + return; + } + instruction ins = genGetInsForOper(tree->OperGet(), targetType); if ((tree->gtFlags & GTF_SET_FLAGS) != 0) @@ -2575,9 +2630,6 @@ void CodeGen::genCodeForBinary(GenTreeOp* tree) } } - // The arithmetic node must be sitting in a register (since it's not contained) - assert(targetReg != REG_NA); - regNumber r = emit->emitInsTernary(ins, emitActualTypeSize(tree), tree, op1, op2); assert(r == targetReg); @@ -4361,8 +4413,6 @@ void CodeGen::genCodeForCompare(GenTreeOp* tree) assert(!op1->isUsedFromMemory()); - genConsumeOperands(tree); - emitAttr cmpSize = EA_ATTR(genTypeSize(op1Type)); assert(genTypeSize(op1Type) == genTypeSize(op2Type)); @@ -4428,7 +4478,7 @@ void CodeGen::genCodeForConditionalCompare(GenTreeOp* tree, insCond cond) var_types op2Type = genActualType(op2->TypeGet()); emitAttr cmpSize = EA_ATTR(genTypeSize(op1Type)); regNumber targetReg = tree->GetRegNum(); - regNumber srcReg1 = genConsumeReg(op1); + regNumber srcReg1 = op1->GetRegNum(); // No float support or swapping op1 and op2 to generate cmp reg, imm. assert(!varTypeIsFloating(op2Type)); @@ -4447,7 +4497,7 @@ void CodeGen::genCodeForConditionalCompare(GenTreeOp* tree, insCond cond) } else { - regNumber srcReg2 = genConsumeReg(op2); + regNumber srcReg2 = op2->GetRegNum(); emit->emitIns_R_R_FLAGS_COND(INS_ccmp, cmpSize, srcReg1, srcReg2, cflags, cond); } } @@ -4465,14 +4515,18 @@ void CodeGen::genCodeForConditionalCompare(GenTreeOp* tree, insCond cond) // Return: // The last compare node generated. // -void CodeGen::genCodeForContainedCompareChain(GenTreeOp* tree, bool* inChain, insCond* prevcond) +void CodeGen::genCodeForContainedCompareChain(GenTree* tree, bool* inChain, insCond* prevcond) { assert(tree->isContained()); + if (!*inChain) + { + JITDUMP("Generating compare chain:\n"); + } if (tree->OperIs(GT_AND)) { - GenTreeOp* op1 = tree->gtGetOp1()->AsOp(); - GenTreeOp* op2 = tree->gtGetOp2()->AsOp(); + GenTree* op1 = tree->gtGetOp1(); + GenTree* op2 = tree->gtGetOp2(); assert(op2->isContained()); @@ -4485,7 +4539,6 @@ void CodeGen::genCodeForContainedCompareChain(GenTreeOp* tree, bool* inChain, in else { emitter* emit = GetEmitter(); - genConsumeReg(op1); emit->emitIns_R_I(INS_cmp, EA_ATTR(genTypeSize(op1)), op1->GetRegNum(), 0); *prevcond = INS_COND_NE; *inChain = true; @@ -4503,13 +4556,13 @@ void CodeGen::genCodeForContainedCompareChain(GenTreeOp* tree, bool* inChain, in if (!*inChain) { // First item in a chain. Use a standard compare. - genCodeForCompare(tree); + genCodeForCompare(tree->AsOp()); } else { // Within the chain. Use a conditional compare (which is // dependent on the previous emitted compare). - genCodeForConditionalCompare(tree, *prevcond); + genCodeForConditionalCompare(tree->AsOp(), *prevcond); } *inChain = true; @@ -4537,12 +4590,12 @@ void CodeGen::genCodeForSelect(GenTreeConditional* tree) assert(!op1->isUsedFromMemory()); assert(genTypeSize(op1Type) == genTypeSize(op2Type)); - insCond cond = INS_COND_EQ; // Dummy value. + insCond cond = INS_COND_EQ; // Dummy value. if (opcond->isContained()) { // Generate the contained condition. bool chain = false; - genCodeForContainedCompareChain(opcond->AsOp(), &chain, &cond); + genCodeForContainedCompareChain(opcond, &chain, &cond); assert(chain); } else diff --git a/src/coreclr/jit/codegenarmarch.cpp b/src/coreclr/jit/codegenarmarch.cpp index 564fce1e05fafd..c95c84b72902ab 100644 --- a/src/coreclr/jit/codegenarmarch.cpp +++ b/src/coreclr/jit/codegenarmarch.cpp @@ -363,6 +363,8 @@ void CodeGen::genCodeForTreeNode(GenTree* treeNode) #ifdef TARGET_ARM64 case GT_TEST_EQ: case GT_TEST_NE: + // Consume early to ensure chains consume correctly. + genConsumeOperands(treeNode->AsOp()); #endif // TARGET_ARM64 genCodeForCompare(treeNode->AsOp()); break; diff --git a/src/coreclr/jit/codegenlinear.cpp b/src/coreclr/jit/codegenlinear.cpp index 2f15f91208e32c..abc2d9e5a0d714 100644 --- a/src/coreclr/jit/codegenlinear.cpp +++ b/src/coreclr/jit/codegenlinear.cpp @@ -1629,9 +1629,9 @@ void CodeGen::genConsumeRegs(GenTree* tree) assert(cast->isContained()); genConsumeAddress(cast->CastOp()); } - else if (tree->OperIsCompare()) + else if (tree->OperIsCompare() || tree->OperIs(GT_AND)) { - // Compares may be contained by a conditional. + // Compares and ANDs may be contained in a conditional chain. genConsumeRegs(tree->gtGetOp1()); genConsumeRegs(tree->gtGetOp2()); } @@ -2636,7 +2636,7 @@ void CodeGen::genCodeForJumpTrue(GenTreeOp* jtrue) assert(compiler->compCurBB->bbJumpKind == BBJ_COND); assert(jtrue->OperIs(GT_JTRUE)); - GenTreeOp* relop = jtrue->gtGetOp1()->AsOp(); + GenTreeOp* relop = jtrue->gtGetOp1()->AsOp(); GenCondition condition = GenCondition::FromRelop(relop); if (condition.PreferSwap()) diff --git a/src/coreclr/jit/gentree.h b/src/coreclr/jit/gentree.h index 832f04c33ebb2f..4c6cd1831b18ba 100644 --- a/src/coreclr/jit/gentree.h +++ b/src/coreclr/jit/gentree.h @@ -915,6 +915,12 @@ struct GenTree return isContained() && IsCnsIntOrI() && !isUsedFromSpillTemp(); } + // Node is contained, but it isn't contained due to being a containable int. + bool isContainedAndNotIntOrIImmed() const + { + return isContained() && !isContainedIntOrIImmed(); + } + bool isContainedFltOrDblImmed() const { return isContained() && OperIs(GT_CNS_DBL); @@ -1088,8 +1094,8 @@ struct GenTree if (gtType == TYP_VOID) { // These are the only operators which can produce either VOID or non-VOID results. - assert(OperIs(GT_NOP, GT_CALL, GT_COMMA) || OperIsCompare() || OperIsLong() || - OperIsSimdOrHWintrinsic() || IsCnsVec()); + assert(OperIs(GT_NOP, GT_CALL, GT_COMMA) || OperIsCompare() || OperIsLong() || OperIsSimdOrHWintrinsic() || + IsCnsVec()); return false; } diff --git a/src/coreclr/jit/lower.cpp b/src/coreclr/jit/lower.cpp index a265f791f240a2..b4852136691e7e 100644 --- a/src/coreclr/jit/lower.cpp +++ b/src/coreclr/jit/lower.cpp @@ -2746,6 +2746,15 @@ GenTree* Lowering::OptimizeConstCompare(GenTree* cmp) GenTreeIntCon* op2 = cmp->gtGetOp2()->AsIntCon(); ssize_t op2Value = op2->IconValue(); +#ifdef TARGET_ARM64 + // Do not optimise further if op1 has a contained chain. + if (op1->OperIs(GT_AND) && + (op1->gtGetOp1()->isContainedAndNotIntOrIImmed() || op1->gtGetOp2()->isContainedAndNotIntOrIImmed())) + { + return cmp; + } +#endif + #ifdef TARGET_XARCH var_types op1Type = op1->TypeGet(); if (IsContainableMemoryOp(op1) && varTypeIsSmall(op1Type) && FitsIn(op1Type, op2Value)) @@ -7014,53 +7023,172 @@ void Lowering::ContainCheckJTrue(GenTreeOp* node) } //------------------------------------------------------------------------ -// ContainCheckCompareChain : determine whether a chain of compares should be contained by the given parent. -// -// Work backwards through the chain until it is no longer valid or containable. +// CompareChainSize : Determine the size of an uncontained chain. // // Arguments: -// tree - the node. Either a compare or a tree of compares connected by ANDs. -// parent - the parent of the tree. -// earliestValid - Returns the earliest valid compare node in the chain (if any). +// child - pointer to the node being checked. +// parent - parent node of the child. +// inChain - set to true if an exisiting contained chain is found. // -// Return Value: -// True if the chain is valid +// Return value: +// Number of uncontained nodes in the chain. // -bool Lowering::ContainCheckCompareChain(GenTree* tree, GenTree* parent, GenTree** earliestValid) +int Lowering::CompareChainSize(GenTree* child, GenTree* parent, bool* inChain) { - if (tree->OperIs(GT_AND)) + assert(parent->OperIs(GT_AND) || parent->OperIs(GT_SELECT)); + int count = 0; + + if (child->isContainedAndNotIntOrIImmed()) { - // To ensure ordering at code generation, Op1 and the parent can - // only be contained if Op2 is contained. - if (ContainCheckCompareChain(tree->AsOp()->gtGetOp2(), tree, earliestValid)) + // Already have a chain. + *inChain = true; + } + else + { + if (child->OperIs(GT_AND)) { - ContainCheckCompareChain(tree->AsOp()->gtGetOp1(), tree, earliestValid); - tree->SetContained(); - return true; + // Count both sides. + int count2 = CompareChainSize(child->AsOp()->gtGetOp2(), child, inChain); + if (count2 > 0) + { + int count1 = CompareChainSize(child->AsOp()->gtGetOp1(), child, inChain); + + if (count1 > 0) + { + count = count1 + count2 + 1; + } + } + } +#ifdef TARGET_ARM64 + else if (child->OperIsCompare()) + { + // Can the child compare be contained. + if (IsSafeToContainMem(parent, child)) + { + count++; + } } +#endif } - else if (tree->OperIsCompare()) + + return count; +} + +//------------------------------------------------------------------------ +// ContainCheckCompareChain : Determine if a chain of ANDs and CMPs can be contained. +// +// Arguments: +// child - pointer to the node being checked. +// parent - parent node of the child. +// earliestValid - If found, returns the earliest valid op in the chain. +// +bool Lowering::ContainCheckCompareChain(GenTree* child, GenTree* parent, GenTree** earliestValid) +{ + assert(parent->OperIs(GT_AND) || parent->OperIs(GT_SELECT)); + + // Can the child compare be contained. + if (IsSafeToContainMem(parent, child)) { - // Can the compare be contained. - if (IsSafeToContainMem(parent, tree)) + if (child->OperIs(GT_AND)) { - tree->AsOp()->SetContained(); + // If Op2 is not contained, then try to contain it. + if (!child->AsOp()->gtGetOp2()->isContainedAndNotIntOrIImmed()) + { + if (!ContainCheckCompareChain(child->gtGetOp2(), child, earliestValid)) + { + // Op2 must be contained in order to contain Op1 or the AND. + return false; + } + } + + // If Op1 is not contained, then try to contain it. + if (!child->AsOp()->gtGetOp1()->isContainedAndNotIntOrIImmed()) + { + if (!ContainCheckCompareChain(child->gtGetOp1(), child, earliestValid)) + { + return false; + } + } + // Contain the AND. + child->SetContained(); + return true; + } #ifdef TARGET_ARM64 + else if (child->OperIsCompare()) + { + + child->AsOp()->SetContained(); + // Ensure the children of the compare are contained correctly. - tree->AsOp()->gtGetOp1()->ClearContained(); - tree->AsOp()->gtGetOp2()->ClearContained(); - ContainCheckConditionalCompare(tree->AsOp()); -#endif - *earliestValid = tree; + child->AsOp()->gtGetOp1()->ClearContained(); + child->AsOp()->gtGetOp2()->ClearContained(); + ContainCheckConditionalCompare(child->AsOp()); + *earliestValid = child; return true; } +#endif } - // Not a valid compare chain return false; } +//------------------------------------------------------------------------ +// ContainCheckAndCompareChain : Determine if an AND is a containable chain +// +// Arguments: +// node - pointer to the node +// +void Lowering::ContainCheckAndChain(GenTree* tree) +{ + assert(tree->OperIs(GT_AND)); + + if (!comp->opts.OptimizationEnabled()) + { + return; + } + + // Determine whether a chain exists. + bool inChain = false; + int chainSize = 0; + int chainSize2 = CompareChainSize(tree->AsOp()->gtGetOp2(), tree, &inChain); + if (chainSize2 > 0 && !inChain) + { + int chainSize1 = CompareChainSize(tree->AsOp()->gtGetOp1(), tree, &inChain); + if (chainSize1 > 0 && !inChain) + { + chainSize = chainSize1 + chainSize2; + } + } + + // Continue an exisiting chain, or start a new one if it's long enough to be worthwhile. + if (chainSize > 1 || inChain) + { + GenTree* startOfChain = nullptr; + + // To ensure ordering at code generation, Op1 and the parent can + // only be contained if Op2 is contained. + if (ContainCheckCompareChain(tree->AsOp()->gtGetOp2(), tree, &startOfChain)) + { + if (ContainCheckCompareChain(tree->AsOp()->gtGetOp1(), tree, &startOfChain)) + { + // If op1 is the start of a chain, then it'll be generated as a standard compare. + if (startOfChain != nullptr) + { + // The earliest node in the chain will be generated as a standard compare. + assert(startOfChain->OperIsCompare()); + startOfChain->AsOp()->gtGetOp1()->ClearContained(); + startOfChain->AsOp()->gtGetOp2()->ClearContained(); + ContainCheckCompare(startOfChain->AsOp()); + } + } + } + + JITDUMP("Lowered And chain: length=%d %s\n", chainSize, inChain ? "" : "(new chain)"); + DISPTREE(tree); + } +} + //------------------------------------------------------------------------ // ContainCheckSelect : determine whether the source of a select should be contained. // @@ -7069,6 +7197,11 @@ bool Lowering::ContainCheckCompareChain(GenTree* tree, GenTree* parent, GenTree* // void Lowering::ContainCheckSelect(GenTreeConditional* node) { + if (!comp->opts.OptimizationEnabled()) + { + return; + } + // Check if the compare does not need to be generated into a register. GenTree* earliestValid = nullptr; ContainCheckCompareChain(node->gtCond, node, &earliestValid); @@ -7076,6 +7209,7 @@ void Lowering::ContainCheckSelect(GenTreeConditional* node) if (earliestValid != nullptr) { // The earliest node in the chain will be generated as a standard compare. + assert(earliestValid->OperIsCompare()); earliestValid->AsOp()->gtGetOp1()->ClearContained(); earliestValid->AsOp()->gtGetOp2()->ClearContained(); ContainCheckCompare(earliestValid->AsOp()); diff --git a/src/coreclr/jit/lower.h b/src/coreclr/jit/lower.h index 4a98e779a18a0b..5e6c75b06b2932 100644 --- a/src/coreclr/jit/lower.h +++ b/src/coreclr/jit/lower.h @@ -88,7 +88,9 @@ class Lowering final : public Phase #ifdef TARGET_ARM64 void ContainCheckConditionalCompare(GenTreeOp* cmp); #endif - bool ContainCheckCompareChain(GenTree* tree, GenTree* parent, GenTree** earliest_valid); + bool ContainCheckCompareChain(GenTree* child, GenTree* parent, GenTree** earliestValid); + int CompareChainSize(GenTree* child, GenTree* parent, bool* inChain); + void ContainCheckAndChain(GenTree* tree); void ContainCheckSelect(GenTreeConditional* node); void ContainCheckBitCast(GenTree* node); void ContainCheckCallOperands(GenTreeCall* call); diff --git a/src/coreclr/jit/lowerarmarch.cpp b/src/coreclr/jit/lowerarmarch.cpp index 77aac81b1929f2..5132dbf14d77a4 100644 --- a/src/coreclr/jit/lowerarmarch.cpp +++ b/src/coreclr/jit/lowerarmarch.cpp @@ -365,6 +365,10 @@ GenTree* Lowering::LowerBinaryArithmetic(GenTreeOp* binOp) binOp->ChangeOper(GT_AND_NOT); BlockRange().Remove(notNode); } + else + { + ContainCheckAndChain(binOp); + } } ContainCheckBinary(binOp); From 165060b97fa2dc2d374995031a72fe002bccce31 Mon Sep 17 00:00:00 2001 From: Alan Hayward Date: Wed, 3 Aug 2022 12:05:31 +0100 Subject: [PATCH 13/26] Fix unsigned compares && reduce chain check recursion --- src/coreclr/jit/codegenarm64.cpp | 43 ++++++++++++++++++++++++++++---- src/coreclr/jit/lower.cpp | 35 +++++++++++++++----------- 2 files changed, 58 insertions(+), 20 deletions(-) diff --git a/src/coreclr/jit/codegenarm64.cpp b/src/coreclr/jit/codegenarm64.cpp index cf11834f092a11..4c3f443a49b0e0 100644 --- a/src/coreclr/jit/codegenarm64.cpp +++ b/src/coreclr/jit/codegenarm64.cpp @@ -2579,6 +2579,7 @@ void CodeGen::genCodeForBinary(GenTreeOp* tree) // Move the result from flags into a register. genTreeOps opCond = GT_EQ; + bool isUnsigned = false; switch (cond) { case INS_COND_EQ: @@ -2599,10 +2600,26 @@ void CodeGen::genCodeForBinary(GenTreeOp* tree) case INS_COND_LE: opCond = GT_LE; break; + case INS_COND_HS: + isUnsigned = true; + opCond = GT_GE; + break; + case INS_COND_HI: + isUnsigned = true; + opCond = GT_GT; + break; + case INS_COND_LO: + isUnsigned = true; + opCond = GT_LT; + break; + case INS_COND_LS: + isUnsigned = true; + opCond = GT_LE; + break; default: assert(!"Unexpected cond"); } - inst_SETCC(GenCondition::FromIntegralRelop(opCond, false), tree->TypeGet(), targetReg); + inst_SETCC(GenCondition::FromIntegralRelop(opCond, isUnsigned), tree->TypeGet(), targetReg); genProduceReg(tree); return; } @@ -10603,13 +10620,13 @@ insCond CodeGen::InsCondForCompareOp(GenTree* tree) case GT_TEST_NE: return INS_COND_NE; case GT_GE: - return INS_COND_GE; + return tree->IsUnsigned() ? INS_COND_HS : INS_COND_GE; case GT_GT: - return INS_COND_GT; + return tree->IsUnsigned() ? INS_COND_HI : INS_COND_GT; case GT_LT: - return INS_COND_LT; + return tree->IsUnsigned() ? INS_COND_LO : INS_COND_LT; case GT_LE: - return INS_COND_LE; + return tree->IsUnsigned() ? INS_COND_LS : INS_COND_LE; default: assert(false && "Invalid condition"); return INS_COND_EQ; @@ -10638,6 +10655,14 @@ insCond CodeGen::InvertInsCond(insCond cond) return INS_COND_GE; case INS_COND_LE: return INS_COND_GT; + case INS_COND_HS: + return INS_COND_LO; + case INS_COND_HI: + return INS_COND_LS; + case INS_COND_LO: + return INS_COND_HS; + case INS_COND_LS: + return INS_COND_HI; default: assert(false && "Invalid condition"); return INS_COND_EQ; @@ -10673,6 +10698,14 @@ insCflags CodeGen::InsCflagsForCcmp(insCond cond) return INS_FLAGS_NC; case INS_COND_LE: return INS_FLAGS_NZC; + case INS_COND_HS: + return INS_FLAGS_C; + case INS_COND_HI: + return INS_FLAGS_C; + case INS_COND_LO: + return INS_FLAGS_NONE; + case INS_COND_LS: + return INS_FLAGS_Z; default: assert(false && "Invalid condition"); return INS_FLAGS_NONE; diff --git a/src/coreclr/jit/lower.cpp b/src/coreclr/jit/lower.cpp index b4852136691e7e..1c5c30734f25dc 100644 --- a/src/coreclr/jit/lower.cpp +++ b/src/coreclr/jit/lower.cpp @@ -7080,21 +7080,27 @@ int Lowering::CompareChainSize(GenTree* child, GenTree* parent, bool* inChain) // Arguments: // child - pointer to the node being checked. // parent - parent node of the child. -// earliestValid - If found, returns the earliest valid op in the chain. +// startOfChain - If found, returns the earliest valid op in the chain. // -bool Lowering::ContainCheckCompareChain(GenTree* child, GenTree* parent, GenTree** earliestValid) +bool Lowering::ContainCheckCompareChain(GenTree* child, GenTree* parent, GenTree** startOfChain) { assert(parent->OperIs(GT_AND) || parent->OperIs(GT_SELECT)); - // Can the child compare be contained. - if (IsSafeToContainMem(parent, child)) + if (child->isContainedAndNotIntOrIImmed()) + { + // Already have a chain. + *startOfChain = nullptr; + return true; + } + // Can the child be contained. + else if (IsSafeToContainMem(parent, child)) { if (child->OperIs(GT_AND)) { // If Op2 is not contained, then try to contain it. if (!child->AsOp()->gtGetOp2()->isContainedAndNotIntOrIImmed()) { - if (!ContainCheckCompareChain(child->gtGetOp2(), child, earliestValid)) + if (!ContainCheckCompareChain(child->gtGetOp2(), child, startOfChain)) { // Op2 must be contained in order to contain Op1 or the AND. return false; @@ -7104,7 +7110,7 @@ bool Lowering::ContainCheckCompareChain(GenTree* child, GenTree* parent, GenTree // If Op1 is not contained, then try to contain it. if (!child->AsOp()->gtGetOp1()->isContainedAndNotIntOrIImmed()) { - if (!ContainCheckCompareChain(child->gtGetOp1(), child, earliestValid)) + if (!ContainCheckCompareChain(child->gtGetOp1(), child, startOfChain)) { return false; } @@ -7117,14 +7123,13 @@ bool Lowering::ContainCheckCompareChain(GenTree* child, GenTree* parent, GenTree #ifdef TARGET_ARM64 else if (child->OperIsCompare()) { - child->AsOp()->SetContained(); // Ensure the children of the compare are contained correctly. child->AsOp()->gtGetOp1()->ClearContained(); child->AsOp()->gtGetOp2()->ClearContained(); ContainCheckConditionalCompare(child->AsOp()); - *earliestValid = child; + *startOfChain = child; return true; } #endif @@ -7203,16 +7208,16 @@ void Lowering::ContainCheckSelect(GenTreeConditional* node) } // Check if the compare does not need to be generated into a register. - GenTree* earliestValid = nullptr; - ContainCheckCompareChain(node->gtCond, node, &earliestValid); + GenTree* startOfChain = nullptr; + ContainCheckCompareChain(node->gtCond, node, &startOfChain); - if (earliestValid != nullptr) + if (startOfChain != nullptr) { // The earliest node in the chain will be generated as a standard compare. - assert(earliestValid->OperIsCompare()); - earliestValid->AsOp()->gtGetOp1()->ClearContained(); - earliestValid->AsOp()->gtGetOp2()->ClearContained(); - ContainCheckCompare(earliestValid->AsOp()); + assert(startOfChain->OperIsCompare()); + startOfChain->AsOp()->gtGetOp1()->ClearContained(); + startOfChain->AsOp()->gtGetOp2()->ClearContained(); + ContainCheckCompare(startOfChain->AsOp()); } } From 6117333ee7ab36685968fa869d60b3ea3e6cbb2e Mon Sep 17 00:00:00 2001 From: Alan Hayward Date: Wed, 3 Aug 2022 17:45:46 +0100 Subject: [PATCH 14/26] Add compare chain tests --- .../JIT/opt/Compares/compareAnd2Chains.cs | 367 ++++++++++++++++++ .../JIT/opt/Compares/compareAnd2Chains.csproj | 12 + .../JIT/opt/Compares/compareAnd3Chains.cs | 367 ++++++++++++++++++ .../JIT/opt/Compares/compareAnd3Chains.csproj | 12 + 4 files changed, 758 insertions(+) create mode 100644 src/tests/JIT/opt/Compares/compareAnd2Chains.cs create mode 100644 src/tests/JIT/opt/Compares/compareAnd2Chains.csproj create mode 100644 src/tests/JIT/opt/Compares/compareAnd3Chains.cs create mode 100644 src/tests/JIT/opt/Compares/compareAnd3Chains.csproj diff --git a/src/tests/JIT/opt/Compares/compareAnd2Chains.cs b/src/tests/JIT/opt/Compares/compareAnd2Chains.cs new file mode 100644 index 00000000000000..92798c9bbc4067 --- /dev/null +++ b/src/tests/JIT/opt/Compares/compareAnd2Chains.cs @@ -0,0 +1,367 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +// unit test for compare AND chains of length 2. + +using System; +using System.Runtime.CompilerServices; + +public class ComparisonTestAnd2Chains +{ + // Using bitwise AND to ensure compare chains are generated. + + [MethodImpl(MethodImplOptions.NoInlining)] + public static bool Eq_byte_2(byte a1, byte a2) => a1 == 10 & a2 == 11; + + [MethodImpl(MethodImplOptions.NoInlining)] + public static bool Eq_short_2(short a1, short a2) => a1 == 10 & a2 == 11; + + [MethodImpl(MethodImplOptions.NoInlining)] + public static bool Eq_int_2(int a1, int a2) => a1 == 10 & a2 == 11; + + [MethodImpl(MethodImplOptions.NoInlining)] + public static bool Eq_long_2(long a1, long a2) => a1 == 10 & a2 == 11; + + [MethodImpl(MethodImplOptions.NoInlining)] + public static bool Eq_ushort_2(ushort a1, ushort a2) => a1 == 10 & a2 == 11; + + [MethodImpl(MethodImplOptions.NoInlining)] + public static bool Eq_uint_2(uint a1, uint a2) => a1 == 10 & a2 == 11; + + [MethodImpl(MethodImplOptions.NoInlining)] + public static bool Eq_ulong_2(ulong a1, ulong a2) => a1 == 10 & a2 == 11; + + + [MethodImpl(MethodImplOptions.NoInlining)] + public static bool Ne_byte_2(byte a1, byte a2) => a1 != 5 & a2 != 5; + + [MethodImpl(MethodImplOptions.NoInlining)] + public static bool Ne_short_2(short a1, short a2) => a1 != 5 & a2 != 5; + + [MethodImpl(MethodImplOptions.NoInlining)] + public static bool Ne_int_2(int a1, int a2) => a1 != 5 & a2 != 5; + + [MethodImpl(MethodImplOptions.NoInlining)] + public static bool Ne_long_2(long a1, long a2) => a1 != 5 & a2 != 5; + + [MethodImpl(MethodImplOptions.NoInlining)] + public static bool Ne_ushort_2(ushort a1, ushort a2) => a1 != 5 & a2 != 5; + + [MethodImpl(MethodImplOptions.NoInlining)] + public static bool Ne_uint_2(uint a1, uint a2) => a1 != 5 & a2 != 5; + + [MethodImpl(MethodImplOptions.NoInlining)] + public static bool Ne_ulong_2(ulong a1, ulong a2) => a1 != 5 & a2 != 5; + + + [MethodImpl(MethodImplOptions.NoInlining)] + public static bool Lt_byte_2(byte a1, byte a2) => a1 < 5 & a2 < 5; + + [MethodImpl(MethodImplOptions.NoInlining)] + public static bool Lt_short_2(short a1, short a2) => a1 < 5 & a2 < 5; + + [MethodImpl(MethodImplOptions.NoInlining)] + public static bool Lt_int_2(int a1, int a2) => a1 < 5 & a2 < 5; + + [MethodImpl(MethodImplOptions.NoInlining)] + public static bool Lt_long_2(long a1, long a2) => a1 < 5 & a2 < 5; + + [MethodImpl(MethodImplOptions.NoInlining)] + public static bool Lt_ushort_2(ushort a1, ushort a2) => a1 < 5 & a2 < 5; + + [MethodImpl(MethodImplOptions.NoInlining)] + public static bool Lt_uint_2(uint a1, uint a2) => a1 < 5 & a2 < 5; + + [MethodImpl(MethodImplOptions.NoInlining)] + public static bool Lt_ulong_2(ulong a1, ulong a2) => a1 < 5 & a2 < 5; + + + [MethodImpl(MethodImplOptions.NoInlining)] + public static bool Le_byte_2(byte a1, byte a2) => a1 <= 5 & a2 <= 5; + + [MethodImpl(MethodImplOptions.NoInlining)] + public static bool Le_short_2(short a1, short a2) => a1 <= 5 & a2 <= 5; + + [MethodImpl(MethodImplOptions.NoInlining)] + public static bool Le_int_2(int a1, int a2) => a1 <= 5 & a2 <= 5; + + [MethodImpl(MethodImplOptions.NoInlining)] + public static bool Le_long_2(long a1, long a2) => a1 <= 5 & a2 <= 5; + + [MethodImpl(MethodImplOptions.NoInlining)] + public static bool Le_ushort_2(ushort a1, ushort a2) => a1 <= 5 & a2 <= 5; + + [MethodImpl(MethodImplOptions.NoInlining)] + public static bool Le_uint_2(uint a1, uint a2) => a1 <= 5 & a2 <= 5; + + [MethodImpl(MethodImplOptions.NoInlining)] + public static bool Le_ulong_2(ulong a1, ulong a2) => a1 <= 5 & a2 <= 5; + + + [MethodImpl(MethodImplOptions.NoInlining)] + public static bool Gt_byte_2(byte a1, byte a2) => a1 > 5 & a2 > 5; + + [MethodImpl(MethodImplOptions.NoInlining)] + public static bool Gt_short_2(short a1, short a2) => a1 > 5 & a2 > 5; + + [MethodImpl(MethodImplOptions.NoInlining)] + public static bool Gt_int_2(int a1, int a2) => a1 > 5 & a2 > 5; + + [MethodImpl(MethodImplOptions.NoInlining)] + public static bool Gt_long_2(long a1, long a2) => a1 > 5 & a2 > 5; + + [MethodImpl(MethodImplOptions.NoInlining)] + public static bool Gt_ushort_2(ushort a1, ushort a2) => a1 > 5 & a2 > 5; + + [MethodImpl(MethodImplOptions.NoInlining)] + public static bool Gt_uint_2(uint a1, uint a2) => a1 > 5 & a2 > 5; + + [MethodImpl(MethodImplOptions.NoInlining)] + public static bool Gt_ulong_2(ulong a1, ulong a2) => a1 > 5 & a2 > 5; + + + [MethodImpl(MethodImplOptions.NoInlining)] + public static bool Ge_byte_2(byte a1, byte a2) => a1 >= 5 & a2 >= 5; + + [MethodImpl(MethodImplOptions.NoInlining)] + public static bool Ge_short_2(short a1, short a2) => a1 >= 5 & a2 >= 5; + + [MethodImpl(MethodImplOptions.NoInlining)] + public static bool Ge_int_2(int a1, int a2) => a1 >= 5 & a2 >= 5; + + [MethodImpl(MethodImplOptions.NoInlining)] + public static bool Ge_long_2(long a1, long a2) => a1 >= 5 & a2 >= 5; + + [MethodImpl(MethodImplOptions.NoInlining)] + public static bool Ge_ushort_2(ushort a1, ushort a2) => a1 >= 5 & a2 >= 5; + + [MethodImpl(MethodImplOptions.NoInlining)] + public static bool Ge_uint_2(uint a1, uint a2) => a1 >= 5 & a2 >= 5; + + [MethodImpl(MethodImplOptions.NoInlining)] + public static bool Ge_ulong_2(ulong a1, ulong a2) => a1 >= 5 & a2 >= 5; + + + [MethodImpl(MethodImplOptions.NoInlining)] + public static int Main() + { + if (!Eq_byte_2(10, 11)) + { + Console.WriteLine("ComparisonTestAnd2Chains:Eq_byte_2(10, 11) failed"); + return 101; + } + if (!Eq_short_2(10, 11)) + { + Console.WriteLine("ComparisonTestAnd2Chains:Eq_short_2(10, 11) failed"); + return 101; + } + if (!Eq_int_2(10, 11)) + { + Console.WriteLine("ComparisonTestAnd2Chains:Eq_int_2(10, 11) failed"); + return 101; + } + if (!Eq_long_2(10, 11)) + { + Console.WriteLine("ComparisonTestAnd2Chains:Eq_long_2(10, 11) failed"); + return 101; + } + if (!Eq_ushort_2(10, 11)) + { + Console.WriteLine("ComparisonTestAnd2Chains:Eq_short_2(10, 11) failed"); + return 101; + } + if (!Eq_uint_2(10, 11)) + { + Console.WriteLine("ComparisonTestAnd2Chains:Eq_int_2(10, 11) failed"); + return 101; + } + if (!Eq_ulong_2(10, 11)) + { + Console.WriteLine("ComparisonTestAnd2Chains:Eq_long_2(10, 11) failed"); + return 101; + } + + if (!Ne_byte_2(10, 11)) + { + Console.WriteLine("ComparisonTestAnd2Chains:Ne_byte_2(10, 11) failed"); + return 101; + } + if (!Ne_short_2(10, 11)) + { + Console.WriteLine("ComparisonTestAnd2Chains:Ne_short_2(10, 11) failed"); + return 101; + } + if (!Ne_int_2(10, 11)) + { + Console.WriteLine("ComparisonTestAnd2Chains:Ne_int_2(10, 11) failed"); + return 101; + } + if (!Ne_long_2(10, 11)) + { + Console.WriteLine("ComparisonTestAnd2Chains:Ne_long_2(10, 11) failed"); + return 101; + } + if (!Ne_ushort_2(10, 11)) + { + Console.WriteLine("ComparisonTestAnd2Chains:Ne_short_2(10, 11) failed"); + return 101; + } + if (!Ne_uint_2(10, 11)) + { + Console.WriteLine("ComparisonTestAnd2Chains:Ne_int_2(10, 11) failed"); + return 101; + } + if (!Ne_ulong_2(10, 11)) + { + Console.WriteLine("ComparisonTestAnd2Chains:Ne_long_2(10, 11) failed"); + return 101; + } + + if (!Lt_byte_2(3, 4)) + { + Console.WriteLine("ComparisonTestAnd2Chains:Lt_byte_2(3, 4) failed"); + return 101; + } + if (!Lt_short_2(3, 4)) + { + Console.WriteLine("ComparisonTestAnd2Chains:Lt_short_2(3, 4) failed"); + return 101; + } + if (!Lt_int_2(3, 4)) + { + Console.WriteLine("ComparisonTestAnd2Chains:Lt_int_2(3, 4) failed"); + return 101; + } + if (!Lt_long_2(3, 4)) + { + Console.WriteLine("ComparisonTestAnd2Chains:Lt_long_2(3, 4) failed"); + return 101; + } + if (!Lt_ushort_2(3, 4)) + { + Console.WriteLine("ComparisonTestAnd2Chains:Lt_short_2(3, 4) failed"); + return 101; + } + if (!Lt_uint_2(3, 4)) + { + Console.WriteLine("ComparisonTestAnd2Chains:Lt_int_2(3, 4) failed"); + return 101; + } + if (!Lt_ulong_2(3, 4)) + { + Console.WriteLine("ComparisonTestAnd2Chains:Lt_long_2(3, 4) failed"); + return 101; + } + + if (!Le_byte_2(3, 4)) + { + Console.WriteLine("ComparisonTestAnd2Chains:Le_byte_2(3, 4) failed"); + return 101; + } + if (!Le_short_2(3, 4)) + { + Console.WriteLine("ComparisonTestAnd2Chains:Le_short_2(3, 4) failed"); + return 101; + } + if (!Le_int_2(3, 4)) + { + Console.WriteLine("ComparisonTestAnd2Chains:Le_int_2(3, 4) failed"); + return 101; + } + if (!Le_long_2(3, 4)) + { + Console.WriteLine("ComparisonTestAnd2Chains:Le_long_2(3, 4) failed"); + return 101; + } + if (!Le_ushort_2(3, 4)) + { + Console.WriteLine("ComparisonTestAnd2Chains:Le_short_2(3, 4) failed"); + return 101; + } + if (!Le_uint_2(3, 4)) + { + Console.WriteLine("ComparisonTestAnd2Chains:Le_int_2(3, 4) failed"); + return 101; + } + if (!Le_ulong_2(3, 4)) + { + Console.WriteLine("ComparisonTestAnd2Chains:Le_long_2(3, 4) failed"); + return 101; + } + + if (!Gt_byte_2(10, 11)) + { + Console.WriteLine("ComparisonTestAnd2Chains:Gt_byte_2(10, 11) failed"); + return 101; + } + if (!Gt_short_2(10, 11)) + { + Console.WriteLine("ComparisonTestAnd2Chains:Gt_short_2(10, 11) failed"); + return 101; + } + if (!Gt_int_2(10, 11)) + { + Console.WriteLine("ComparisonTestAnd2Chains:Gt_int_2(10, 11) failed"); + return 101; + } + if (!Gt_long_2(10, 11)) + { + Console.WriteLine("ComparisonTestAnd2Chains:Gt_long_2(10, 11) failed"); + return 101; + } + if (!Gt_ushort_2(10, 11)) + { + Console.WriteLine("ComparisonTestAnd2Chains:Gt_short_2(10, 11) failed"); + return 101; + } + if (!Gt_uint_2(10, 11)) + { + Console.WriteLine("ComparisonTestAnd2Chains:Gt_int_2(10, 11) failed"); + return 101; + } + if (!Gt_ulong_2(10, 11)) + { + Console.WriteLine("ComparisonTestAnd2Chains:Gt_long_2(10, 11) failed"); + return 101; + } + + if (!Ge_byte_2(10, 11)) + { + Console.WriteLine("ComparisonTestAnd2Chains:Ge_byte_2(10, 11) failed"); + return 101; + } + if (!Ge_short_2(10, 11)) + { + Console.WriteLine("ComparisonTestAnd2Chains:Ge_short_2(10, 11) failed"); + return 101; + } + if (!Ge_int_2(10, 11)) + { + Console.WriteLine("ComparisonTestAnd2Chains:Ge_int_2(10, 11) failed"); + return 101; + } + if (!Ge_long_2(10, 11)) + { + Console.WriteLine("ComparisonTestAnd2Chains:Ge_long_2(10, 11) failed"); + return 101; + } + if (!Ge_ushort_2(10, 11)) + { + Console.WriteLine("ComparisonTestAnd2Chains:Ge_short_2(10, 11) failed"); + return 101; + } + if (!Ge_uint_2(10, 11)) + { + Console.WriteLine("ComparisonTestAnd2Chains:Ge_int_2(10, 11) failed"); + return 101; + } + if (!Ge_ulong_2(10, 11)) + { + Console.WriteLine("ComparisonTestAnd2Chains:Le_long_2(10, 11) failed"); + return 101; + } + + Console.WriteLine("PASSED"); + return 100; + } +} diff --git a/src/tests/JIT/opt/Compares/compareAnd2Chains.csproj b/src/tests/JIT/opt/Compares/compareAnd2Chains.csproj new file mode 100644 index 00000000000000..5e5fbae5cb863b --- /dev/null +++ b/src/tests/JIT/opt/Compares/compareAnd2Chains.csproj @@ -0,0 +1,12 @@ + + + Exe + + + PdbOnly + True + + + + + diff --git a/src/tests/JIT/opt/Compares/compareAnd3Chains.cs b/src/tests/JIT/opt/Compares/compareAnd3Chains.cs new file mode 100644 index 00000000000000..3de83e911deafc --- /dev/null +++ b/src/tests/JIT/opt/Compares/compareAnd3Chains.cs @@ -0,0 +1,367 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +// unit test for compare AND chains of length 3. + +using System; +using System.Runtime.CompilerServices; + +public class ComparisonTestAnd3Chains +{ + // Using bitwise AND to ensure compare chains are generated. + + [MethodImpl(MethodImplOptions.NoInlining)] + public static bool Eq_byte_3(byte a1, byte a2, byte a3) => a1 == 10 & a2 == 11 & a3 == 12; + + [MethodImpl(MethodImplOptions.NoInlining)] + public static bool Eq_short_3(short a1, short a2, short a3) => a1 == 10 & a2 == 11 & a3 == 12; + + [MethodImpl(MethodImplOptions.NoInlining)] + public static bool Eq_int_3(int a1, int a2, int a3) => a1 == 10 & a2 == 11 & a3 == 12; + + [MethodImpl(MethodImplOptions.NoInlining)] + public static bool Eq_long_3(long a1, long a2, long a3) => a1 == 10 & a2 == 11 & a3 == 12; + + [MethodImpl(MethodImplOptions.NoInlining)] + public static bool Eq_ushort_3(ushort a1, ushort a2, ushort a3) => a1 == 10 & a2 == 11 & a3 == 12; + + [MethodImpl(MethodImplOptions.NoInlining)] + public static bool Eq_uint_3(uint a1, uint a2, uint a3) => a1 == 10 & a2 == 11 & a3 == 12; + + [MethodImpl(MethodImplOptions.NoInlining)] + public static bool Eq_ulong_3(ulong a1, ulong a2, ulong a3) => a1 == 10 & a2 == 11 & a3 == 12; + + + [MethodImpl(MethodImplOptions.NoInlining)] + public static bool Ne_byte_3(byte a1, byte a2, byte a3) => a1 != 5 & a2 != 5 & a3 != 5; + + [MethodImpl(MethodImplOptions.NoInlining)] + public static bool Ne_short_3(short a1, short a2, short a3) => a1 != 5 & a2 != 5 & a3 != 5; + + [MethodImpl(MethodImplOptions.NoInlining)] + public static bool Ne_int_3(int a1, int a2, int a3) => a1 != 5 & a2 != 5 & a3 != 5; + + [MethodImpl(MethodImplOptions.NoInlining)] + public static bool Ne_long_3(long a1, long a2, long a3) => a1 != 5 & a2 != 5 & a3 != 5; + + [MethodImpl(MethodImplOptions.NoInlining)] + public static bool Ne_ushort_3(ushort a1, ushort a2, ushort a3) => a1 != 5 & a2 != 5 & a3 != 5; + + [MethodImpl(MethodImplOptions.NoInlining)] + public static bool Ne_uint_3(uint a1, uint a2, uint a3) => a1 != 5 & a2 != 5 & a3 != 5; + + [MethodImpl(MethodImplOptions.NoInlining)] + public static bool Ne_ulong_3(ulong a1, ulong a2, ulong a3) => a1 != 5 & a2 != 5 & a3 != 5; + + + [MethodImpl(MethodImplOptions.NoInlining)] + public static bool Lt_byte_3(byte a1, byte a2, byte a3) => a1 < 5 & a2 < 5 & a3 < 5; + + [MethodImpl(MethodImplOptions.NoInlining)] + public static bool Lt_short_3(short a1, short a2, short a3) => a1 < 5 & a2 < 5 & a3 < 5; + + [MethodImpl(MethodImplOptions.NoInlining)] + public static bool Lt_int_3(int a1, int a2, int a3) => a1 < 5 & a2 < 5 & a3 < 5; + + [MethodImpl(MethodImplOptions.NoInlining)] + public static bool Lt_long_3(long a1, long a2, long a3) => a1 < 5 & a2 < 5 & a3 < 5; + + [MethodImpl(MethodImplOptions.NoInlining)] + public static bool Lt_ushort_3(ushort a1, ushort a2, ushort a3) => a1 < 5 & a2 < 5 & a3 < 5; + + [MethodImpl(MethodImplOptions.NoInlining)] + public static bool Lt_uint_3(uint a1, uint a2, uint a3) => a1 < 5 & a2 < 5 & a3 < 5; + + [MethodImpl(MethodImplOptions.NoInlining)] + public static bool Lt_ulong_3(ulong a1, ulong a2, ulong a3) => a1 < 5 & a2 < 5 & a3 < 5; + + + [MethodImpl(MethodImplOptions.NoInlining)] + public static bool Le_byte_3(byte a1, byte a2, byte a3) => a1 <= 5 & a2 <= 5 & a3 <= 5; + + [MethodImpl(MethodImplOptions.NoInlining)] + public static bool Le_short_3(short a1, short a2, short a3) => a1 <= 5 & a2 <= 5 & a3 <= 5; + + [MethodImpl(MethodImplOptions.NoInlining)] + public static bool Le_int_3(int a1, int a2, int a3) => a1 <= 5 & a2 <= 5 & a3 <= 5; + + [MethodImpl(MethodImplOptions.NoInlining)] + public static bool Le_long_3(long a1, long a2, long a3) => a1 <= 5 & a2 <= 5 & a3 <= 5; + + [MethodImpl(MethodImplOptions.NoInlining)] + public static bool Le_ushort_3(ushort a1, ushort a2, ushort a3) => a1 <= 5 & a2 <= 5 & a3 <= 5; + + [MethodImpl(MethodImplOptions.NoInlining)] + public static bool Le_uint_3(uint a1, uint a2, uint a3) => a1 <= 5 & a2 <= 5 & a3 <= 5; + + [MethodImpl(MethodImplOptions.NoInlining)] + public static bool Le_ulong_3(ulong a1, ulong a2, ulong a3) => a1 <= 5 & a2 <= 5 & a3 <= 5; + + + [MethodImpl(MethodImplOptions.NoInlining)] + public static bool Gt_byte_3(byte a1, byte a2, byte a3) => a1 > 5 & a2 > 5 & a3 > 5; + + [MethodImpl(MethodImplOptions.NoInlining)] + public static bool Gt_short_3(short a1, short a2, short a3) => a1 > 5 & a2 > 5 & a3 > 5; + + [MethodImpl(MethodImplOptions.NoInlining)] + public static bool Gt_int_3(int a1, int a2, int a3) => a1 > 5 & a2 > 5 & a3 > 5; + + [MethodImpl(MethodImplOptions.NoInlining)] + public static bool Gt_long_3(long a1, long a2, long a3) => a1 > 5 & a2 > 5 & a3 > 5; + + [MethodImpl(MethodImplOptions.NoInlining)] + public static bool Gt_ushort_3(ushort a1, ushort a2, ushort a3) => a1 > 5 & a2 > 5 & a3 > 5; + + [MethodImpl(MethodImplOptions.NoInlining)] + public static bool Gt_uint_3(uint a1, uint a2, uint a3) => a1 > 5 & a2 > 5 & a3 > 5; + + [MethodImpl(MethodImplOptions.NoInlining)] + public static bool Gt_ulong_3(ulong a1, ulong a2, ulong a3) => a1 > 5 & a2 > 5 & a3 > 5; + + + [MethodImpl(MethodImplOptions.NoInlining)] + public static bool Ge_byte_3(byte a1, byte a2, byte a3) => a1 >= 5 & a2 >= 5 & a3 >= 5; + + [MethodImpl(MethodImplOptions.NoInlining)] + public static bool Ge_short_3(short a1, short a2, short a3) => a1 >= 5 & a2 >= 5 & a3 >= 5; + + [MethodImpl(MethodImplOptions.NoInlining)] + public static bool Ge_int_3(int a1, int a2, int a3) => a1 >= 5 & a2 >= 5 & a3 >= 5; + + [MethodImpl(MethodImplOptions.NoInlining)] + public static bool Ge_long_3(long a1, long a2, long a3) => a1 >= 5 & a2 >= 5 & a3 >= 5; + + [MethodImpl(MethodImplOptions.NoInlining)] + public static bool Ge_ushort_3(ushort a1, ushort a2, ushort a3) => a1 >= 5 & a2 >= 5 & a3 >= 5; + + [MethodImpl(MethodImplOptions.NoInlining)] + public static bool Ge_uint_3(uint a1, uint a2, uint a3) => a1 >= 5 & a2 >= 5 & a3 >= 5; + + [MethodImpl(MethodImplOptions.NoInlining)] + public static bool Ge_ulong_3(ulong a1, ulong a2, ulong a3) => a1 >= 5 & a2 >= 5 & a3 >= 5; + + + [MethodImpl(MethodImplOptions.NoInlining)] + public static int Main() + { + if (!Eq_byte_3(10, 11, 12)) + { + Console.WriteLine("ComparisonTestAnd2Chains:Eq_byte_3(10, 11, 12) failed"); + return 101; + } + if (!Eq_short_3(10, 11, 12)) + { + Console.WriteLine("ComparisonTestAnd2Chains:Eq_short_3(10, 11, 12) failed"); + return 101; + } + if (!Eq_int_3(10, 11, 12)) + { + Console.WriteLine("ComparisonTestAnd2Chains:Eq_int_3(10, 11, 12) failed"); + return 101; + } + if (!Eq_long_3(10, 11, 12)) + { + Console.WriteLine("ComparisonTestAnd2Chains:Eq_long_3(10, 11, 12) failed"); + return 101; + } + if (!Eq_ushort_3(10, 11, 12)) + { + Console.WriteLine("ComparisonTestAnd2Chains:Eq_short_3(10, 11, 12) failed"); + return 101; + } + if (!Eq_uint_3(10, 11, 12)) + { + Console.WriteLine("ComparisonTestAnd2Chains:Eq_int_3(10, 11, 12) failed"); + return 101; + } + if (!Eq_ulong_3(10, 11, 12)) + { + Console.WriteLine("ComparisonTestAnd2Chains:Eq_long_3(10, 11, 12) failed"); + return 101; + } + + if (!Ne_byte_3(10, 11, 12)) + { + Console.WriteLine("ComparisonTestAnd2Chains:Ne_byte_3(10, 11, 12) failed"); + return 101; + } + if (!Ne_short_3(10, 11, 12)) + { + Console.WriteLine("ComparisonTestAnd2Chains:Ne_short_3(10, 11, 12) failed"); + return 101; + } + if (!Ne_int_3(10, 11, 12)) + { + Console.WriteLine("ComparisonTestAnd2Chains:Ne_int_3(10, 11, 12) failed"); + return 101; + } + if (!Ne_long_3(10, 11, 12)) + { + Console.WriteLine("ComparisonTestAnd2Chains:Ne_long_3(10, 11, 12) failed"); + return 101; + } + if (!Ne_ushort_3(10, 11, 12)) + { + Console.WriteLine("ComparisonTestAnd2Chains:Ne_short_3(10, 11, 12) failed"); + return 101; + } + if (!Ne_uint_3(10, 11, 12)) + { + Console.WriteLine("ComparisonTestAnd2Chains:Ne_int_3(10, 11, 12) failed"); + return 101; + } + if (!Ne_ulong_3(10, 11, 12)) + { + Console.WriteLine("ComparisonTestAnd2Chains:Ne_long_3(10, 11, 12) failed"); + return 101; + } + + if (!Lt_byte_3(2, 3, 4)) + { + Console.WriteLine("ComparisonTestAnd2Chains:Lt_byte_3(2, 3, 4) failed"); + return 101; + } + if (!Lt_short_3(2, 3, 4)) + { + Console.WriteLine("ComparisonTestAnd2Chains:Lt_short_3(2, 3, 4) failed"); + return 101; + } + if (!Lt_int_3(2, 3, 4)) + { + Console.WriteLine("ComparisonTestAnd2Chains:Lt_int_3(2, 3, 4) failed"); + return 101; + } + if (!Lt_long_3(2, 3, 4)) + { + Console.WriteLine("ComparisonTestAnd2Chains:Lt_long_3(2, 3, 4) failed"); + return 101; + } + if (!Lt_ushort_3(2, 3, 4)) + { + Console.WriteLine("ComparisonTestAnd2Chains:Lt_short_3(2, 3, 4) failed"); + return 101; + } + if (!Lt_uint_3(2, 3, 4)) + { + Console.WriteLine("ComparisonTestAnd2Chains:Lt_int_3(2, 3, 4) failed"); + return 101; + } + if (!Lt_ulong_3(2, 3, 4)) + { + Console.WriteLine("ComparisonTestAnd2Chains:Lt_long_3(2, 3, 4) failed"); + return 101; + } + + if (!Le_byte_3(2, 3, 4)) + { + Console.WriteLine("ComparisonTestAnd2Chains:Le_byte_3(2, 3, 4) failed"); + return 101; + } + if (!Le_short_3(2, 3, 4)) + { + Console.WriteLine("ComparisonTestAnd2Chains:Le_short_3(2, 3, 4) failed"); + return 101; + } + if (!Le_int_3(2, 3, 4)) + { + Console.WriteLine("ComparisonTestAnd2Chains:Le_int_3(2, 3, 4) failed"); + return 101; + } + if (!Le_long_3(2, 3, 4)) + { + Console.WriteLine("ComparisonTestAnd2Chains:Le_long_3(2, 3, 4) failed"); + return 101; + } + if (!Le_ushort_3(2, 3, 4)) + { + Console.WriteLine("ComparisonTestAnd2Chains:Le_short_3(2, 3, 4) failed"); + return 101; + } + if (!Le_uint_3(2, 3, 4)) + { + Console.WriteLine("ComparisonTestAnd2Chains:Le_int_3(2, 3, 4) failed"); + return 101; + } + if (!Le_ulong_3(2, 3, 4)) + { + Console.WriteLine("ComparisonTestAnd2Chains:Le_long_3(2, 3, 4) failed"); + return 101; + } + + if (!Gt_byte_3(10, 11, 12)) + { + Console.WriteLine("ComparisonTestAnd2Chains:Gt_byte_3(10, 11, 12) failed"); + return 101; + } + if (!Gt_short_3(10, 11, 12)) + { + Console.WriteLine("ComparisonTestAnd2Chains:Gt_short_3(10, 11, 12) failed"); + return 101; + } + if (!Gt_int_3(10, 11, 12)) + { + Console.WriteLine("ComparisonTestAnd2Chains:Gt_int_3(10, 11, 12) failed"); + return 101; + } + if (!Gt_long_3(10, 11, 12)) + { + Console.WriteLine("ComparisonTestAnd2Chains:Gt_long_3(10, 11, 12) failed"); + return 101; + } + if (!Gt_ushort_3(10, 11, 12)) + { + Console.WriteLine("ComparisonTestAnd2Chains:Gt_short_3(10, 11, 12) failed"); + return 101; + } + if (!Gt_uint_3(10, 11, 12)) + { + Console.WriteLine("ComparisonTestAnd2Chains:Gt_int_3(10, 11, 12) failed"); + return 101; + } + if (!Gt_ulong_3(10, 11, 12)) + { + Console.WriteLine("ComparisonTestAnd2Chains:Gt_long_3(10, 11, 12) failed"); + return 101; + } + + if (!Ge_byte_3(10, 11, 12)) + { + Console.WriteLine("ComparisonTestAnd2Chains:Ge_byte_3(10, 11, 12) failed"); + return 101; + } + if (!Ge_short_3(10, 11, 12)) + { + Console.WriteLine("ComparisonTestAnd2Chains:Ge_short_3(10, 11, 12) failed"); + return 101; + } + if (!Ge_int_3(10, 11, 12)) + { + Console.WriteLine("ComparisonTestAnd2Chains:Ge_int_3(10, 11, 12) failed"); + return 101; + } + if (!Ge_long_3(10, 11, 12)) + { + Console.WriteLine("ComparisonTestAnd2Chains:Ge_long_3(10, 11, 12) failed"); + return 101; + } + if (!Ge_ushort_3(10, 11, 12)) + { + Console.WriteLine("ComparisonTestAnd2Chains:Ge_short_3(10, 11, 12) failed"); + return 101; + } + if (!Ge_uint_3(10, 11, 12)) + { + Console.WriteLine("ComparisonTestAnd2Chains:Ge_int_3(10, 11, 12) failed"); + return 101; + } + if (!Ge_ulong_3(10, 11, 12)) + { + Console.WriteLine("ComparisonTestAnd2Chains:Le_long_3(10, 11, 12) failed"); + return 101; + } + + Console.WriteLine("PASSED"); + return 100; + } +} diff --git a/src/tests/JIT/opt/Compares/compareAnd3Chains.csproj b/src/tests/JIT/opt/Compares/compareAnd3Chains.csproj new file mode 100644 index 00000000000000..5e5fbae5cb863b --- /dev/null +++ b/src/tests/JIT/opt/Compares/compareAnd3Chains.csproj @@ -0,0 +1,12 @@ + + + Exe + + + PdbOnly + True + + + + + From 574742fe575afbce2c11c5dc6c26b0e71962a881 Mon Sep 17 00:00:00 2001 From: Alan Hayward Date: Wed, 3 Aug 2022 18:02:41 +0100 Subject: [PATCH 15/26] Review fixes --- src/coreclr/jit/codegenarm64.cpp | 18 +++++++++--------- src/coreclr/jit/codegenarmarch.cpp | 5 ++++- 2 files changed, 13 insertions(+), 10 deletions(-) diff --git a/src/coreclr/jit/codegenarm64.cpp b/src/coreclr/jit/codegenarm64.cpp index 4c3f443a49b0e0..5e8021c27c7cbf 100644 --- a/src/coreclr/jit/codegenarm64.cpp +++ b/src/coreclr/jit/codegenarm64.cpp @@ -2569,7 +2569,7 @@ void CodeGen::genCodeForBinary(GenTreeOp* tree) else { // Op1 is not contained, move it from a register into flags. - emit->emitIns_R_I(INS_cmp, EA_ATTR(genTypeSize(op1)), op1->GetRegNum(), 0); + emit->emitIns_R_I(INS_cmp, emitActualTypeSize(op1), op1->GetRegNum(), 0); cond = INS_COND_NE; chain = true; } @@ -2578,8 +2578,8 @@ void CodeGen::genCodeForBinary(GenTreeOp* tree) assert(chain); // Move the result from flags into a register. - genTreeOps opCond = GT_EQ; - bool isUnsigned = false; + genTreeOps opCond = GT_EQ; + bool isUnsigned = false; switch (cond) { case INS_COND_EQ: @@ -2602,19 +2602,19 @@ void CodeGen::genCodeForBinary(GenTreeOp* tree) break; case INS_COND_HS: isUnsigned = true; - opCond = GT_GE; + opCond = GT_GE; break; case INS_COND_HI: isUnsigned = true; - opCond = GT_GT; + opCond = GT_GT; break; case INS_COND_LO: isUnsigned = true; - opCond = GT_LT; + opCond = GT_LT; break; case INS_COND_LS: isUnsigned = true; - opCond = GT_LE; + opCond = GT_LE; break; default: assert(!"Unexpected cond"); @@ -4556,7 +4556,7 @@ void CodeGen::genCodeForContainedCompareChain(GenTree* tree, bool* inChain, insC else { emitter* emit = GetEmitter(); - emit->emitIns_R_I(INS_cmp, EA_ATTR(genTypeSize(op1)), op1->GetRegNum(), 0); + emit->emitIns_R_I(INS_cmp, emitActualTypeSize(op1), op1->GetRegNum(), 0); *prevcond = INS_COND_NE; *inChain = true; } @@ -4619,7 +4619,7 @@ void CodeGen::genCodeForSelect(GenTreeConditional* tree) { // Condition has been generated into a register - move it into flags. genConsumeReg(opcond); - emit->emitIns_R_I(INS_cmp, EA_ATTR(genTypeSize(opcond)), opcond->GetRegNum(), 0); + emit->emitIns_R_I(INS_cmp, emitActualTypeSize(opcond), opcond->GetRegNum(), 0); cond = INS_COND_NE; } diff --git a/src/coreclr/jit/codegenarmarch.cpp b/src/coreclr/jit/codegenarmarch.cpp index c95c84b72902ab..536797eaba9593 100644 --- a/src/coreclr/jit/codegenarmarch.cpp +++ b/src/coreclr/jit/codegenarmarch.cpp @@ -363,7 +363,10 @@ void CodeGen::genCodeForTreeNode(GenTree* treeNode) #ifdef TARGET_ARM64 case GT_TEST_EQ: case GT_TEST_NE: - // Consume early to ensure chains consume correctly. + // On ARM64 genCodeForCompare does not consume its own operands because + // genCodeForBinary also has this behavior and it can end up calling + // genCodeForCompare when generating compare chains for GT_AND. + // Thus, we must do it here. genConsumeOperands(treeNode->AsOp()); #endif // TARGET_ARM64 genCodeForCompare(treeNode->AsOp()); From eae824a04aa370148d2e6a9803c2f584d40d1299 Mon Sep 17 00:00:00 2001 From: Alan Hayward Date: Thu, 4 Aug 2022 11:18:06 +0100 Subject: [PATCH 16/26] Use GenCondition --- src/coreclr/jit/codegen.h | 9 +- src/coreclr/jit/codegenarm64.cpp | 232 ++++++++++++------------------- src/coreclr/jit/instr.cpp | 55 +------- 3 files changed, 92 insertions(+), 204 deletions(-) diff --git a/src/coreclr/jit/codegen.h b/src/coreclr/jit/codegen.h index 4f93d4a34bc3d7..cb1701f0ff6b0a 100644 --- a/src/coreclr/jit/codegen.h +++ b/src/coreclr/jit/codegen.h @@ -1047,8 +1047,8 @@ XXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXX void genCkfinite(GenTree* treeNode); void genCodeForCompare(GenTreeOp* tree); #ifdef TARGET_ARM64 - void genCodeForConditionalCompare(GenTreeOp* tree, insCond cond); - void genCodeForContainedCompareChain(GenTree* tree, bool* inchain, insCond* prevcond); + void genCodeForConditionalCompare(GenTreeOp* tree, GenCondition prevCond); + void genCodeForContainedCompareChain(GenTree* tree, bool* inchain, GenCondition* prevCond); void genCodeForSelect(GenTreeConditional* tree); #endif void genIntrinsic(GenTree* treeNode); @@ -1714,9 +1714,8 @@ XXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXX #endif // TARGET_XARCH #ifdef TARGET_ARM64 - static insCond InsCondForCompareOp(GenTree* tree); - static insCond InvertInsCond(insCond cond); - static insCflags InsCflagsForCcmp(insCond cond); + static insCflags InsCflagsForCcmp(GenCondition cond); + static insCond JumpKindToInsCond(emitJumpKind condition); #endif #ifndef TARGET_LOONGARCH64 diff --git a/src/coreclr/jit/codegenarm64.cpp b/src/coreclr/jit/codegenarm64.cpp index 5e8021c27c7cbf..1da5e76a7b9838 100644 --- a/src/coreclr/jit/codegenarm64.cpp +++ b/src/coreclr/jit/codegenarm64.cpp @@ -2557,8 +2557,8 @@ void CodeGen::genCodeForBinary(GenTreeOp* tree) if (tree->OperIs(GT_AND) && op2->isContainedAndNotIntOrIImmed()) { - insCond cond = INS_COND_EQ; // Dummy value. - bool chain = false; + GenCondition cond; + bool chain = false; if (op1->isContained()) { @@ -2570,7 +2570,7 @@ void CodeGen::genCodeForBinary(GenTreeOp* tree) { // Op1 is not contained, move it from a register into flags. emit->emitIns_R_I(INS_cmp, emitActualTypeSize(op1), op1->GetRegNum(), 0); - cond = INS_COND_NE; + cond = GenCondition::NE; chain = true; } // Gen Op2 into flags. @@ -2578,48 +2578,7 @@ void CodeGen::genCodeForBinary(GenTreeOp* tree) assert(chain); // Move the result from flags into a register. - genTreeOps opCond = GT_EQ; - bool isUnsigned = false; - switch (cond) - { - case INS_COND_EQ: - opCond = GT_EQ; - break; - case INS_COND_NE: - opCond = GT_NE; - break; - case INS_COND_GE: - opCond = GT_GE; - break; - case INS_COND_LT: - opCond = GT_LT; - break; - case INS_COND_GT: - opCond = GT_GT; - break; - case INS_COND_LE: - opCond = GT_LE; - break; - case INS_COND_HS: - isUnsigned = true; - opCond = GT_GE; - break; - case INS_COND_HI: - isUnsigned = true; - opCond = GT_GT; - break; - case INS_COND_LO: - isUnsigned = true; - opCond = GT_LT; - break; - case INS_COND_LS: - isUnsigned = true; - opCond = GT_LE; - break; - default: - assert(!"Unexpected cond"); - } - inst_SETCC(GenCondition::FromIntegralRelop(opCond, isUnsigned), tree->TypeGet(), targetReg); + inst_SETCC(cond, tree->TypeGet(), targetReg); genProduceReg(tree); return; } @@ -4485,7 +4444,7 @@ void CodeGen::genCodeForCompare(GenTreeOp* tree) // tree - a compare node (GT_EQ etc) // cond - the condition of the previous generated compare. // -void CodeGen::genCodeForConditionalCompare(GenTreeOp* tree, insCond cond) +void CodeGen::genCodeForConditionalCompare(GenTreeOp* tree, GenCondition prevCond) { emitter* emit = GetEmitter(); @@ -4505,17 +4464,21 @@ void CodeGen::genCodeForConditionalCompare(GenTreeOp* tree, insCond cond) assert(targetReg == REG_NA); // For the ccmp flags, invert the condition of the compare. - insCflags cflags = InsCflagsForCcmp(InsCondForCompareOp(tree)); + insCflags cflags = InsCflagsForCcmp(GenCondition::FromRelop(tree)); + + // For the condition, use the previous compare. + const GenConditionDesc& prevDesc = GenConditionDesc::Get(prevCond); + insCond prevInsCond = JumpKindToInsCond(prevDesc.jumpKind1); if (op2->isContainedIntOrIImmed()) { GenTreeIntConCommon* intConst = op2->AsIntConCommon(); - emit->emitIns_R_I_FLAGS_COND(INS_ccmp, cmpSize, srcReg1, (int)intConst->IconValue(), cflags, cond); + emit->emitIns_R_I_FLAGS_COND(INS_ccmp, cmpSize, srcReg1, (int)intConst->IconValue(), cflags, prevInsCond); } else { regNumber srcReg2 = op2->GetRegNum(); - emit->emitIns_R_R_FLAGS_COND(INS_ccmp, cmpSize, srcReg1, srcReg2, cflags, cond); + emit->emitIns_R_R_FLAGS_COND(INS_ccmp, cmpSize, srcReg1, srcReg2, cflags, prevInsCond); } } @@ -4528,11 +4491,11 @@ void CodeGen::genCodeForConditionalCompare(GenTreeOp* tree, insCond cond) // Arguments: // tree - the node. Either a compare or a tree of compares connected by ANDs. // inChain - whether a contained chain is in progress. -// prev - If a chain is in progress, the condition of the previous compare. +// prevCond - If a chain is in progress, the condition of the previous compare. // Return: // The last compare node generated. // -void CodeGen::genCodeForContainedCompareChain(GenTree* tree, bool* inChain, insCond* prevcond) +void CodeGen::genCodeForContainedCompareChain(GenTree* tree, bool* inChain, GenCondition* prevCond) { assert(tree->isContained()); if (!*inChain) @@ -4550,19 +4513,19 @@ void CodeGen::genCodeForContainedCompareChain(GenTree* tree, bool* inChain, insC // If Op1 is contained, generate into flags. Otherwise, move the result into flags. if (op1->isContained()) { - genCodeForContainedCompareChain(op1, inChain, prevcond); + genCodeForContainedCompareChain(op1, inChain, prevCond); assert(*inChain); } else { emitter* emit = GetEmitter(); emit->emitIns_R_I(INS_cmp, emitActualTypeSize(op1), op1->GetRegNum(), 0); - *prevcond = INS_COND_NE; + *prevCond = GenCondition::NE; *inChain = true; } // Generate Op2 based on Op1. - genCodeForContainedCompareChain(op2, inChain, prevcond); + genCodeForContainedCompareChain(op2, inChain, prevCond); assert(*inChain); } else @@ -4579,11 +4542,11 @@ void CodeGen::genCodeForContainedCompareChain(GenTree* tree, bool* inChain, insC { // Within the chain. Use a conditional compare (which is // dependent on the previous emitted compare). - genCodeForConditionalCompare(tree->AsOp(), *prevcond); + genCodeForConditionalCompare(tree->AsOp(), *prevCond); } *inChain = true; - *prevcond = InsCondForCompareOp(tree); + *prevCond = GenCondition::FromRelop(tree); } } @@ -4607,12 +4570,12 @@ void CodeGen::genCodeForSelect(GenTreeConditional* tree) assert(!op1->isUsedFromMemory()); assert(genTypeSize(op1Type) == genTypeSize(op2Type)); - insCond cond = INS_COND_EQ; // Dummy value. + GenCondition prevCond; if (opcond->isContained()) { // Generate the contained condition. bool chain = false; - genCodeForContainedCompareChain(opcond, &chain, &cond); + genCodeForContainedCompareChain(opcond, &chain, &prevCond); assert(chain); } else @@ -4620,14 +4583,15 @@ void CodeGen::genCodeForSelect(GenTreeConditional* tree) // Condition has been generated into a register - move it into flags. genConsumeReg(opcond); emit->emitIns_R_I(INS_cmp, emitActualTypeSize(opcond), opcond->GetRegNum(), 0); - cond = INS_COND_NE; + prevCond = GenCondition::NE; } - regNumber targetReg = tree->GetRegNum(); - regNumber srcReg1 = genConsumeReg(op1); - regNumber srcReg2 = genConsumeReg(op2); + regNumber targetReg = tree->GetRegNum(); + regNumber srcReg1 = genConsumeReg(op1); + regNumber srcReg2 = genConsumeReg(op2); + const GenConditionDesc& prevDesc = GenConditionDesc::Get(prevCond); - emit->emitIns_R_R_R_COND(INS_csel, cmpSize, targetReg, srcReg1, srcReg2, cond); + emit->emitIns_R_R_R_COND(INS_csel, cmpSize, targetReg, srcReg1, srcReg2, JumpKindToInsCond(prevDesc.jumpKind1)); regSet.verifyRegUsed(targetReg); } @@ -10601,74 +10565,6 @@ void CodeGen::genCodeForCond(GenTreeOp* tree) genProduceReg(tree); } -//------------------------------------------------------------------------ -// InsCondForCompareOp: Map the condition in a Compare/Conditional op to a insCond. -// -// Arguments: -// tree - the node -// -insCond CodeGen::InsCondForCompareOp(GenTree* tree) -{ - assert(tree->OperIsCompare()); - - switch (tree->AsOp()->OperGet()) - { - case GT_EQ: - case GT_TEST_EQ: - return INS_COND_EQ; - case GT_NE: - case GT_TEST_NE: - return INS_COND_NE; - case GT_GE: - return tree->IsUnsigned() ? INS_COND_HS : INS_COND_GE; - case GT_GT: - return tree->IsUnsigned() ? INS_COND_HI : INS_COND_GT; - case GT_LT: - return tree->IsUnsigned() ? INS_COND_LO : INS_COND_LT; - case GT_LE: - return tree->IsUnsigned() ? INS_COND_LS : INS_COND_LE; - default: - assert(false && "Invalid condition"); - return INS_COND_EQ; - } -} - -//------------------------------------------------------------------------ -// InvertInsCond: Invert an insCond -// -// Arguments: -// cond - the insCond. -// -insCond CodeGen::InvertInsCond(insCond cond) -{ - switch (cond) - { - case INS_COND_EQ: - return INS_COND_NE; - case INS_COND_NE: - return INS_COND_EQ; - case INS_COND_GE: - return INS_COND_LT; - case INS_COND_GT: - return INS_COND_LE; - case INS_COND_LT: - return INS_COND_GE; - case INS_COND_LE: - return INS_COND_GT; - case INS_COND_HS: - return INS_COND_LO; - case INS_COND_HI: - return INS_COND_LS; - case INS_COND_LO: - return INS_COND_HS; - case INS_COND_LS: - return INS_COND_HI; - default: - assert(false && "Invalid condition"); - return INS_COND_EQ; - } -} - //------------------------------------------------------------------------ // InsCflagsForCcmp: Get the Cflags for a required for a CCMP instruction. // @@ -10680,36 +10576,82 @@ insCond CodeGen::InvertInsCond(insCond cond) // Given COND, this function returns A. // // Arguments: -// cond - the insCond. +// cond - the GenCondition. // -insCflags CodeGen::InsCflagsForCcmp(insCond cond) +insCflags CodeGen::InsCflagsForCcmp(GenCondition cond) { - switch (InvertInsCond(cond)) + GenCondition inverted = GenCondition::Reverse(cond); + switch (inverted.GetCode()) { - case INS_COND_EQ: + case GenCondition::EQ: return INS_FLAGS_Z; - case INS_COND_NE: + case GenCondition::NE: return INS_FLAGS_NONE; - case INS_COND_GE: + case GenCondition::SGE: return INS_FLAGS_Z; - case INS_COND_GT: + case GenCondition::SGT: return INS_FLAGS_NONE; - case INS_COND_LT: + case GenCondition::SLT: return INS_FLAGS_NC; - case INS_COND_LE: + case GenCondition::SLE: return INS_FLAGS_NZC; - case INS_COND_HS: + case GenCondition::UGE: return INS_FLAGS_C; - case INS_COND_HI: + case GenCondition::UGT: return INS_FLAGS_C; - case INS_COND_LO: + case GenCondition::ULT: return INS_FLAGS_NONE; - case INS_COND_LS: + case GenCondition::ULE: return INS_FLAGS_Z; default: - assert(false && "Invalid condition"); + NO_WAY("unexpected condition type"); return INS_FLAGS_NONE; } } +//------------------------------------------------------------------------ +// JumpKindToInsCond: Convert a Jump Kind to a condition. +// +// Arguments: +// condition - the emitJumpKind. +// +insCond CodeGen::JumpKindToInsCond(emitJumpKind condition) +{ + /* Convert the condition to an insCond value */ + switch (condition) + { + case EJ_eq: + return INS_COND_EQ; + case EJ_ne: + return INS_COND_NE; + case EJ_hs: + return INS_COND_HS; + case EJ_lo: + return INS_COND_LO; + case EJ_mi: + return INS_COND_MI; + case EJ_pl: + return INS_COND_PL; + case EJ_vs: + return INS_COND_VS; + case EJ_vc: + return INS_COND_VC; + case EJ_hi: + return INS_COND_HI; + case EJ_ls: + return INS_COND_LS; + case EJ_ge: + return INS_COND_GE; + case EJ_lt: + return INS_COND_LT; + case EJ_gt: + return INS_COND_GT; + case EJ_le: + return INS_COND_LE; + default: + NO_WAY("unexpected condition type"); + return INS_COND_EQ; + } +} + #endif // TARGET_ARM64 diff --git a/src/coreclr/jit/instr.cpp b/src/coreclr/jit/instr.cpp index 9843482bbef6c6..c9fbb2b27a821d 100644 --- a/src/coreclr/jit/instr.cpp +++ b/src/coreclr/jit/instr.cpp @@ -326,61 +326,8 @@ void CodeGen::inst_SET(emitJumpKind condition, regNumber reg) // These instructions only write the low byte of 'reg' GetEmitter()->emitIns_R(ins, EA_1BYTE, reg); #elif defined(TARGET_ARM64) - insCond cond; - /* Convert the condition to an insCond value */ - switch (condition) - { - case EJ_eq: - cond = INS_COND_EQ; - break; - case EJ_ne: - cond = INS_COND_NE; - break; - case EJ_hs: - cond = INS_COND_HS; - break; - case EJ_lo: - cond = INS_COND_LO; - break; - case EJ_mi: - cond = INS_COND_MI; - break; - case EJ_pl: - cond = INS_COND_PL; - break; - case EJ_vs: - cond = INS_COND_VS; - break; - case EJ_vc: - cond = INS_COND_VC; - break; - - case EJ_hi: - cond = INS_COND_HI; - break; - case EJ_ls: - cond = INS_COND_LS; - break; - case EJ_ge: - cond = INS_COND_GE; - break; - case EJ_lt: - cond = INS_COND_LT; - break; - - case EJ_gt: - cond = INS_COND_GT; - break; - case EJ_le: - cond = INS_COND_LE; - break; - - default: - NO_WAY("unexpected condition type"); - return; - } - GetEmitter()->emitIns_R_COND(INS_cset, EA_8BYTE, reg, cond); + GetEmitter()->emitIns_R_COND(INS_cset, EA_8BYTE, reg, JumpKindToInsCond(condition)); #else NYI("inst_SET"); #endif From 8b282f9b819eafccaf9e2497fd19c3db4fe7470c Mon Sep 17 00:00:00 2001 From: Alan Hayward Date: Thu, 4 Aug 2022 11:59:26 +0100 Subject: [PATCH 17/26] Change CompareChainSize to IsValidCompareChain --- src/coreclr/jit/lower.cpp | 50 ++++++++++----------------------------- src/coreclr/jit/lower.h | 2 +- 2 files changed, 14 insertions(+), 38 deletions(-) diff --git a/src/coreclr/jit/lower.cpp b/src/coreclr/jit/lower.cpp index 1c5c30734f25dc..3a0937c8a0afd5 100644 --- a/src/coreclr/jit/lower.cpp +++ b/src/coreclr/jit/lower.cpp @@ -7023,55 +7023,43 @@ void Lowering::ContainCheckJTrue(GenTreeOp* node) } //------------------------------------------------------------------------ -// CompareChainSize : Determine the size of an uncontained chain. +// IsValidCompareChain : Determine if the node contains a valid compare chain. // // Arguments: // child - pointer to the node being checked. // parent - parent node of the child. -// inChain - set to true if an exisiting contained chain is found. // // Return value: -// Number of uncontained nodes in the chain. +// True if a valid chain is found. // -int Lowering::CompareChainSize(GenTree* child, GenTree* parent, bool* inChain) +bool Lowering::IsValidCompareChain(GenTree* child, GenTree* parent) { assert(parent->OperIs(GT_AND) || parent->OperIs(GT_SELECT)); - int count = 0; if (child->isContainedAndNotIntOrIImmed()) { // Already have a chain. - *inChain = true; + assert(child->OperIs(GT_AND) || child->OperIsCompare()); + return true; } else { if (child->OperIs(GT_AND)) { // Count both sides. - int count2 = CompareChainSize(child->AsOp()->gtGetOp2(), child, inChain); - if (count2 > 0) - { - int count1 = CompareChainSize(child->AsOp()->gtGetOp1(), child, inChain); - - if (count1 > 0) - { - count = count1 + count2 + 1; - } - } + return IsValidCompareChain(child->AsOp()->gtGetOp2(), child) && + IsValidCompareChain(child->AsOp()->gtGetOp1(), child); } #ifdef TARGET_ARM64 else if (child->OperIsCompare()) { // Can the child compare be contained. - if (IsSafeToContainMem(parent, child)) - { - count++; - } + return IsSafeToContainMem(parent, child); } #endif } - return count; + return false; } //------------------------------------------------------------------------ @@ -7153,21 +7141,9 @@ void Lowering::ContainCheckAndChain(GenTree* tree) return; } - // Determine whether a chain exists. - bool inChain = false; - int chainSize = 0; - int chainSize2 = CompareChainSize(tree->AsOp()->gtGetOp2(), tree, &inChain); - if (chainSize2 > 0 && !inChain) - { - int chainSize1 = CompareChainSize(tree->AsOp()->gtGetOp1(), tree, &inChain); - if (chainSize1 > 0 && !inChain) - { - chainSize = chainSize1 + chainSize2; - } - } - - // Continue an exisiting chain, or start a new one if it's long enough to be worthwhile. - if (chainSize > 1 || inChain) + // First check there is a valid chain. + if (IsValidCompareChain(tree->AsOp()->gtGetOp2(), tree) && + IsValidCompareChain(tree->AsOp()->gtGetOp1(), tree)) { GenTree* startOfChain = nullptr; @@ -7189,7 +7165,7 @@ void Lowering::ContainCheckAndChain(GenTree* tree) } } - JITDUMP("Lowered And chain: length=%d %s\n", chainSize, inChain ? "" : "(new chain)"); + JITDUMP("Lowered And chain:\n"); DISPTREE(tree); } } diff --git a/src/coreclr/jit/lower.h b/src/coreclr/jit/lower.h index 5e6c75b06b2932..25efdacfa10c80 100644 --- a/src/coreclr/jit/lower.h +++ b/src/coreclr/jit/lower.h @@ -88,8 +88,8 @@ class Lowering final : public Phase #ifdef TARGET_ARM64 void ContainCheckConditionalCompare(GenTreeOp* cmp); #endif + bool IsValidCompareChain(GenTree* child, GenTree* parent); bool ContainCheckCompareChain(GenTree* child, GenTree* parent, GenTree** earliestValid); - int CompareChainSize(GenTree* child, GenTree* parent, bool* inChain); void ContainCheckAndChain(GenTree* tree); void ContainCheckSelect(GenTreeConditional* node); void ContainCheckBitCast(GenTree* node); From ce5882d9020c4853155341cf47bcb802bc3200c4 Mon Sep 17 00:00:00 2001 From: Alan Hayward Date: Thu, 4 Aug 2022 13:30:56 +0100 Subject: [PATCH 18/26] Move lowering functions to lowerarmarch --- src/coreclr/jit/lower.cpp | 179 +------------------------------ src/coreclr/jit/lower.h | 4 +- src/coreclr/jit/lowerarmarch.cpp | 178 +++++++++++++++++++++++++++++- 3 files changed, 182 insertions(+), 179 deletions(-) diff --git a/src/coreclr/jit/lower.cpp b/src/coreclr/jit/lower.cpp index 3a0937c8a0afd5..3808abf08d7191 100644 --- a/src/coreclr/jit/lower.cpp +++ b/src/coreclr/jit/lower.cpp @@ -239,7 +239,9 @@ GenTree* Lowering::LowerNode(GenTree* node) return LowerJTrue(node->AsOp()); case GT_SELECT: +#ifdef TARGET_ARM64 ContainCheckSelect(node->AsConditional()); +#endif break; case GT_JMP: @@ -6829,7 +6831,9 @@ void Lowering::ContainCheckNode(GenTree* node) break; case GT_SELECT: +#ifdef TARGET_ARM64 ContainCheckSelect(node->AsConditional()); +#endif break; case GT_ADD: @@ -7022,181 +7026,6 @@ void Lowering::ContainCheckJTrue(GenTreeOp* node) cmp->gtFlags |= GTF_SET_FLAGS; } -//------------------------------------------------------------------------ -// IsValidCompareChain : Determine if the node contains a valid compare chain. -// -// Arguments: -// child - pointer to the node being checked. -// parent - parent node of the child. -// -// Return value: -// True if a valid chain is found. -// -bool Lowering::IsValidCompareChain(GenTree* child, GenTree* parent) -{ - assert(parent->OperIs(GT_AND) || parent->OperIs(GT_SELECT)); - - if (child->isContainedAndNotIntOrIImmed()) - { - // Already have a chain. - assert(child->OperIs(GT_AND) || child->OperIsCompare()); - return true; - } - else - { - if (child->OperIs(GT_AND)) - { - // Count both sides. - return IsValidCompareChain(child->AsOp()->gtGetOp2(), child) && - IsValidCompareChain(child->AsOp()->gtGetOp1(), child); - } -#ifdef TARGET_ARM64 - else if (child->OperIsCompare()) - { - // Can the child compare be contained. - return IsSafeToContainMem(parent, child); - } -#endif - } - - return false; -} - -//------------------------------------------------------------------------ -// ContainCheckCompareChain : Determine if a chain of ANDs and CMPs can be contained. -// -// Arguments: -// child - pointer to the node being checked. -// parent - parent node of the child. -// startOfChain - If found, returns the earliest valid op in the chain. -// -bool Lowering::ContainCheckCompareChain(GenTree* child, GenTree* parent, GenTree** startOfChain) -{ - assert(parent->OperIs(GT_AND) || parent->OperIs(GT_SELECT)); - - if (child->isContainedAndNotIntOrIImmed()) - { - // Already have a chain. - *startOfChain = nullptr; - return true; - } - // Can the child be contained. - else if (IsSafeToContainMem(parent, child)) - { - if (child->OperIs(GT_AND)) - { - // If Op2 is not contained, then try to contain it. - if (!child->AsOp()->gtGetOp2()->isContainedAndNotIntOrIImmed()) - { - if (!ContainCheckCompareChain(child->gtGetOp2(), child, startOfChain)) - { - // Op2 must be contained in order to contain Op1 or the AND. - return false; - } - } - - // If Op1 is not contained, then try to contain it. - if (!child->AsOp()->gtGetOp1()->isContainedAndNotIntOrIImmed()) - { - if (!ContainCheckCompareChain(child->gtGetOp1(), child, startOfChain)) - { - return false; - } - } - - // Contain the AND. - child->SetContained(); - return true; - } -#ifdef TARGET_ARM64 - else if (child->OperIsCompare()) - { - child->AsOp()->SetContained(); - - // Ensure the children of the compare are contained correctly. - child->AsOp()->gtGetOp1()->ClearContained(); - child->AsOp()->gtGetOp2()->ClearContained(); - ContainCheckConditionalCompare(child->AsOp()); - *startOfChain = child; - return true; - } -#endif - } - - return false; -} - -//------------------------------------------------------------------------ -// ContainCheckAndCompareChain : Determine if an AND is a containable chain -// -// Arguments: -// node - pointer to the node -// -void Lowering::ContainCheckAndChain(GenTree* tree) -{ - assert(tree->OperIs(GT_AND)); - - if (!comp->opts.OptimizationEnabled()) - { - return; - } - - // First check there is a valid chain. - if (IsValidCompareChain(tree->AsOp()->gtGetOp2(), tree) && - IsValidCompareChain(tree->AsOp()->gtGetOp1(), tree)) - { - GenTree* startOfChain = nullptr; - - // To ensure ordering at code generation, Op1 and the parent can - // only be contained if Op2 is contained. - if (ContainCheckCompareChain(tree->AsOp()->gtGetOp2(), tree, &startOfChain)) - { - if (ContainCheckCompareChain(tree->AsOp()->gtGetOp1(), tree, &startOfChain)) - { - // If op1 is the start of a chain, then it'll be generated as a standard compare. - if (startOfChain != nullptr) - { - // The earliest node in the chain will be generated as a standard compare. - assert(startOfChain->OperIsCompare()); - startOfChain->AsOp()->gtGetOp1()->ClearContained(); - startOfChain->AsOp()->gtGetOp2()->ClearContained(); - ContainCheckCompare(startOfChain->AsOp()); - } - } - } - - JITDUMP("Lowered And chain:\n"); - DISPTREE(tree); - } -} - -//------------------------------------------------------------------------ -// ContainCheckSelect : determine whether the source of a select should be contained. -// -// Arguments: -// node - pointer to the node -// -void Lowering::ContainCheckSelect(GenTreeConditional* node) -{ - if (!comp->opts.OptimizationEnabled()) - { - return; - } - - // Check if the compare does not need to be generated into a register. - GenTree* startOfChain = nullptr; - ContainCheckCompareChain(node->gtCond, node, &startOfChain); - - if (startOfChain != nullptr) - { - // The earliest node in the chain will be generated as a standard compare. - assert(startOfChain->OperIsCompare()); - startOfChain->AsOp()->gtGetOp1()->ClearContained(); - startOfChain->AsOp()->gtGetOp2()->ClearContained(); - ContainCheckCompare(startOfChain->AsOp()); - } -} - //------------------------------------------------------------------------ // ContainCheckBitCast: determine whether the source of a BITCAST should be contained. // diff --git a/src/coreclr/jit/lower.h b/src/coreclr/jit/lower.h index 25efdacfa10c80..491bf233d51ba3 100644 --- a/src/coreclr/jit/lower.h +++ b/src/coreclr/jit/lower.h @@ -86,12 +86,12 @@ class Lowering final : public Phase void ContainCheckRet(GenTreeUnOp* ret); void ContainCheckJTrue(GenTreeOp* node); #ifdef TARGET_ARM64 - void ContainCheckConditionalCompare(GenTreeOp* cmp); -#endif bool IsValidCompareChain(GenTree* child, GenTree* parent); bool ContainCheckCompareChain(GenTree* child, GenTree* parent, GenTree** earliestValid); void ContainCheckAndChain(GenTree* tree); + void ContainCheckConditionalCompare(GenTreeOp* cmp); void ContainCheckSelect(GenTreeConditional* node); +#endif void ContainCheckBitCast(GenTree* node); void ContainCheckCallOperands(GenTreeCall* call); void ContainCheckIndir(GenTreeIndir* indirNode); diff --git a/src/coreclr/jit/lowerarmarch.cpp b/src/coreclr/jit/lowerarmarch.cpp index 5132dbf14d77a4..3e8d5fe7f4024c 100644 --- a/src/coreclr/jit/lowerarmarch.cpp +++ b/src/coreclr/jit/lowerarmarch.cpp @@ -365,10 +365,12 @@ GenTree* Lowering::LowerBinaryArithmetic(GenTreeOp* binOp) binOp->ChangeOper(GT_AND_NOT); BlockRange().Remove(notNode); } +#ifdef TARGET_ARM64 else { ContainCheckAndChain(binOp); } +#endif } ContainCheckBinary(binOp); @@ -2046,13 +2048,157 @@ void Lowering::ContainCheckCompare(GenTreeOp* cmp) CheckImmedAndMakeContained(cmp, cmp->gtOp2); } +#ifdef TARGET_ARM64 +//------------------------------------------------------------------------ +// IsValidCompareChain : Determine if the node contains a valid compare chain. +// +// Arguments: +// child - pointer to the node being checked. +// parent - parent node of the child. +// +// Return value: +// True if a valid chain is found. +// +bool Lowering::IsValidCompareChain(GenTree* child, GenTree* parent) +{ + assert(parent->OperIs(GT_AND) || parent->OperIs(GT_SELECT)); + + if (child->isContainedAndNotIntOrIImmed()) + { + // Already have a chain. + assert(child->OperIs(GT_AND) || child->OperIsCompare()); + return true; + } + else + { + if (child->OperIs(GT_AND)) + { + // Count both sides. + return IsValidCompareChain(child->AsOp()->gtGetOp2(), child) && + IsValidCompareChain(child->AsOp()->gtGetOp1(), child); + } + else if (child->OperIsCompare()) + { + // Can the child compare be contained. + return IsSafeToContainMem(parent, child); + } + } + + return false; +} + +//------------------------------------------------------------------------ +// ContainCheckCompareChain : Determine if a chain of ANDs and CMPs can be contained. +// +// Arguments: +// child - pointer to the node being checked. +// parent - parent node of the child. +// startOfChain - If found, returns the earliest valid op in the chain. +// +bool Lowering::ContainCheckCompareChain(GenTree* child, GenTree* parent, GenTree** startOfChain) +{ + assert(parent->OperIs(GT_AND) || parent->OperIs(GT_SELECT)); + + if (child->isContainedAndNotIntOrIImmed()) + { + // Already have a chain. + *startOfChain = nullptr; + return true; + } + // Can the child be contained. + else if (IsSafeToContainMem(parent, child)) + { + if (child->OperIs(GT_AND)) + { + // If Op2 is not contained, then try to contain it. + if (!child->AsOp()->gtGetOp2()->isContainedAndNotIntOrIImmed()) + { + if (!ContainCheckCompareChain(child->gtGetOp2(), child, startOfChain)) + { + // Op2 must be contained in order to contain Op1 or the AND. + return false; + } + } + + // If Op1 is not contained, then try to contain it. + if (!child->AsOp()->gtGetOp1()->isContainedAndNotIntOrIImmed()) + { + if (!ContainCheckCompareChain(child->gtGetOp1(), child, startOfChain)) + { + return false; + } + } + + // Contain the AND. + child->SetContained(); + return true; + } + else if (child->OperIsCompare()) + { + child->AsOp()->SetContained(); + + // Ensure the children of the compare are contained correctly. + child->AsOp()->gtGetOp1()->ClearContained(); + child->AsOp()->gtGetOp2()->ClearContained(); + ContainCheckConditionalCompare(child->AsOp()); + *startOfChain = child; + return true; + } + } + + return false; +} + +//------------------------------------------------------------------------ +// ContainCheckAndCompareChain : Determine if an AND is a containable chain +// +// Arguments: +// node - pointer to the node +// +void Lowering::ContainCheckAndChain(GenTree* tree) +{ + assert(tree->OperIs(GT_AND)); + + if (!comp->opts.OptimizationEnabled()) + { + return; + } + + // First check there is a valid chain. + if (IsValidCompareChain(tree->AsOp()->gtGetOp2(), tree) && + IsValidCompareChain(tree->AsOp()->gtGetOp1(), tree)) + { + GenTree* startOfChain = nullptr; + + // To ensure ordering at code generation, Op1 and the parent can + // only be contained if Op2 is contained. + if (ContainCheckCompareChain(tree->AsOp()->gtGetOp2(), tree, &startOfChain)) + { + if (ContainCheckCompareChain(tree->AsOp()->gtGetOp1(), tree, &startOfChain)) + { + // If op1 is the start of a chain, then it'll be generated as a standard compare. + if (startOfChain != nullptr) + { + // The earliest node in the chain will be generated as a standard compare. + assert(startOfChain->OperIsCompare()); + startOfChain->AsOp()->gtGetOp1()->ClearContained(); + startOfChain->AsOp()->gtGetOp2()->ClearContained(); + ContainCheckCompare(startOfChain->AsOp()); + } + } + } + + JITDUMP("Lowered And chain:\n"); + DISPTREE(tree); + } +} + //------------------------------------------------------------------------ // ContainCheckConditionalCompare: determine whether the source of a compare within a compare chain should be contained. // // Arguments: // node - pointer to the node // -#ifdef TARGET_ARM64 void Lowering::ContainCheckConditionalCompare(GenTreeOp* cmp) { assert(cmp->OperIsCompare()); @@ -2068,7 +2214,35 @@ void Lowering::ContainCheckConditionalCompare(GenTreeOp* cmp) } } } -#endif + +//------------------------------------------------------------------------ +// ContainCheckSelect : determine whether the source of a select should be contained. +// +// Arguments: +// node - pointer to the node +// +void Lowering::ContainCheckSelect(GenTreeConditional* node) +{ + if (!comp->opts.OptimizationEnabled()) + { + return; + } + + // Check if the compare does not need to be generated into a register. + GenTree* startOfChain = nullptr; + ContainCheckCompareChain(node->gtCond, node, &startOfChain); + + if (startOfChain != nullptr) + { + // The earliest node in the chain will be generated as a standard compare. + assert(startOfChain->OperIsCompare()); + startOfChain->AsOp()->gtGetOp1()->ClearContained(); + startOfChain->AsOp()->gtGetOp2()->ClearContained(); + ContainCheckCompare(startOfChain->AsOp()); + } +} + +#endif //TARGET_ARM64 //------------------------------------------------------------------------ // ContainCheckBoundsChk: determine whether any source of a bounds check node should be contained. From c548f9f59a1047ba028a10743a9ad13eebf30350 Mon Sep 17 00:00:00 2001 From: Alan Hayward Date: Thu, 4 Aug 2022 14:02:03 +0100 Subject: [PATCH 19/26] Formatting fixes --- src/coreclr/jit/codegenarm64.cpp | 6 +++--- src/coreclr/jit/lowerarmarch.cpp | 5 ++--- 2 files changed, 5 insertions(+), 6 deletions(-) diff --git a/src/coreclr/jit/codegenarm64.cpp b/src/coreclr/jit/codegenarm64.cpp index 1da5e76a7b9838..e78038633f0771 100644 --- a/src/coreclr/jit/codegenarm64.cpp +++ b/src/coreclr/jit/codegenarm64.cpp @@ -4467,8 +4467,8 @@ void CodeGen::genCodeForConditionalCompare(GenTreeOp* tree, GenCondition prevCon insCflags cflags = InsCflagsForCcmp(GenCondition::FromRelop(tree)); // For the condition, use the previous compare. - const GenConditionDesc& prevDesc = GenConditionDesc::Get(prevCond); - insCond prevInsCond = JumpKindToInsCond(prevDesc.jumpKind1); + const GenConditionDesc& prevDesc = GenConditionDesc::Get(prevCond); + insCond prevInsCond = JumpKindToInsCond(prevDesc.jumpKind1); if (op2->isContainedIntOrIImmed()) { @@ -4520,7 +4520,7 @@ void CodeGen::genCodeForContainedCompareChain(GenTree* tree, bool* inChain, GenC { emitter* emit = GetEmitter(); emit->emitIns_R_I(INS_cmp, emitActualTypeSize(op1), op1->GetRegNum(), 0); - *prevCond = GenCondition::NE; + *prevCond = GenCondition::NE; *inChain = true; } diff --git a/src/coreclr/jit/lowerarmarch.cpp b/src/coreclr/jit/lowerarmarch.cpp index 3e8d5fe7f4024c..cf615f87c5f24f 100644 --- a/src/coreclr/jit/lowerarmarch.cpp +++ b/src/coreclr/jit/lowerarmarch.cpp @@ -2165,8 +2165,7 @@ void Lowering::ContainCheckAndChain(GenTree* tree) } // First check there is a valid chain. - if (IsValidCompareChain(tree->AsOp()->gtGetOp2(), tree) && - IsValidCompareChain(tree->AsOp()->gtGetOp1(), tree)) + if (IsValidCompareChain(tree->AsOp()->gtGetOp2(), tree) && IsValidCompareChain(tree->AsOp()->gtGetOp1(), tree)) { GenTree* startOfChain = nullptr; @@ -2242,7 +2241,7 @@ void Lowering::ContainCheckSelect(GenTreeConditional* node) } } -#endif //TARGET_ARM64 +#endif // TARGET_ARM64 //------------------------------------------------------------------------ // ContainCheckBoundsChk: determine whether any source of a bounds check node should be contained. From df67d67373afaaab42e065673a88ffe2a9a2aa8e Mon Sep 17 00:00:00 2001 From: Alan Hayward Date: Fri, 5 Aug 2022 09:28:53 +0100 Subject: [PATCH 20/26] Fix SELECT issues --- src/coreclr/jit/codegenarm64.cpp | 8 +++----- src/coreclr/jit/lowerarmarch.cpp | 2 +- 2 files changed, 4 insertions(+), 6 deletions(-) diff --git a/src/coreclr/jit/codegenarm64.cpp b/src/coreclr/jit/codegenarm64.cpp index e78038633f0771..ce158fce2cce4d 100644 --- a/src/coreclr/jit/codegenarm64.cpp +++ b/src/coreclr/jit/codegenarm64.cpp @@ -2560,6 +2560,7 @@ void CodeGen::genCodeForBinary(GenTreeOp* tree) GenCondition cond; bool chain = false; + JITDUMP("Generating compare chain:\n"); if (op1->isContained()) { // Generate Op1 into flags. @@ -4498,10 +4499,6 @@ void CodeGen::genCodeForConditionalCompare(GenTreeOp* tree, GenCondition prevCon void CodeGen::genCodeForContainedCompareChain(GenTree* tree, bool* inChain, GenCondition* prevCond) { assert(tree->isContained()); - if (!*inChain) - { - JITDUMP("Generating compare chain:\n"); - } if (tree->OperIs(GT_AND)) { @@ -4571,17 +4568,18 @@ void CodeGen::genCodeForSelect(GenTreeConditional* tree) assert(genTypeSize(op1Type) == genTypeSize(op2Type)); GenCondition prevCond; + genConsumeRegs(opcond); if (opcond->isContained()) { // Generate the contained condition. bool chain = false; + JITDUMP("Generating compare chain:\n"); genCodeForContainedCompareChain(opcond, &chain, &prevCond); assert(chain); } else { // Condition has been generated into a register - move it into flags. - genConsumeReg(opcond); emit->emitIns_R_I(INS_cmp, emitActualTypeSize(opcond), opcond->GetRegNum(), 0); prevCond = GenCondition::NE; } diff --git a/src/coreclr/jit/lowerarmarch.cpp b/src/coreclr/jit/lowerarmarch.cpp index cf615f87c5f24f..e17dc50777cdb2 100644 --- a/src/coreclr/jit/lowerarmarch.cpp +++ b/src/coreclr/jit/lowerarmarch.cpp @@ -2098,11 +2098,11 @@ bool Lowering::IsValidCompareChain(GenTree* child, GenTree* parent) bool Lowering::ContainCheckCompareChain(GenTree* child, GenTree* parent, GenTree** startOfChain) { assert(parent->OperIs(GT_AND) || parent->OperIs(GT_SELECT)); + *startOfChain = nullptr; // Nothing found yet. if (child->isContainedAndNotIntOrIImmed()) { // Already have a chain. - *startOfChain = nullptr; return true; } // Can the child be contained. From d1854f89183af89b374afa8236759268d94cd76c Mon Sep 17 00:00:00 2001 From: Alan Hayward Date: Fri, 5 Aug 2022 16:41:53 +0100 Subject: [PATCH 21/26] Fix test output messages. --- .../JIT/opt/Compares/compareAnd2Chains.cs | 36 +++++++++---------- .../JIT/opt/Compares/compareAnd3Chains.cs | 36 +++++++++---------- 2 files changed, 36 insertions(+), 36 deletions(-) diff --git a/src/tests/JIT/opt/Compares/compareAnd2Chains.cs b/src/tests/JIT/opt/Compares/compareAnd2Chains.cs index 92798c9bbc4067..0fd39d96cbcfa6 100644 --- a/src/tests/JIT/opt/Compares/compareAnd2Chains.cs +++ b/src/tests/JIT/opt/Compares/compareAnd2Chains.cs @@ -167,17 +167,17 @@ public static int Main() } if (!Eq_ushort_2(10, 11)) { - Console.WriteLine("ComparisonTestAnd2Chains:Eq_short_2(10, 11) failed"); + Console.WriteLine("ComparisonTestAnd2Chains:Eq_ushort_2(10, 11) failed"); return 101; } if (!Eq_uint_2(10, 11)) { - Console.WriteLine("ComparisonTestAnd2Chains:Eq_int_2(10, 11) failed"); + Console.WriteLine("ComparisonTestAnd2Chains:Eq_uint_2(10, 11) failed"); return 101; } if (!Eq_ulong_2(10, 11)) { - Console.WriteLine("ComparisonTestAnd2Chains:Eq_long_2(10, 11) failed"); + Console.WriteLine("ComparisonTestAnd2Chains:Eq_ulong_2(10, 11) failed"); return 101; } @@ -203,17 +203,17 @@ public static int Main() } if (!Ne_ushort_2(10, 11)) { - Console.WriteLine("ComparisonTestAnd2Chains:Ne_short_2(10, 11) failed"); + Console.WriteLine("ComparisonTestAnd2Chains:Ne_ushort_2(10, 11) failed"); return 101; } if (!Ne_uint_2(10, 11)) { - Console.WriteLine("ComparisonTestAnd2Chains:Ne_int_2(10, 11) failed"); + Console.WriteLine("ComparisonTestAnd2Chains:Ne_uint_2(10, 11) failed"); return 101; } if (!Ne_ulong_2(10, 11)) { - Console.WriteLine("ComparisonTestAnd2Chains:Ne_long_2(10, 11) failed"); + Console.WriteLine("ComparisonTestAnd2Chains:Ne_ulong_2(10, 11) failed"); return 101; } @@ -239,17 +239,17 @@ public static int Main() } if (!Lt_ushort_2(3, 4)) { - Console.WriteLine("ComparisonTestAnd2Chains:Lt_short_2(3, 4) failed"); + Console.WriteLine("ComparisonTestAnd2Chains:Lt_ushort_2(3, 4) failed"); return 101; } if (!Lt_uint_2(3, 4)) { - Console.WriteLine("ComparisonTestAnd2Chains:Lt_int_2(3, 4) failed"); + Console.WriteLine("ComparisonTestAnd2Chains:Lt_uint_2(3, 4) failed"); return 101; } if (!Lt_ulong_2(3, 4)) { - Console.WriteLine("ComparisonTestAnd2Chains:Lt_long_2(3, 4) failed"); + Console.WriteLine("ComparisonTestAnd2Chains:Lt_ulong_2(3, 4) failed"); return 101; } @@ -275,17 +275,17 @@ public static int Main() } if (!Le_ushort_2(3, 4)) { - Console.WriteLine("ComparisonTestAnd2Chains:Le_short_2(3, 4) failed"); + Console.WriteLine("ComparisonTestAnd2Chains:Le_ushort_2(3, 4) failed"); return 101; } if (!Le_uint_2(3, 4)) { - Console.WriteLine("ComparisonTestAnd2Chains:Le_int_2(3, 4) failed"); + Console.WriteLine("ComparisonTestAnd2Chains:Le_uint_2(3, 4) failed"); return 101; } if (!Le_ulong_2(3, 4)) { - Console.WriteLine("ComparisonTestAnd2Chains:Le_long_2(3, 4) failed"); + Console.WriteLine("ComparisonTestAnd2Chains:Le_ulong_2(3, 4) failed"); return 101; } @@ -311,17 +311,17 @@ public static int Main() } if (!Gt_ushort_2(10, 11)) { - Console.WriteLine("ComparisonTestAnd2Chains:Gt_short_2(10, 11) failed"); + Console.WriteLine("ComparisonTestAnd2Chains:Gt_ushort_2(10, 11) failed"); return 101; } if (!Gt_uint_2(10, 11)) { - Console.WriteLine("ComparisonTestAnd2Chains:Gt_int_2(10, 11) failed"); + Console.WriteLine("ComparisonTestAnd2Chains:Gt_uint_2(10, 11) failed"); return 101; } if (!Gt_ulong_2(10, 11)) { - Console.WriteLine("ComparisonTestAnd2Chains:Gt_long_2(10, 11) failed"); + Console.WriteLine("ComparisonTestAnd2Chains:Gt_ulong_2(10, 11) failed"); return 101; } @@ -347,17 +347,17 @@ public static int Main() } if (!Ge_ushort_2(10, 11)) { - Console.WriteLine("ComparisonTestAnd2Chains:Ge_short_2(10, 11) failed"); + Console.WriteLine("ComparisonTestAnd2Chains:Ge_ushort_2(10, 11) failed"); return 101; } if (!Ge_uint_2(10, 11)) { - Console.WriteLine("ComparisonTestAnd2Chains:Ge_int_2(10, 11) failed"); + Console.WriteLine("ComparisonTestAnd2Chains:Ge_uint_2(10, 11) failed"); return 101; } if (!Ge_ulong_2(10, 11)) { - Console.WriteLine("ComparisonTestAnd2Chains:Le_long_2(10, 11) failed"); + Console.WriteLine("ComparisonTestAnd2Chains:Le_ulong_2(10, 11) failed"); return 101; } diff --git a/src/tests/JIT/opt/Compares/compareAnd3Chains.cs b/src/tests/JIT/opt/Compares/compareAnd3Chains.cs index 3de83e911deafc..c609cdebc2d011 100644 --- a/src/tests/JIT/opt/Compares/compareAnd3Chains.cs +++ b/src/tests/JIT/opt/Compares/compareAnd3Chains.cs @@ -167,17 +167,17 @@ public static int Main() } if (!Eq_ushort_3(10, 11, 12)) { - Console.WriteLine("ComparisonTestAnd2Chains:Eq_short_3(10, 11, 12) failed"); + Console.WriteLine("ComparisonTestAnd2Chains:Eq_ushort_3(10, 11, 12) failed"); return 101; } if (!Eq_uint_3(10, 11, 12)) { - Console.WriteLine("ComparisonTestAnd2Chains:Eq_int_3(10, 11, 12) failed"); + Console.WriteLine("ComparisonTestAnd2Chains:Eq_uint_3(10, 11, 12) failed"); return 101; } if (!Eq_ulong_3(10, 11, 12)) { - Console.WriteLine("ComparisonTestAnd2Chains:Eq_long_3(10, 11, 12) failed"); + Console.WriteLine("ComparisonTestAnd2Chains:Eq_ulong_3(10, 11, 12) failed"); return 101; } @@ -203,17 +203,17 @@ public static int Main() } if (!Ne_ushort_3(10, 11, 12)) { - Console.WriteLine("ComparisonTestAnd2Chains:Ne_short_3(10, 11, 12) failed"); + Console.WriteLine("ComparisonTestAnd2Chains:Ne_ushort_3(10, 11, 12) failed"); return 101; } if (!Ne_uint_3(10, 11, 12)) { - Console.WriteLine("ComparisonTestAnd2Chains:Ne_int_3(10, 11, 12) failed"); + Console.WriteLine("ComparisonTestAnd2Chains:Ne_uint_3(10, 11, 12) failed"); return 101; } if (!Ne_ulong_3(10, 11, 12)) { - Console.WriteLine("ComparisonTestAnd2Chains:Ne_long_3(10, 11, 12) failed"); + Console.WriteLine("ComparisonTestAnd2Chains:Ne_ulong_3(10, 11, 12) failed"); return 101; } @@ -239,17 +239,17 @@ public static int Main() } if (!Lt_ushort_3(2, 3, 4)) { - Console.WriteLine("ComparisonTestAnd2Chains:Lt_short_3(2, 3, 4) failed"); + Console.WriteLine("ComparisonTestAnd2Chains:Lt_ushort_3(2, 3, 4) failed"); return 101; } if (!Lt_uint_3(2, 3, 4)) { - Console.WriteLine("ComparisonTestAnd2Chains:Lt_int_3(2, 3, 4) failed"); + Console.WriteLine("ComparisonTestAnd2Chains:Lt_uint_3(2, 3, 4) failed"); return 101; } if (!Lt_ulong_3(2, 3, 4)) { - Console.WriteLine("ComparisonTestAnd2Chains:Lt_long_3(2, 3, 4) failed"); + Console.WriteLine("ComparisonTestAnd2Chains:Lt_ulong_3(2, 3, 4) failed"); return 101; } @@ -275,17 +275,17 @@ public static int Main() } if (!Le_ushort_3(2, 3, 4)) { - Console.WriteLine("ComparisonTestAnd2Chains:Le_short_3(2, 3, 4) failed"); + Console.WriteLine("ComparisonTestAnd2Chains:Le_ushort_3(2, 3, 4) failed"); return 101; } if (!Le_uint_3(2, 3, 4)) { - Console.WriteLine("ComparisonTestAnd2Chains:Le_int_3(2, 3, 4) failed"); + Console.WriteLine("ComparisonTestAnd2Chains:Le_uint_3(2, 3, 4) failed"); return 101; } if (!Le_ulong_3(2, 3, 4)) { - Console.WriteLine("ComparisonTestAnd2Chains:Le_long_3(2, 3, 4) failed"); + Console.WriteLine("ComparisonTestAnd2Chains:Le_ulong_3(2, 3, 4) failed"); return 101; } @@ -311,17 +311,17 @@ public static int Main() } if (!Gt_ushort_3(10, 11, 12)) { - Console.WriteLine("ComparisonTestAnd2Chains:Gt_short_3(10, 11, 12) failed"); + Console.WriteLine("ComparisonTestAnd2Chains:Gt_ushort_3(10, 11, 12) failed"); return 101; } if (!Gt_uint_3(10, 11, 12)) { - Console.WriteLine("ComparisonTestAnd2Chains:Gt_int_3(10, 11, 12) failed"); + Console.WriteLine("ComparisonTestAnd2Chains:Gt_uint_3(10, 11, 12) failed"); return 101; } if (!Gt_ulong_3(10, 11, 12)) { - Console.WriteLine("ComparisonTestAnd2Chains:Gt_long_3(10, 11, 12) failed"); + Console.WriteLine("ComparisonTestAnd2Chains:Gt_ulong_3(10, 11, 12) failed"); return 101; } @@ -347,17 +347,17 @@ public static int Main() } if (!Ge_ushort_3(10, 11, 12)) { - Console.WriteLine("ComparisonTestAnd2Chains:Ge_short_3(10, 11, 12) failed"); + Console.WriteLine("ComparisonTestAnd2Chains:Ge_ushort_3(10, 11, 12) failed"); return 101; } if (!Ge_uint_3(10, 11, 12)) { - Console.WriteLine("ComparisonTestAnd2Chains:Ge_int_3(10, 11, 12) failed"); + Console.WriteLine("ComparisonTestAnd2Chains:Ge_uint_3(10, 11, 12) failed"); return 101; } if (!Ge_ulong_3(10, 11, 12)) { - Console.WriteLine("ComparisonTestAnd2Chains:Le_long_3(10, 11, 12) failed"); + Console.WriteLine("ComparisonTestAnd2Chains:Le_ulong_3(10, 11, 12) failed"); return 101; } From 83100179043fc4ce9f57df4aeffc2e35ed3a6465 Mon Sep 17 00:00:00 2001 From: Alan Hayward Date: Fri, 5 Aug 2022 17:03:50 +0100 Subject: [PATCH 22/26] Better explanations for AND chains --- src/coreclr/jit/lower.h | 2 +- src/coreclr/jit/lowerarmarch.cpp | 25 ++++++++++++++++++++----- src/coreclr/jit/lsrabuild.cpp | 2 +- 3 files changed, 22 insertions(+), 7 deletions(-) diff --git a/src/coreclr/jit/lower.h b/src/coreclr/jit/lower.h index 491bf233d51ba3..b0d880c030227a 100644 --- a/src/coreclr/jit/lower.h +++ b/src/coreclr/jit/lower.h @@ -88,7 +88,7 @@ class Lowering final : public Phase #ifdef TARGET_ARM64 bool IsValidCompareChain(GenTree* child, GenTree* parent); bool ContainCheckCompareChain(GenTree* child, GenTree* parent, GenTree** earliestValid); - void ContainCheckAndChain(GenTree* tree); + void ContainCheckCompareChainForAnd(GenTree* tree); void ContainCheckConditionalCompare(GenTreeOp* cmp); void ContainCheckSelect(GenTreeConditional* node); #endif diff --git a/src/coreclr/jit/lowerarmarch.cpp b/src/coreclr/jit/lowerarmarch.cpp index e17dc50777cdb2..099be20de2848c 100644 --- a/src/coreclr/jit/lowerarmarch.cpp +++ b/src/coreclr/jit/lowerarmarch.cpp @@ -368,7 +368,7 @@ GenTree* Lowering::LowerBinaryArithmetic(GenTreeOp* binOp) #ifdef TARGET_ARM64 else { - ContainCheckAndChain(binOp); + ContainCheckCompareChainForAnd(binOp); } #endif } @@ -2050,7 +2050,7 @@ void Lowering::ContainCheckCompare(GenTreeOp* cmp) #ifdef TARGET_ARM64 //------------------------------------------------------------------------ -// IsValidCompareChain : Determine if the node contains a valid compare chain. +// IsValidCompareChain : Determine if the node contains a valid chain of ANDs and CMPs. // // Arguments: // child - pointer to the node being checked. @@ -2059,6 +2059,15 @@ void Lowering::ContainCheckCompare(GenTreeOp* cmp) // Return value: // True if a valid chain is found. // +// Notes: +// A compare chain is a sequence of CMP nodes connected by AND nodes. +// For example: AND (AND (CMP A B) (CMP C D)) (CMP E F) +// The chain can just be a single compare node, however it's parent +// must always be an AND or SELECT node. +// If a CMP or AND node is contained then it and all it's children are +// considered to be in a valid chain. +// Chains are built up during the lowering of each successive parent. +// bool Lowering::IsValidCompareChain(GenTree* child, GenTree* parent) { assert(parent->OperIs(GT_AND) || parent->OperIs(GT_SELECT)); @@ -2095,6 +2104,12 @@ bool Lowering::IsValidCompareChain(GenTree* child, GenTree* parent) // parent - parent node of the child. // startOfChain - If found, returns the earliest valid op in the chain. // +// Return value: +// True if a valid chain is was contained. +// +// Notes: +// Assumes the chain was checked via IsValidCompareChain. +// bool Lowering::ContainCheckCompareChain(GenTree* child, GenTree* parent, GenTree** startOfChain) { assert(parent->OperIs(GT_AND) || parent->OperIs(GT_SELECT)); @@ -2150,12 +2165,12 @@ bool Lowering::ContainCheckCompareChain(GenTree* child, GenTree* parent, GenTree } //------------------------------------------------------------------------ -// ContainCheckAndCompareChain : Determine if an AND is a containable chain +// ContainCheckCompareChainForAnd : Determine if an AND is a containable chain // // Arguments: // node - pointer to the node // -void Lowering::ContainCheckAndChain(GenTree* tree) +void Lowering::ContainCheckCompareChainForAnd(GenTree* tree) { assert(tree->OperIs(GT_AND)); @@ -2187,7 +2202,7 @@ void Lowering::ContainCheckAndChain(GenTree* tree) } } - JITDUMP("Lowered And chain:\n"); + JITDUMP("Lowered `AND` chain:\n"); DISPTREE(tree); } } diff --git a/src/coreclr/jit/lsrabuild.cpp b/src/coreclr/jit/lsrabuild.cpp index 58889895249e12..e8ef972f95a98d 100644 --- a/src/coreclr/jit/lsrabuild.cpp +++ b/src/coreclr/jit/lsrabuild.cpp @@ -3145,7 +3145,7 @@ int LinearScan::BuildOperandUses(GenTree* node, regMaskTP candidates) if (node->OperIs(GT_MUL) || node->OperIsCompare() || node->OperIs(GT_AND)) { // Can be contained for MultiplyAdd on arm64. - // Compare and And may be contained due to If Conversion. + // Compare and AND may be contained due to If Conversion. return BuildBinaryUses(node->AsOp(), candidates); } if (node->OperIs(GT_NEG, GT_CAST, GT_LSH)) From e2c8f5ddcc335c2f9f8ca6d9a2cf5c18c555078a Mon Sep 17 00:00:00 2001 From: Alan Hayward Date: Mon, 8 Aug 2022 16:56:51 +0100 Subject: [PATCH 23/26] Compare chains should not contain tst compares Change-Id: I8a1761e1e89f589e1daf0318e120aae5dd3d7241 CustomizedGitHooks: yes --- src/coreclr/jit/gentree.h | 12 ++++++++++++ src/coreclr/jit/lowerarmarch.cpp | 12 ++++++------ 2 files changed, 18 insertions(+), 6 deletions(-) diff --git a/src/coreclr/jit/gentree.h b/src/coreclr/jit/gentree.h index 4c6cd1831b18ba..43ea5b7d992583 100644 --- a/src/coreclr/jit/gentree.h +++ b/src/coreclr/jit/gentree.h @@ -1359,6 +1359,18 @@ struct GenTree return OperIsCompare(OperGet()); } + // Oper is a compare that generates a cmp instruction (as opposed to a test instruction). + static bool OperIsCmpCompare(genTreeOps gtOper) + { + static_assert_no_msg(AreContiguous(GT_EQ, GT_NE, GT_LT, GT_LE, GT_GE, GT_GT)); + return (GT_EQ <= gtOper) && (gtOper <= GT_GT); + } + + bool OperIsCmpCompare() const + { + return OperIsCmpCompare(OperGet()); + } + static bool OperIsConditional(genTreeOps gtOper) { return (GT_SELECT == gtOper); diff --git a/src/coreclr/jit/lowerarmarch.cpp b/src/coreclr/jit/lowerarmarch.cpp index 099be20de2848c..0ff88151503201 100644 --- a/src/coreclr/jit/lowerarmarch.cpp +++ b/src/coreclr/jit/lowerarmarch.cpp @@ -2075,7 +2075,7 @@ bool Lowering::IsValidCompareChain(GenTree* child, GenTree* parent) if (child->isContainedAndNotIntOrIImmed()) { // Already have a chain. - assert(child->OperIs(GT_AND) || child->OperIsCompare()); + assert(child->OperIs(GT_AND) || child->OperIsCmpCompare()); return true; } else @@ -2086,7 +2086,7 @@ bool Lowering::IsValidCompareChain(GenTree* child, GenTree* parent) return IsValidCompareChain(child->AsOp()->gtGetOp2(), child) && IsValidCompareChain(child->AsOp()->gtGetOp1(), child); } - else if (child->OperIsCompare()) + else if (child->OperIsCmpCompare()) { // Can the child compare be contained. return IsSafeToContainMem(parent, child); @@ -2148,7 +2148,7 @@ bool Lowering::ContainCheckCompareChain(GenTree* child, GenTree* parent, GenTree child->SetContained(); return true; } - else if (child->OperIsCompare()) + else if (child->OperIsCmpCompare()) { child->AsOp()->SetContained(); @@ -2194,7 +2194,7 @@ void Lowering::ContainCheckCompareChainForAnd(GenTree* tree) if (startOfChain != nullptr) { // The earliest node in the chain will be generated as a standard compare. - assert(startOfChain->OperIsCompare()); + assert(startOfChain->OperIsCmpCompare()); startOfChain->AsOp()->gtGetOp1()->ClearContained(); startOfChain->AsOp()->gtGetOp2()->ClearContained(); ContainCheckCompare(startOfChain->AsOp()); @@ -2215,7 +2215,7 @@ void Lowering::ContainCheckCompareChainForAnd(GenTree* tree) // void Lowering::ContainCheckConditionalCompare(GenTreeOp* cmp) { - assert(cmp->OperIsCompare()); + assert(cmp->OperIsCmpCompare()); GenTree* op2 = cmp->gtOp2; if (op2->IsCnsIntOrI() && !op2->AsIntCon()->ImmedValNeedsReloc(comp)) @@ -2249,7 +2249,7 @@ void Lowering::ContainCheckSelect(GenTreeConditional* node) if (startOfChain != nullptr) { // The earliest node in the chain will be generated as a standard compare. - assert(startOfChain->OperIsCompare()); + assert(startOfChain->OperIsCmpCompare()); startOfChain->AsOp()->gtGetOp1()->ClearContained(); startOfChain->AsOp()->gtGetOp2()->ClearContained(); ContainCheckCompare(startOfChain->AsOp()); From 25696f1d4f28387580246c1a80bf76489c8fdde9 Mon Sep 17 00:00:00 2001 From: Alan Hayward Date: Tue, 9 Aug 2022 10:58:57 +0100 Subject: [PATCH 24/26] Don't allow tst compares in codegeneration of compare chains --- src/coreclr/jit/codegenarm64.cpp | 2 +- src/coreclr/jit/codegenlinear.cpp | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/coreclr/jit/codegenarm64.cpp b/src/coreclr/jit/codegenarm64.cpp index ce158fce2cce4d..14bff5b5c67e2c 100644 --- a/src/coreclr/jit/codegenarm64.cpp +++ b/src/coreclr/jit/codegenarm64.cpp @@ -4527,7 +4527,7 @@ void CodeGen::genCodeForContainedCompareChain(GenTree* tree, bool* inChain, GenC } else { - assert(tree->OperIsCompare()); + assert(tree->OperIsCmpCompare()); // Generate the compare, putting the result in the flags register. if (!*inChain) diff --git a/src/coreclr/jit/codegenlinear.cpp b/src/coreclr/jit/codegenlinear.cpp index abc2d9e5a0d714..e3ac21ba504a27 100644 --- a/src/coreclr/jit/codegenlinear.cpp +++ b/src/coreclr/jit/codegenlinear.cpp @@ -1629,7 +1629,7 @@ void CodeGen::genConsumeRegs(GenTree* tree) assert(cast->isContained()); genConsumeAddress(cast->CastOp()); } - else if (tree->OperIsCompare() || tree->OperIs(GT_AND)) + else if (tree->OperIsCmpCompare() || tree->OperIs(GT_AND)) { // Compares and ANDs may be contained in a conditional chain. genConsumeRegs(tree->gtGetOp1()); From a3d5d0cb20f5ad03a33c6a28ceea7c52d193237c Mon Sep 17 00:00:00 2001 From: Alan Hayward Date: Tue, 9 Aug 2022 11:29:49 +0100 Subject: [PATCH 25/26] Add tests for chains with tst compares --- .../JIT/opt/Compares/compareAndTestChains.cs | 367 ++++++++++++++++++ .../opt/Compares/compareAndTestChains.csproj | 12 + 2 files changed, 379 insertions(+) create mode 100644 src/tests/JIT/opt/Compares/compareAndTestChains.cs create mode 100644 src/tests/JIT/opt/Compares/compareAndTestChains.csproj diff --git a/src/tests/JIT/opt/Compares/compareAndTestChains.cs b/src/tests/JIT/opt/Compares/compareAndTestChains.cs new file mode 100644 index 00000000000000..c89e04f487ecf1 --- /dev/null +++ b/src/tests/JIT/opt/Compares/compareAndTestChains.cs @@ -0,0 +1,367 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +// unit test for compare AND chains that include a binary test. + +using System; +using System.Runtime.CompilerServices; + +public class ComparisonTestAndTestChains +{ + // Using bitwise AND to ensure compare chains are generated. + + [MethodImpl(MethodImplOptions.NoInlining)] + public static bool Eq_byte_bool(byte a1, bool a2) => (a1 == 10) & !a2; + + [MethodImpl(MethodImplOptions.NoInlining)] + public static bool Eq_short_bool(short a1, bool a2) => (a1 == 10) & !a2; + + [MethodImpl(MethodImplOptions.NoInlining)] + public static bool Eq_int_bool(int a1, bool a2) => (a1 == 10) & !a2; + + [MethodImpl(MethodImplOptions.NoInlining)] + public static bool Eq_long_bool(long a1, bool a2) => (a1 == 10) & !a2; + + [MethodImpl(MethodImplOptions.NoInlining)] + public static bool Eq_ushort_bool(ushort a1, bool a2) => (a1 == 10) & !a2; + + [MethodImpl(MethodImplOptions.NoInlining)] + public static bool Eq_uint_bool(uint a1, bool a2) => (a1 == 10) & !a2; + + [MethodImpl(MethodImplOptions.NoInlining)] + public static bool Eq_ulong_bool(ulong a1, bool a2) => (a1 == 10) & !a2; + + + [MethodImpl(MethodImplOptions.NoInlining)] + public static bool Ne_byte_bool(byte a1, bool a2) => (a1 != 5) & !a2; + + [MethodImpl(MethodImplOptions.NoInlining)] + public static bool Ne_short_bool(short a1, bool a2) => (a1 != 5) & !a2; + + [MethodImpl(MethodImplOptions.NoInlining)] + public static bool Ne_int_bool(int a1, bool a2) => (a1 != 5) & !a2; + + [MethodImpl(MethodImplOptions.NoInlining)] + public static bool Ne_long_bool(long a1, bool a2) => (a1 != 5) & !a2; + + [MethodImpl(MethodImplOptions.NoInlining)] + public static bool Ne_ushort_bool(ushort a1, bool a2) => (a1 != 5) & !a2; + + [MethodImpl(MethodImplOptions.NoInlining)] + public static bool Ne_uint_bool(uint a1, bool a2) => (a1 != 5) & !a2; + + [MethodImpl(MethodImplOptions.NoInlining)] + public static bool Ne_ulong_bool(ulong a1, bool a2) => (a1 != 5) & !a2; + + + [MethodImpl(MethodImplOptions.NoInlining)] + public static bool Lt_byte_bool(byte a1, bool a2) => (a1 < 5) & !a2; + + [MethodImpl(MethodImplOptions.NoInlining)] + public static bool Lt_short_bool(short a1, bool a2) => (a1 < 5) & !a2; + + [MethodImpl(MethodImplOptions.NoInlining)] + public static bool Lt_int_bool(int a1, bool a2) => (a1 < 5) & !a2; + + [MethodImpl(MethodImplOptions.NoInlining)] + public static bool Lt_long_bool(long a1, bool a2) => (a1 < 5) & !a2; + + [MethodImpl(MethodImplOptions.NoInlining)] + public static bool Lt_ushort_bool(ushort a1, bool a2) => (a1 < 5) & !a2; + + [MethodImpl(MethodImplOptions.NoInlining)] + public static bool Lt_uint_bool(uint a1, bool a2) => (a1 < 5) & !a2; + + [MethodImpl(MethodImplOptions.NoInlining)] + public static bool Lt_ulong_bool(ulong a1, bool a2) => (a1 < 5) & !a2; + + + [MethodImpl(MethodImplOptions.NoInlining)] + public static bool Le_byte_bool(byte a1, bool a2) => (a1 <= 5) & !a2; + + [MethodImpl(MethodImplOptions.NoInlining)] + public static bool Le_short_bool(short a1, bool a2) => (a1 <= 5) & !a2; + + [MethodImpl(MethodImplOptions.NoInlining)] + public static bool Le_int_bool(int a1, bool a2) => (a1 <= 5) & !a2; + + [MethodImpl(MethodImplOptions.NoInlining)] + public static bool Le_long_bool(long a1, bool a2) => (a1 <= 5) & !a2; + + [MethodImpl(MethodImplOptions.NoInlining)] + public static bool Le_ushort_bool(ushort a1, bool a2) => (a1 <= 5) & !a2; + + [MethodImpl(MethodImplOptions.NoInlining)] + public static bool Le_uint_bool(uint a1, bool a2) => (a1 <= 5) & !a2; + + [MethodImpl(MethodImplOptions.NoInlining)] + public static bool Le_ulong_bool(ulong a1, bool a2) => (a1 <= 5) & !a2; + + + [MethodImpl(MethodImplOptions.NoInlining)] + public static bool Gt_byte_bool(byte a1, bool a2) => (a1 > 5) & !a2; + + [MethodImpl(MethodImplOptions.NoInlining)] + public static bool Gt_short_bool(short a1, bool a2) => (a1 > 5) & !a2; + + [MethodImpl(MethodImplOptions.NoInlining)] + public static bool Gt_int_bool(int a1, bool a2) => (a1 > 5) & !a2; + + [MethodImpl(MethodImplOptions.NoInlining)] + public static bool Gt_long_bool(long a1, bool a2) => (a1 > 5) & !a2; + + [MethodImpl(MethodImplOptions.NoInlining)] + public static bool Gt_ushort_bool(ushort a1, bool a2) => (a1 > 5) & !a2; + + [MethodImpl(MethodImplOptions.NoInlining)] + public static bool Gt_uint_bool(uint a1, bool a2) => (a1 > 5) & !a2; + + [MethodImpl(MethodImplOptions.NoInlining)] + public static bool Gt_ulong_bool(ulong a1, bool a2) => (a1 > 5) & !a2; + + + [MethodImpl(MethodImplOptions.NoInlining)] + public static bool Ge_byte_bool(byte a1, bool a2) => (a1 >= 5) & !a2; + + [MethodImpl(MethodImplOptions.NoInlining)] + public static bool Ge_short_bool(short a1, bool a2) => (a1 >= 5) & !a2; + + [MethodImpl(MethodImplOptions.NoInlining)] + public static bool Ge_int_bool(int a1, bool a2) => (a1 >= 5) & !a2; + + [MethodImpl(MethodImplOptions.NoInlining)] + public static bool Ge_long_bool(long a1, bool a2) => (a1 >= 5) & !a2; + + [MethodImpl(MethodImplOptions.NoInlining)] + public static bool Ge_ushort_bool(ushort a1, bool a2) => (a1 >= 5) & !a2; + + [MethodImpl(MethodImplOptions.NoInlining)] + public static bool Ge_uint_bool(uint a1, bool a2) => (a1 >= 5) & !a2; + + [MethodImpl(MethodImplOptions.NoInlining)] + public static bool Ge_ulong_bool(ulong a1, bool a2) => (a1 >= 5) & !a2; + + + [MethodImpl(MethodImplOptions.NoInlining)] + public static int Main() + { + if (!Eq_byte_bool(10, false)) + { + Console.WriteLine("ComparisonTestAndTestChains:Eq_byte_bool(10, false) failed"); + return 101; + } + if (!Eq_short_bool(10, false)) + { + Console.WriteLine("ComparisonTestAndTestChains:Eq_short_bool(10, false) failed"); + return 101; + } + if (!Eq_int_bool(10, false)) + { + Console.WriteLine("ComparisonTestAndTestChains:Eq_int_bool(10, false) failed"); + return 101; + } + if (!Eq_long_bool(10, false)) + { + Console.WriteLine("ComparisonTestAndTestChains:Eq_long_bool(10, false) failed"); + return 101; + } + if (!Eq_ushort_bool(10, false)) + { + Console.WriteLine("ComparisonTestAndTestChains:Eq_ushort_bool(10, false) failed"); + return 101; + } + if (!Eq_uint_bool(10, false)) + { + Console.WriteLine("ComparisonTestAndTestChains:Eq_uint_bool(10, false) failed"); + return 101; + } + if (!Eq_ulong_bool(10, false)) + { + Console.WriteLine("ComparisonTestAndTestChains:Eq_ulong_bool(10, false) failed"); + return 101; + } + + if (!Ne_byte_bool(10, false)) + { + Console.WriteLine("ComparisonTestAndTestChains:Ne_byte_bool(10, false) failed"); + return 101; + } + if (!Ne_short_bool(10, false)) + { + Console.WriteLine("ComparisonTestAndTestChains:Ne_short_bool(10, false) failed"); + return 101; + } + if (!Ne_int_bool(10, false)) + { + Console.WriteLine("ComparisonTestAndTestChains:Ne_int_bool(10, false) failed"); + return 101; + } + if (!Ne_long_bool(10, false)) + { + Console.WriteLine("ComparisonTestAndTestChains:Ne_long_bool(10, false) failed"); + return 101; + } + if (!Ne_ushort_bool(10, false)) + { + Console.WriteLine("ComparisonTestAndTestChains:Ne_ushort_bool(10, false) failed"); + return 101; + } + if (!Ne_uint_bool(10, false)) + { + Console.WriteLine("ComparisonTestAndTestChains:Ne_uint_bool(10, false) failed"); + return 101; + } + if (!Ne_ulong_bool(10, false)) + { + Console.WriteLine("ComparisonTestAndTestChains:Ne_ulong_bool(10, false) failed"); + return 101; + } + + if (!Lt_byte_bool(3, false)) + { + Console.WriteLine("ComparisonTestAndTestChains:Lt_byte_bool(3, false) failed"); + return 101; + } + if (!Lt_short_bool(3, false)) + { + Console.WriteLine("ComparisonTestAndTestChains:Lt_short_bool(3, false) failed"); + return 101; + } + if (!Lt_int_bool(3, false)) + { + Console.WriteLine("ComparisonTestAndTestChains:Lt_int_bool(3, false) failed"); + return 101; + } + if (!Lt_long_bool(3, false)) + { + Console.WriteLine("ComparisonTestAndTestChains:Lt_long_bool(3, false) failed"); + return 101; + } + if (!Lt_ushort_bool(3, false)) + { + Console.WriteLine("ComparisonTestAndTestChains:Lt_ushort_bool(3, false) failed"); + return 101; + } + if (!Lt_uint_bool(3, false)) + { + Console.WriteLine("ComparisonTestAndTestChains:Lt_uint_bool(3, false) failed"); + return 101; + } + if (!Lt_ulong_bool(3, false)) + { + Console.WriteLine("ComparisonTestAndTestChains:Lt_ulong_bool(3, false) failed"); + return 101; + } + + if (!Le_byte_bool(3, false)) + { + Console.WriteLine("ComparisonTestAndTestChains:Le_byte_bool(3, false) failed"); + return 101; + } + if (!Le_short_bool(3, false)) + { + Console.WriteLine("ComparisonTestAndTestChains:Le_short_bool(3, false) failed"); + return 101; + } + if (!Le_int_bool(3, false)) + { + Console.WriteLine("ComparisonTestAndTestChains:Le_int_bool(3, false) failed"); + return 101; + } + if (!Le_long_bool(3, false)) + { + Console.WriteLine("ComparisonTestAndTestChains:Le_long_bool(3, false) failed"); + return 101; + } + if (!Le_ushort_bool(3, false)) + { + Console.WriteLine("ComparisonTestAndTestChains:Le_ushort_bool(3, false) failed"); + return 101; + } + if (!Le_uint_bool(3, false)) + { + Console.WriteLine("ComparisonTestAndTestChains:Le_uint_bool(3, false) failed"); + return 101; + } + if (!Le_ulong_bool(3, false)) + { + Console.WriteLine("ComparisonTestAndTestChains:Le_ulong_bool(3, false) failed"); + return 101; + } + + if (!Gt_byte_bool(10, false)) + { + Console.WriteLine("ComparisonTestAndTestChains:Gt_byte_bool(10, false) failed"); + return 101; + } + if (!Gt_short_bool(10, false)) + { + Console.WriteLine("ComparisonTestAndTestChains:Gt_short_bool(10, false) failed"); + return 101; + } + if (!Gt_int_bool(10, false)) + { + Console.WriteLine("ComparisonTestAndTestChains:Gt_int_bool(10, false) failed"); + return 101; + } + if (!Gt_long_bool(10, false)) + { + Console.WriteLine("ComparisonTestAndTestChains:Gt_long_bool(10, false) failed"); + return 101; + } + if (!Gt_ushort_bool(10, false)) + { + Console.WriteLine("ComparisonTestAndTestChains:Gt_ushort_bool(10, false) failed"); + return 101; + } + if (!Gt_uint_bool(10, false)) + { + Console.WriteLine("ComparisonTestAndTestChains:Gt_uint_bool(10, false) failed"); + return 101; + } + if (!Gt_ulong_bool(10, false)) + { + Console.WriteLine("ComparisonTestAndTestChains:Gt_ulong_bool(10, false) failed"); + return 101; + } + + if (!Ge_byte_bool(10, false)) + { + Console.WriteLine("ComparisonTestAndTestChains:Ge_byte_bool(10, false) failed"); + return 101; + } + if (!Ge_short_bool(10, false)) + { + Console.WriteLine("ComparisonTestAndTestChains:Ge_short_bool(10, false) failed"); + return 101; + } + if (!Ge_int_bool(10, false)) + { + Console.WriteLine("ComparisonTestAndTestChains:Ge_int_bool(10, false) failed"); + return 101; + } + if (!Ge_long_bool(10, false)) + { + Console.WriteLine("ComparisonTestAndTestChains:Ge_long_bool(10, false) failed"); + return 101; + } + if (!Ge_ushort_bool(10, false)) + { + Console.WriteLine("ComparisonTestAndTestChains:Ge_ushort_bool(10, false) failed"); + return 101; + } + if (!Ge_uint_bool(10, false)) + { + Console.WriteLine("ComparisonTestAndTestChains:Ge_uint_bool(10, false) failed"); + return 101; + } + if (!Ge_ulong_bool(10, false)) + { + Console.WriteLine("ComparisonTestAndTestChains:Le_ulong_bool(10, false) failed"); + return 101; + } + + Console.WriteLine("PASSED"); + return 100; + } +} diff --git a/src/tests/JIT/opt/Compares/compareAndTestChains.csproj b/src/tests/JIT/opt/Compares/compareAndTestChains.csproj new file mode 100644 index 00000000000000..5e5fbae5cb863b --- /dev/null +++ b/src/tests/JIT/opt/Compares/compareAndTestChains.csproj @@ -0,0 +1,12 @@ + + + Exe + + + PdbOnly + True + + + + + From cac2740e1192f8f2de0d6076d7203033cbd67175 Mon Sep 17 00:00:00 2001 From: Alan Hayward Date: Tue, 9 Aug 2022 16:29:42 +0100 Subject: [PATCH 26/26] Don't allow tst compares in lsrabuild of compare chains --- src/coreclr/jit/lsrabuild.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/coreclr/jit/lsrabuild.cpp b/src/coreclr/jit/lsrabuild.cpp index e8ef972f95a98d..8ae4f5107cd5fc 100644 --- a/src/coreclr/jit/lsrabuild.cpp +++ b/src/coreclr/jit/lsrabuild.cpp @@ -3142,7 +3142,7 @@ int LinearScan::BuildOperandUses(GenTree* node, regMaskTP candidates) } #endif // FEATURE_HW_INTRINSICS #ifdef TARGET_ARM64 - if (node->OperIs(GT_MUL) || node->OperIsCompare() || node->OperIs(GT_AND)) + if (node->OperIs(GT_MUL) || node->OperIsCmpCompare() || node->OperIs(GT_AND)) { // Can be contained for MultiplyAdd on arm64. // Compare and AND may be contained due to If Conversion.