Skip to content

Commit

Permalink
Remove the "anything + null => null" optimization (#61518)
Browse files Browse the repository at this point in the history
This optimization is only legal if:
1) "Anything" is a sufficiently small constant itself.
2) We are in a context where we know the address will in
   fact be used for an indirection.

It is the second point that is problematic - one would
like to use MorphAddrContext, but it is not suitable
for this purpose, as an unknown context is counted as
an indirecting one. Additionally, the value of this
optimization is rather low. I am guessing it was meant
to support the legacy nullchecks, before GT_NULLCHECK
was introduced, and had higher impact then.

So, just remove the optimization and leave the 5 small
regressions across all of SPMI be.
  • Loading branch information
SingleAccretion authored Nov 15, 2021
1 parent 4cf86c2 commit 3d7e89c
Show file tree
Hide file tree
Showing 3 changed files with 33 additions and 18 deletions.
18 changes: 0 additions & 18 deletions src/coreclr/jit/morph.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -12401,27 +12401,9 @@ GenTree* Compiler::fgMorphSmpOp(GenTree* tree, MorphAddrContext* mac)

if (op2->IsCnsIntOrI() && varTypeIsIntegralOrI(typ))
{
CLANG_FORMAT_COMMENT_ANCHOR;

// Fold (x + 0).

if ((op2->AsIntConCommon()->IconValue() == 0) && !gtIsActiveCSE_Candidate(tree))
{

// If this addition is adding an offset to a null pointer,
// avoid the work and yield the null pointer immediately.
// Dereferencing the pointer in either case will have the
// same effect.

if (!optValnumCSE_phase && varTypeIsGC(op2->TypeGet()) &&
((op1->gtFlags & GTF_ALL_EFFECT) == 0))
{
op2->gtType = tree->gtType;
DEBUG_DESTROY_NODE(op1);
DEBUG_DESTROY_NODE(tree);
return op2;
}

// Remove the addition iff it won't change the tree type
// to TYP_REF.

Expand Down
23 changes: 23 additions & 0 deletions src/tests/JIT/Regression/JitBlue/Runtime_61510/Runtime_61510.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.

using System.Runtime.CompilerServices;

unsafe class Runtime_61510
{
[FixedAddressValueType]
private static byte s_field;

public static int Main()
{
ref byte result = ref AddZeroByrefToNativeInt((nint)Unsafe.AsPointer(ref s_field));

return Unsafe.AreSame(ref s_field, ref result) ? 100 : 101;
}

[MethodImpl(MethodImplOptions.NoInlining)]
private static ref byte AddZeroByrefToNativeInt(nint addr)
{
return ref Unsafe.Add(ref Unsafe.NullRef<byte>(), addr);
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
<Project Sdk="Microsoft.NET.Sdk">
<PropertyGroup>
<OutputType>Exe</OutputType>
<Optimize>True</Optimize>
<AllowUnsafeBlocks>true</AllowUnsafeBlocks>
</PropertyGroup>
<ItemGroup>
<Compile Include="$(MSBuildProjectName).cs" />
</ItemGroup>
</Project>

0 comments on commit 3d7e89c

Please sign in to comment.