Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

JIT: extend RBO partial inference to unsigned relops #75804

Merged
merged 3 commits into from
Sep 19, 2022
+160 −56
Merged

JIT: extend RBO partial inference to unsigned relops #75804

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
42026b9
JIT: extend RBO partial inference to unsigned relops
AndyAyersMS Sep 17, 2022
461fedd
fix macro
AndyAyersMS Sep 18, 2022
74380a0
fix test
AndyAyersMS Sep 19, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
75 changes: 51 additions & 24 deletions src/coreclr/jit/redundantbranchopts.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -143,45 +143,74 @@ 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[] =
{
// EQ
{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},
{V(LE), false, true, V(NE), true},
{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

Expand Down Expand Up @@ -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;
}
Expand All @@ -261,39 +288,38 @@ 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;
rii->canInferFromTrue = rule.canInferFromTrue;
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;
}
}
Expand All @@ -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))
{
Expand Down
79 changes: 50 additions & 29 deletions src/coreclr/jit/valuenum.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
//
Expand Down Expand Up @@ -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;
}
}

Expand Down
10 changes: 7 additions & 3 deletions src/coreclr/jit/valuenum.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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.
Expand Down
42 changes: 42 additions & 0 deletions src/tests/JIT/opt/RedundantBranch/RedundantBranchUnsigned.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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<char> M1(ReadOnlySpan<char> span, int i)
{
// Note `<` here instead of `<=`
//
if ((uint)i < (uint)span.Length)
{
return span.Slice(i);
}
return default;
}

[MethodImpl(MethodImplOptions.NoInlining)]
public ReadOnlySpan<char> M2(ReadOnlySpan<char> span, int i)
{
if ((uint)i <= (uint)span.Length)
{
return span.Slice(i);
}
return default;
}

public static int Main()
{
C c = new C();
var m1 = c.M1("hello", 2);
var m2 = c.M2("hello", 3);

return m1.Length + m2.Length + 95;
}
}
10 changes: 10 additions & 0 deletions src/tests/JIT/opt/RedundantBranch/RedundantBranchUnsigned.csproj
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
<Project Sdk="Microsoft.NET.Sdk">
<PropertyGroup>
<OutputType>Exe</OutputType>
<DebugType />
<Optimize>True</Optimize>
</PropertyGroup>
<ItemGroup>
<Compile Include="$(MSBuildProjectName).cs" />
</ItemGroup>
</Project>