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.

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: broaden cloning invariant checks #70232

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
70 changes: 18 additions & 52 deletions src/coreclr/jit/optimizer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -5958,60 +5958,26 @@ bool Compiler::optIsSetAssgLoop(unsigned lnum, ALLVARSET_VALARG_TP vars, varRefK
return true;
}

switch (loop->lpAsgCall)
// If caller is worried about possible indirect effects, check
// what we know about the calls in the loop.
//
if (inds != 0)
Comment on lines +5961 to +5964
Copy link
Contributor

Choose a reason for hiding this comment

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

Are any of the callers worried?

(I do not see us pass anything but VR_NONE for inds).

Copy link
Member Author

Choose a reason for hiding this comment

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

You're right, nobody actually cares right now. And there are other parts of the analysis that are likewise unused.

I'm deliberately leaving it for now. It's possible we'll run across cases where we want to try and use it for something.

{
case CALLINT_ALL:

/* Can't hoist if the call might have side effect on an indirection. */

if (loop->lpAsgInds != VR_NONE)
{
return true;
}

break;

case CALLINT_REF_INDIRS:

/* Can't hoist if the call might have side effect on an ref indirection. */

if (loop->lpAsgInds & VR_IND_REF)
{
return true;
}

break;

case CALLINT_SCL_INDIRS:

/* Can't hoist if the call might have side effect on an non-ref indirection. */

if (loop->lpAsgInds & VR_IND_SCL)
{
return true;
}

break;

case CALLINT_ALL_INDIRS:

/* Can't hoist if the call might have side effect on any indirection. */

if (loop->lpAsgInds & (VR_IND_REF | VR_IND_SCL))
{
switch (loop->lpAsgCall)
{
case CALLINT_ALL:
return true;
}

break;

case CALLINT_NONE:

/* Other helpers kill nothing */

break;

default:
noway_assert(!"Unexpected lpAsgCall value");
case CALLINT_REF_INDIRS:
return (inds & VR_IND_REF) != 0;
case CALLINT_SCL_INDIRS:
return (inds & VR_IND_SCL) != 0;
case CALLINT_ALL_INDIRS:
return (inds & (VR_IND_REF | VR_IND_SCL)) != 0;
case CALLINT_NONE:
return false;
default:
noway_assert(!"Unexpected lpAsgCall value");
}
}

return false;
Expand Down
5 changes: 3 additions & 2 deletions src/coreclr/jit/varset.h
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,7 @@ typedef BitSetOpsWithCounter<VARSET_TP,
typedef VarSetOpsRaw VarSetOps;
#endif

#define ALLVARSET_REP BSUInt64
#define ALLVARSET_REP BSShortLong

#if ALLVARSET_REP == BSUInt64

Expand Down Expand Up @@ -141,7 +141,8 @@ typedef BitSetOps</*BitSetType*/ BitSetShortLongRep,

typedef BitSetShortLongRep ALLVARSET_TP;

const unsigned lclMAX_ALLSET_TRACKED = lclMAX_TRACKED;
// default value for JitConfig.JitMaxLocalsToTrack()
const unsigned lclMAX_ALLSET_TRACKED = 0x400;

#define ALLVARSET_REP_IS_CLASS 0

Expand Down
54 changes: 54 additions & 0 deletions src/tests/JIT/opt/Cloning/callandindir.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,54 @@
// 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;

// Loops in F, G, H should all clone

class CallAndIndir
{
[MethodImpl(MethodImplOptions.NoInlining)]
public static void S() { }

[MethodImpl(MethodImplOptions.NoInlining)]
public static void F(int[] a, int low, int high, ref int z)
{
for (int i = low; i < high; i++)
{
z += a[i];
S();
}
}

[MethodImpl(MethodImplOptions.NoInlining)]
public static void G(int[] a, int low, int high, ref int z)
{
for (int i = low; i < high; i++)
{
z += a[i];
}
}

[MethodImpl(MethodImplOptions.NoInlining)]
public static void H(int[] a, int low, int high, ref int z)
{
int r = 0;
for (int i = low; i < high; i++)
{
r += a[i];
S();
}
z += r;
}

public static int Main()
{
int[] a = new int[] { 1, 2, 3, 4 };
int z = 0;
F(a, 2, 4, ref z);
G(a, 2, 4, ref z);
H(a, 2, 4, ref z);
return z + 79;
}
}
9 changes: 9 additions & 0 deletions src/tests/JIT/opt/Cloning/callandindir.csproj
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
<Project Sdk="Microsoft.NET.Sdk">
<PropertyGroup>
<OutputType>Exe</OutputType>
<Optimize>True</Optimize>
</PropertyGroup>
<ItemGroup>
<Compile Include="$(MSBuildProjectName).cs" />
</ItemGroup>
</Project>