From 7ce5d0fdf728bb2ae213541b17c2e489d2b06d09 Mon Sep 17 00:00:00 2001 From: Jakob Botsch Nielsen Date: Sat, 11 May 2024 18:57:31 +0200 Subject: [PATCH] JIT: Expand optRelopImpliesRelop for constants as op1 (#102024) VN does not guarantee that constants in relops are first, so swap them if this isn't the case. This allows the logic to prove the value of more relops. --- src/coreclr/jit/compiler.h | 3 + src/coreclr/jit/redundantbranchopts.cpp | 172 ++++++++++++++++-------- 2 files changed, 116 insertions(+), 59 deletions(-) diff --git a/src/coreclr/jit/compiler.h b/src/coreclr/jit/compiler.h index 88b63c49ff8a4..080274dfb9d1b 100644 --- a/src/coreclr/jit/compiler.h +++ b/src/coreclr/jit/compiler.h @@ -7502,6 +7502,9 @@ class Compiler BitVecTraits* optReachableBitVecTraits; BitVec optReachableBitVec; void optRelopImpliesRelop(RelopImplicationInfo* rii); + bool optRelopTryInferWithOneEqualOperand(const VNFuncApp& domApp, + const VNFuncApp& treeApp, + RelopImplicationInfo* rii); /************************************************************************** * Value/Assertion propagation diff --git a/src/coreclr/jit/redundantbranchopts.cpp b/src/coreclr/jit/redundantbranchopts.cpp index d4a678875848b..ce5a2733f0534 100644 --- a/src/coreclr/jit/redundantbranchopts.cpp +++ b/src/coreclr/jit/redundantbranchopts.cpp @@ -509,66 +509,11 @@ void Compiler::optRelopImpliesRelop(RelopImplicationInfo* rii) } } - // Given R(x, cns1) and R*(x, cns2) see if we can infer R* from R. - // We assume cns1 and cns2 are always on the RHS of the compare. - if ((treeApp.m_args[0] == domApp.m_args[0]) && vnStore->IsVNConstant(treeApp.m_args[1]) && - vnStore->IsVNConstant(domApp.m_args[1]) && varTypeIsIntOrI(vnStore->TypeOfVN(treeApp.m_args[1])) && - vnStore->TypeOfVN(domApp.m_args[0]) == vnStore->TypeOfVN(treeApp.m_args[1]) && - vnStore->TypeOfVN(domApp.m_args[1]) == vnStore->TypeOfVN(treeApp.m_args[1])) + if (((treeApp.m_args[0] == domApp.m_args[0]) || (treeApp.m_args[0] == domApp.m_args[1]) || + (treeApp.m_args[1] == domApp.m_args[0]) || (treeApp.m_args[1] == domApp.m_args[1])) && + optRelopTryInferWithOneEqualOperand(domApp, treeApp, rii)) { - // We currently don't handle VNF_relop_UN funcs here - if (ValueNumStore::VNFuncIsSignedComparison(domApp.m_func) && - ValueNumStore::VNFuncIsSignedComparison(treeApp.m_func)) - { - // Dominating "X relop CNS" - const genTreeOps domOper = static_cast(domApp.m_func); - const target_ssize_t domCns = vnStore->CoercedConstantValue(domApp.m_args[1]); - - // Dominated "X relop CNS" - const genTreeOps treeOper = static_cast(treeApp.m_func); - const target_ssize_t treeCns = vnStore->CoercedConstantValue(treeApp.m_args[1]); - - // Example: - // - // void Test(int x) - // { - // if (x > 100) - // if (x > 10) - // Console.WriteLine("Taken!"); - // } - // - - // Corresponding BB layout: - // - // BB1: - // if (x <= 100) - // goto BB4 - // - // BB2: - // // x is known to be > 100 here - // if (x <= 10) // never true - // goto BB4 - // - // BB3: - // Console.WriteLine("Taken!"); - // - // BB4: - // return; - - // Check whether the dominating compare being "false" implies the dominated compare is known - // to be either "true" or "false". - RelopResult treeOperStatus = - IsCmp2ImpliedByCmp1(GenTree::ReverseRelop(domOper), domCns, treeOper, treeCns); - if (treeOperStatus != RelopResult::Unknown) - { - rii->canInfer = true; - rii->vnRelation = ValueNumStore::VN_RELATION_KIND::VRK_Inferred; - rii->canInferFromTrue = false; - rii->canInferFromFalse = true; - rii->reverseSense = treeOperStatus == RelopResult::AlwaysTrue; - return; - } - } + return; } } @@ -646,6 +591,115 @@ void Compiler::optRelopImpliesRelop(RelopImplicationInfo* rii) } } +//------------------------------------------------------------------------ +// optRelopTryInferWithOneEqualOperand: Given a domnating relop R(x, y) and +// another relop R*(a, b) that share an operand, try to see if we can infer +// something about R*(a, b). +// +// Arguments: +// domApp - The dominating relop R*(x, y) +// treeApp - The dominated relop R*(a, b) +// rii - [out] struct with relop implication information +// +// Returns: +// True if something was inferred; otherwise false. +// +bool Compiler::optRelopTryInferWithOneEqualOperand(const VNFuncApp& domApp, + const VNFuncApp& treeApp, + RelopImplicationInfo* rii) +{ + // Canonicalize constants to be on the right. + VNFunc domFunc = domApp.m_func; + ValueNum domOp1 = domApp.m_args[0]; + ValueNum domOp2 = domApp.m_args[1]; + + VNFunc treeFunc = treeApp.m_func; + ValueNum treeOp1 = treeApp.m_args[0]; + ValueNum treeOp2 = treeApp.m_args[1]; + + if (vnStore->IsVNConstant(domOp1)) + { + std::swap(domOp1, domOp2); + domFunc = ValueNumStore::SwapRelop(domFunc); + } + + if (vnStore->IsVNConstant(treeOp1)) + { + std::swap(treeOp1, treeOp2); + treeFunc = ValueNumStore::SwapRelop(treeFunc); + } + + // Given R(x, cns1) and R*(x, cns2) see if we can infer R* from R. + if ((treeOp1 != domOp1) || !vnStore->IsVNConstant(treeOp2) || !vnStore->IsVNConstant(domOp2)) + { + return false; + } + + var_types treeOp1Type = vnStore->TypeOfVN(treeOp1); + var_types treeOp2Type = vnStore->TypeOfVN(treeOp2); + var_types domOp1Type = vnStore->TypeOfVN(domOp1); + var_types domOp2Type = vnStore->TypeOfVN(domOp2); + if (!varTypeIsIntOrI(treeOp1Type) || (domOp1Type != treeOp2Type) || (domOp2Type != treeOp2Type)) + { + return false; + } + // We currently don't handle VNF_relop_UN funcs here + if (!ValueNumStore::VNFuncIsSignedComparison(domFunc) || !ValueNumStore::VNFuncIsSignedComparison(treeFunc)) + { + return false; + } + + // Dominating "X relop CNS" + const genTreeOps domOper = static_cast(domFunc); + const target_ssize_t domCns = vnStore->CoercedConstantValue(domOp2); + + // Dominated "X relop CNS" + const genTreeOps treeOper = static_cast(treeFunc); + const target_ssize_t treeCns = vnStore->CoercedConstantValue(treeOp2); + + // Example: + // + // void Test(int x) + // { + // if (x > 100) + // if (x > 10) + // Console.WriteLine("Taken!"); + // } + // + + // Corresponding BB layout: + // + // BB1: + // if (x <= 100) + // goto BB4 + // + // BB2: + // // x is known to be > 100 here + // if (x <= 10) // never true + // goto BB4 + // + // BB3: + // Console.WriteLine("Taken!"); + // + // BB4: + // return; + + // Check whether the dominating compare being "false" implies the dominated compare is known + // to be either "true" or "false". + RelopResult treeOperStatus = IsCmp2ImpliedByCmp1(GenTree::ReverseRelop(domOper), domCns, treeOper, treeCns); + if (treeOperStatus == RelopResult::Unknown) + { + return false; + } + + rii->canInfer = true; + rii->vnRelation = ValueNumStore::VN_RELATION_KIND::VRK_Inferred; + rii->canInferFromTrue = false; + rii->canInferFromFalse = true; + rii->reverseSense = treeOperStatus == RelopResult::AlwaysTrue; + return true; +} + //------------------------------------------------------------------------ // optRedundantBranch: try and optimize a possibly redundant branch //