diff --git a/src/coreclr/jit/redundantbranchopts.cpp b/src/coreclr/jit/redundantbranchopts.cpp index ad7f1255a39636..4b65d006af5d0d 100644 --- a/src/coreclr/jit/redundantbranchopts.cpp +++ b/src/coreclr/jit/redundantbranchopts.cpp @@ -143,7 +143,8 @@ struct RelopImplicationRule // // clang-format off // -#define V(x) (VNFunc)GT_##x +#define V(x) (VNFunc)GT_ ## x +#define U(x) VNF_ ## x ## _UN static const RelopImplicationRule s_implicationRules[] = { @@ -151,13 +152,17 @@ static const RelopImplicationRule s_implicationRules[] = {V(EQ), true, false, V(GE), false}, {V(EQ), true, false, V(LE), false}, {V(EQ), true, false, V(GT), true}, + {V(EQ), true, false, U(GT), true}, {V(EQ), true, false, V(LT), true}, + {V(EQ), true, false, U(LT), true}, // NE {V(NE), false, true, V(GE), true}, {V(NE), false, true, V(LE), true}, {V(NE), false, true, V(GT), false}, + {V(NE), false, true, U(GT), false}, {V(NE), false, true, V(LT), false}, + {V(NE), false, true, U(LT), false}, // LE {V(LE), false, true, V(EQ), false}, @@ -165,23 +170,47 @@ static const RelopImplicationRule s_implicationRules[] = {V(LE), false, true, V(GE), true}, {V(LE), false, true, V(LT), false}, + // LE_UN + {U(LE), false, true, V(EQ), false}, + {U(LE), false, true, V(NE), true}, + {U(LE), false, true, U(GE), true}, + {U(LE), false, true, U(LT), false}, + // GT {V(GT), true, false, V(EQ), true}, {V(GT), true, false, V(NE), false}, {V(GT), true, false, V(GE), false}, {V(GT), true, false, V(LT), true}, + // GT_UN + {U(GT), true, false, V(EQ), true}, + {U(GT), true, false, V(NE), false}, + {U(GT), true, false, U(GE), false}, + {U(GT), true, false, U(LT), true}, + // GE {V(GE), false, true, V(EQ), false}, {V(GE), false, true, V(NE), true}, {V(GE), false, true, V(LE), true}, {V(GE), false, true, V(GT), false}, + // GE_UN + {U(GE), false, true, V(EQ), false}, + {U(GE), false, true, V(NE), true}, + {U(GE), false, true, U(LE), true}, + {U(GE), false, true, U(GT), false}, + // LT {V(LT), true, false, V(EQ), true}, {V(LT), true, false, V(NE), false}, {V(LT), true, false, V(LE), false}, {V(LT), true, false, V(GT), true}, + + // LT_UN + {U(LT), true, false, V(EQ), true}, + {U(LT), true, false, V(NE), false}, + {U(LT), true, false, U(LE), false}, + {U(LT), true, false, U(GT), true}, }; // clang-format on @@ -237,17 +266,15 @@ void Compiler::optRelopImpliesRelop(RelopImplicationInfo* rii) // VNs are not directly related. See if dominating // compare encompasses a related VN. // - VNFuncApp funcApp; - if (!vnStore->GetVNFunc(rii->domCmpNormVN, &funcApp)) + VNFuncApp domApp; + if (!vnStore->GetVNFunc(rii->domCmpNormVN, &domApp)) { return; } - genTreeOps const oper = genTreeOps(funcApp.m_func); - // Exclude floating point relops. // - if (varTypeIsFloating(vnStore->TypeOfVN(funcApp.m_args[0]))) + if (varTypeIsFloating(vnStore->TypeOfVN(domApp.m_args[0]))) { return; } @@ -261,30 +288,29 @@ void Compiler::optRelopImpliesRelop(RelopImplicationInfo* rii) const bool inRange = true; #endif - // Dominating compare has the form R(x,y) - // See if tree compare has the form R*(x,y) or R*(y,x) where we can infer R* from R - // - // Could also extend to the unsigned VN relops. + // If the dominating compare has the form R(x,y), see if tree compare has the + // form R*(x,y) or R*(y,x) where we can infer R* from R. // - VNFuncApp treeApp; - if (inRange && GenTree::OperIsCompare(oper) && vnStore->GetVNFunc(rii->treeNormVN, &treeApp)) + VNFunc const domFunc = domApp.m_func; + VNFuncApp treeApp; + if (inRange && ValueNumStore::VNFuncIsComparison(domFunc) && vnStore->GetVNFunc(rii->treeNormVN, &treeApp)) { - genTreeOps const treeOper = genTreeOps(treeApp.m_func); - genTreeOps domOper = oper; - - if (((treeApp.m_args[0] == funcApp.m_args[0]) && (treeApp.m_args[1] == funcApp.m_args[1])) || - ((treeApp.m_args[0] == funcApp.m_args[1]) && (treeApp.m_args[1] == funcApp.m_args[0]))) + if (((treeApp.m_args[0] == domApp.m_args[0]) && (treeApp.m_args[1] == domApp.m_args[1])) || + ((treeApp.m_args[0] == domApp.m_args[1]) && (treeApp.m_args[1] == domApp.m_args[0]))) { - const bool swapped = (treeApp.m_args[0] == funcApp.m_args[1]); + const bool swapped = (treeApp.m_args[0] == domApp.m_args[1]); + + VNFunc const treeFunc = treeApp.m_func; + VNFunc domFunc1 = domFunc; if (swapped) { - domOper = GenTree::SwapRelop(domOper); + domFunc1 = ValueNumStore::SwapRelop(domFunc); } for (const RelopImplicationRule& rule : s_implicationRules) { - if ((rule.domRelop == (VNFunc)domOper) && (rule.treeRelop == (VNFunc)treeOper)) + if ((rule.domRelop == domFunc1) && (rule.treeRelop == treeFunc)) { rii->canInfer = true; rii->vnRelation = ValueNumStore::VN_RELATION_KIND::VRK_Inferred; @@ -292,8 +318,8 @@ void Compiler::optRelopImpliesRelop(RelopImplicationInfo* rii) rii->canInferFromFalse = rule.canInferFromFalse; rii->reverseSense = rule.reverse; - JITDUMP("Can infer %s from [%s] %s\n", GenTree::OpName(treeOper), - rii->canInferFromTrue ? "true" : "false", GenTree::OpName(oper)); + JITDUMP("Can infer %s from [%s] dominating %s\n", ValueNumStore::VNFuncName(treeFunc), + rii->canInferFromTrue ? "true" : "false", ValueNumStore::VNFuncName(domFunc)); return; } } @@ -305,17 +331,18 @@ void Compiler::optRelopImpliesRelop(RelopImplicationInfo* rii) // // Look for {EQ,NE}({AND,OR,NOT}, 0) // + genTreeOps const oper = genTreeOps(domFunc); if (!GenTree::StaticOperIs(oper, GT_EQ, GT_NE)) { return; } - if (funcApp.m_args[1] != vnStore->VNZeroForType(TYP_INT)) + if (domApp.m_args[1] != vnStore->VNZeroForType(TYP_INT)) { return; } - const ValueNum predVN = funcApp.m_args[0]; + const ValueNum predVN = domApp.m_args[0]; VNFuncApp predFuncApp; if (!vnStore->GetVNFunc(predVN, &predFuncApp)) { diff --git a/src/coreclr/jit/valuenum.cpp b/src/coreclr/jit/valuenum.cpp index 2495cd4e90efae..1e2a09171601bb 100644 --- a/src/coreclr/jit/valuenum.cpp +++ b/src/coreclr/jit/valuenum.cpp @@ -5283,6 +5283,52 @@ bool ValueNumStore::IsVNHandle(ValueNum vn) return c->m_attribs == CEA_Handle; } +//------------------------------------------------------------------------ +// SwapRelop: return VNFunc for swapped relop +// +// Arguments: +// vnf - vnf for original relop +// +// Returns: +// VNFunc for swapped relop, or VNF_MemOpaque if the original VNFunc +// was not a relop. +// +VNFunc ValueNumStore::SwapRelop(VNFunc vnf) +{ + VNFunc swappedFunc = VNF_MemOpaque; + if (vnf >= VNF_Boundary) + { + switch (vnf) + { + case VNF_LT_UN: + swappedFunc = VNF_GT_UN; + break; + case VNF_LE_UN: + swappedFunc = VNF_GE_UN; + break; + case VNF_GE_UN: + swappedFunc = VNF_LE_UN; + break; + case VNF_GT_UN: + swappedFunc = VNF_LT_UN; + break; + default: + break; + } + } + else + { + const genTreeOps op = (genTreeOps)vnf; + + if (GenTree::OperIsCompare(op)) + { + swappedFunc = (VNFunc)GenTree::SwapRelop(op); + } + } + + return swappedFunc; +} + //------------------------------------------------------------------------ // GetRelatedRelop: return value number for reversed/swapped comparison // @@ -5354,36 +5400,11 @@ ValueNum ValueNumStore::GetRelatedRelop(ValueNum vn, VN_RELATION_KIND vrk) // if (swap) { - if (newFunc >= VNF_Boundary) - { - switch (newFunc) - { - case VNF_LT_UN: - newFunc = VNF_GT_UN; - break; - case VNF_LE_UN: - newFunc = VNF_GE_UN; - break; - case VNF_GE_UN: - newFunc = VNF_LE_UN; - break; - case VNF_GT_UN: - newFunc = VNF_LT_UN; - break; - default: - return NoVN; - } - } - else - { - const genTreeOps op = (genTreeOps)newFunc; + newFunc = SwapRelop(newFunc); - if (!GenTree::OperIsCompare(op)) - { - return NoVN; - } - - newFunc = (VNFunc)GenTree::SwapRelop(op); + if (newFunc == VNF_MemOpaque) + { + return NoVN; } } diff --git a/src/coreclr/jit/valuenum.h b/src/coreclr/jit/valuenum.h index d998591039ffa6..dad46cb8cd840c 100644 --- a/src/coreclr/jit/valuenum.h +++ b/src/coreclr/jit/valuenum.h @@ -304,9 +304,6 @@ class ValueNumStore // (Requires InitValueNumStoreStatics to have been run.) static bool VNFuncIsCommutative(VNFunc vnf); - // Returns "true" iff "vnf" is a comparison (and thus binary) operator. - static bool VNFuncIsComparison(VNFunc vnf); - bool VNEvalCanFoldBinaryFunc(var_types type, VNFunc func, ValueNum arg0VN, ValueNum arg1VN); bool VNEvalCanFoldUnaryFunc(var_types type, VNFunc func, ValueNum arg0VN); @@ -982,6 +979,13 @@ class ValueNumStore // ValueNum GetRelatedRelop(ValueNum vn, VN_RELATION_KIND vrk); + // Return VNFunc for swapped relop, or VNF_MemOpaque if the function + // is not a relop. + static VNFunc SwapRelop(VNFunc vnf); + + // Returns "true" iff "vnf" is a comparison (and thus binary) operator. + static bool VNFuncIsComparison(VNFunc vnf); + // Convert a vartype_t to the value number's storage type for that vartype_t. // For example, ValueNum of type TYP_LONG are stored in a map of INT64 variables. // Lang is the language (C++) type for the corresponding vartype_t. diff --git a/src/tests/JIT/opt/RedundantBranch/RedundantBranchUnsigned.cs b/src/tests/JIT/opt/RedundantBranch/RedundantBranchUnsigned.cs new file mode 100644 index 00000000000000..788d487af22b2f --- /dev/null +++ b/src/tests/JIT/opt/RedundantBranch/RedundantBranchUnsigned.cs @@ -0,0 +1,42 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using System; +using System.Runtime.CompilerServices; + +// Runtime 65327 +// M1 and M2 should generate the same code + +public class RedundantBranchUnsigned +{ + [MethodImpl(MethodImplOptions.NoInlining)] + public ReadOnlySpan M1(ReadOnlySpan span, int i) + { + // Note `<` here instead of `<=` + // + if ((uint)i < (uint)span.Length) + { + return span.Slice(i); + } + return default; + } + + [MethodImpl(MethodImplOptions.NoInlining)] + public ReadOnlySpan M2(ReadOnlySpan span, int i) + { + if ((uint)i <= (uint)span.Length) + { + return span.Slice(i); + } + return default; + } + + public static int Main() + { + var rbu = new RedundantBranchUnsigned(); + var m1 = rbu.M1("hello", 2); + var m2 = rbu.M2("hello", 3); + + return m1.Length + m2.Length + 95; + } +} diff --git a/src/tests/JIT/opt/RedundantBranch/RedundantBranchUnsigned.csproj b/src/tests/JIT/opt/RedundantBranch/RedundantBranchUnsigned.csproj new file mode 100644 index 00000000000000..19781e26c20d81 --- /dev/null +++ b/src/tests/JIT/opt/RedundantBranch/RedundantBranchUnsigned.csproj @@ -0,0 +1,10 @@ + + + Exe + + True + + + + +