Skip to content

JIT: Handle some "field offset computation" patterns #81998

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

Merged
jakobbotsch merged 10 commits into from
Apr 4, 2023
Merged

JIT: Handle some "field offset computation" patterns #81998

Show file tree
Hide file tree
Changes from 9 commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
9f755be
JIT: Handle some "field offset computation" patterns
jakobbotsch Feb 11, 2023
df73501
Oops
jakobbotsch Feb 11, 2023
b07d687
Merge branch 'main' into fold-local-offsets
jakobbotsch Mar 29, 2023
1c4c0ae
Merge branch 'main' of github.com:dotnet/runtime into fold-local-offsets
jakobbotsch Mar 31, 2023
8f9c4f2
Fix wrong identity for overflowing SUBs
jakobbotsch Mar 31, 2023
b944901
Handle in local morph instead; revert previous changes
jakobbotsch Mar 31, 2023
e1f4777
Handle field-to-field offset
jakobbotsch Mar 31, 2023
4239377
Generalize
jakobbotsch Mar 31, 2023
adb3474
Expand assertion to cover TYP_BYREF
jakobbotsch Apr 4, 2023
70627f4
Apply suggestions from code review
jakobbotsch Apr 4, 2023
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
28 changes: 28 additions & 0 deletions src/coreclr/jit/lclmorph.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -677,6 +677,34 @@ class LocalAddressVisitor final : public GenTreeVisitor<LocalAddressVisitor>
PopValue();
break;

case GT_SUB:
{
Value& rhs = TopValue(0);
Value& lhs = TopValue(1);
if (m_compiler->opts.OptimizationEnabled() && lhs.IsAddress() && rhs.IsAddress() &&
(lhs.LclNum() == rhs.LclNum()) && (rhs.Offset() <= lhs.Offset()) &&
FitsIn<int>(lhs.Offset() - rhs.Offset()))
{
// TODO-Correctness: Due to inlining we may end up with incorrectly typed SUB trees here.
Comment thread
jakobbotsch marked this conversation as resolved.
Outdated
assert(node->TypeIs(TYP_I_IMPL, TYP_BYREF));
Copy link
Copy Markdown
Member Author

@jakobbotsch jakobbotsch Apr 4, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Opened #84291 for this.


ssize_t result = (ssize_t)(lhs.Offset() - rhs.Offset());
*use = node = m_compiler->gtNewIconNode(result, TYP_I_IMPL);
Comment thread
jakobbotsch marked this conversation as resolved.
Outdated
INDEBUG(lhs.Consume());
INDEBUG(rhs.Consume());
PopValue();
PopValue();
m_stmtModified = true;
break;
}

EscapeValue(TopValue(0), node);
PopValue();
EscapeValue(TopValue(0), node);
PopValue();
break;
}

case GT_FIELD_ADDR:
if (node->AsField()->IsInstance())
{
Expand Down
39 changes: 36 additions & 3 deletions src/coreclr/jit/valuenum.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4450,7 +4450,7 @@ ValueNum ValueNumStore::EvalUsingMathIdentity(var_types typ, VNFunc func, ValueN
// (x - 0) == x
// (x - x) == 0
// This identity does not apply for floating point (when x == -0.0).
auto identityForSubtraction = [=]() -> ValueNum {
auto identityForSubtraction = [=](bool ovf) -> ValueNum {
if (!varTypeIsFloating(typ))
{
ValueNum ZeroVN = VNZeroForType(typ);
Expand All @@ -4462,6 +4462,39 @@ ValueNum ValueNumStore::EvalUsingMathIdentity(var_types typ, VNFunc func, ValueN
{
return ZeroVN;
}

if (!ovf)
{
// (x + a) - x == a
// (a + x) - x == a
VNFuncApp add;
if (GetVNFunc(arg0VN, &add) && (add.m_func == (VNFunc)GT_ADD))
{
if (add.m_args[0] == arg1VN)
return add.m_args[1];
if (add.m_args[1] == arg1VN)
return add.m_args[0];

// (x + a) - (x + b) == a - b
// (a + x) - (x + b) == a - b
// (x + a) - (b + x) == a - b
// (a + x) - (b + x) == a - b
VNFuncApp add2;
if (GetVNFunc(arg1VN, &add2) && (add2.m_func == (VNFunc)GT_ADD))
{
for (int a = 0; a < 2; a++)
{
for (int b = 0; b < 2; b++)
{
if (add.m_args[a] == add2.m_args[b])
{
return VNForFunc(typ, (VNFunc)GT_SUB, add.m_args[1 - a], add2.m_args[1 - b]);
}
}
}
}
}
}
}

return NoVN;
Expand Down Expand Up @@ -4515,7 +4548,7 @@ ValueNum ValueNumStore::EvalUsingMathIdentity(var_types typ, VNFunc func, ValueN
break;

case GT_SUB:
resultVN = identityForSubtraction();
resultVN = identityForSubtraction(false);
Comment thread
jakobbotsch marked this conversation as resolved.
Outdated
break;

case GT_MUL:
Expand Down Expand Up @@ -4790,7 +4823,7 @@ ValueNum ValueNumStore::EvalUsingMathIdentity(var_types typ, VNFunc func, ValueN

case VNF_SUB_OVF:
case VNF_SUB_UN_OVF:
resultVN = identityForSubtraction();
resultVN = identityForSubtraction(true);
Comment thread
jakobbotsch marked this conversation as resolved.
Outdated
break;

case VNF_MUL_OVF:
Expand Down