Skip to content

Avoid repeated SIMD mask proof traversal in value numbering #134530

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
tannergooding merged 2 commits into from
Sep 25, 2026
+186 −27
Merged

Avoid repeated SIMD mask proof traversal in value numbering #134530

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
1e7acbb
Bound and memoize vector mask value number proofs
tannergooding Sep 23, 2026
8670dcc
Reuse identical liberal and conservative intrinsic evaluations
tannergooding Sep 23, 2026
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
98 changes: 75 additions & 23 deletions src/coreclr/jit/valuenum.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9183,13 +9183,11 @@ ValueNum ValueNumStore::EvalHWIntrinsicFunBinary(
return VNZeroForType(type);
}
}
else if (IsVectorPerElementMask(argVN, baseType, simdSize))
else if (cnsVN == VNAllBitsForType(type, simdSize))
Comment thread
tannergooding marked this conversation as resolved.
{
// Handle `Equals(PerElementMask, AllBitsSet)` and `Equals(AllBitsSet, PerElementMask)` for
// integrals
ValueNum allBitsVN = VNAllBitsForType(type, simdSize);

if (cnsVN == allBitsVN)
if (IsVectorPerElementMask(argVN, baseType, simdSize))
{
// We are comparing something that is known per element to be either
// AllBitsSet or Zero, with AllBitsSet.
Expand Down Expand Up @@ -9361,12 +9359,10 @@ ValueNum ValueNumStore::EvalHWIntrinsicFunBinary(
return VNAllBitsForType(type, elementCount);
}
}
else if (IsVectorPerElementMask(argVN, baseType, simdSize))
else if (cnsVN == VNZeroForType(type))
{
// Handle `(Mask != Zero) == Mask` and `(Zero != Mask) == Mask` for integral types
ValueNum zeroVN = VNZeroForType(type);

if (cnsVN == zeroVN)
if (IsVectorPerElementMask(argVN, baseType, simdSize))
{
// We are comparing something that is known per element to be either
// AllBitsSet or Zero, with Zero.
Expand Down Expand Up @@ -9979,22 +9975,42 @@ ValueNum ValueNumStore::EvalHWIntrinsicFunTernary(
}

//-------------------------------------------------------------------
// IsVectorPerElementMask: returns true if the ValueNum is a vector constant per-element mask
// IsVectorPerElementMask: returns true if the ValueNum is a vector per-element mask
// (every element has either all bits set or none of them) for the
// given simd size and base type.
//
// Arguments:
// vn - the value number to check
// simdBaseType - the base type of the constant being checked.
// simdBaseType - the base type being checked.
// simdSize - the size of the SIMD type of the intrinsic.
//
// Returns:
// True if vn is a per-element mask compatible with simdBaseType and simdSize
//
bool ValueNumStore::IsVectorPerElementMask(ValueNum vn, var_types simdBaseType, unsigned simdSize)
{
SmallValueNumSet knownMasks;
return IsVectorPerElementMask(vn, simdBaseType, simdSize, knownMasks, 0);
}

// Cache successful compound proofs for this query only: mask validity depends on the requested element size.
// As in scalar evolution analysis, limit recursion to 64 levels to bound native stack usage.
bool ValueNumStore::IsVectorPerElementMask(
ValueNum vn, var_types simdBaseType, unsigned simdSize, SmallValueNumSet& knownMasks, unsigned depth)
Comment thread
tannergooding marked this conversation as resolved.
{
// This should be kept in sync with GenTree::IsVectorPerElementMask

if (knownMasks.Lookup(vn))
{
return true;
}

const unsigned MaxDepth = 64;
if (depth >= MaxDepth)
{
return false;
}

var_types simdType = TypeOfVN(vn);
unsigned elementCount = GenTreeVecCon::ElementCount(simdSize, simdBaseType);

Expand Down Expand Up @@ -10053,6 +10069,8 @@ bool ValueNumStore::IsVectorPerElementMask(ValueNum vn, var_types simdBaseType,
}
#endif // TARGET_ARM64

bool isMask = false;

switch (oper)
{
case GT_AND:
Expand All @@ -10068,14 +10086,16 @@ bool ValueNumStore::IsVectorPerElementMask(ValueNum vn, var_types simdBaseType,
// there isn't any way to statically determine this for non-constants and
// the constant cases should've already been folded.

return IsVectorPerElementMask(funcApp.GetArg(0), simdBaseType, simdSize) &&
IsVectorPerElementMask(funcApp.GetArg(1), simdBaseType, simdSize);
isMask = IsVectorPerElementMask(funcApp.GetArg(0), simdBaseType, simdSize, knownMasks, depth + 1) &&
IsVectorPerElementMask(funcApp.GetArg(1), simdBaseType, simdSize, knownMasks, depth + 1);
break;
}

case GT_NOT:
{
// We are an unary bitwise operation where the input is a per-element mask
return IsVectorPerElementMask(funcApp.GetArg(0), simdBaseType, simdSize);
isMask = IsVectorPerElementMask(funcApp.GetArg(0), simdBaseType, simdSize, knownMasks, depth + 1);
break;
}

default:
Expand All @@ -10085,7 +10105,12 @@ bool ValueNumStore::IsVectorPerElementMask(ValueNum vn, var_types simdBaseType,
}
}

return false;
if (isMask)
{
knownMasks.Add(m_compiler, vn);
}

return isMask;
}

#endif // FEATURE_HW_INTRINSICS
Expand Down Expand Up @@ -14234,8 +14259,17 @@ void Compiler::fgValueNumberHWIntrinsic(GenTreeHWIntrinsic* tree)
{
ValueNum normalLVN =
vnStore->EvalHWIntrinsicFunUnary(tree, func, op1vnp.GetLiberal(), resultTypeVNPair.GetLiberal());
ValueNum normalCVN = vnStore->EvalHWIntrinsicFunUnary(tree, func, op1vnp.GetConservative(),
resultTypeVNPair.GetConservative());
ValueNum normalCVN;

if (op1vnp.BothEqual())
{
normalCVN = normalLVN;
}
else
{
normalCVN = vnStore->EvalHWIntrinsicFunUnary(tree, func, op1vnp.GetConservative(),
resultTypeVNPair.GetConservative());
}
Comment on lines -14237 to +14272

@tannergooding tannergooding Sep 23, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Finally, this is other redundant work we were doing, where most other VN functions have a similar BothEqual check to avoid such redundancy

@EgorBo EgorBo Sep 24, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

We do this everywhere else I think, feels weird to expand only for rarely used stuff. You already added the budget check, do we still need these verbose fast paths?

@tannergooding tannergooding Sep 25, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

We do this everywhere else I think, feels weird to expand only for rarely used stuff

Not sure what you mean? We pretty consistently use BothEqual() to avoid duplicating work and you can see it in all the VNPairFor* helpers, including VNPairForFunc. HWIntrinsics were the odd one out by doing the work twice instead.


normalPair = ValueNumPair(normalLVN, normalCVN);
excSetPair = op1Xvnp;
Expand All @@ -14251,9 +14285,18 @@ void Compiler::fgValueNumberHWIntrinsic(GenTreeHWIntrinsic* tree)
ValueNum normalLVN =
vnStore->EvalHWIntrinsicFunBinary(tree, func, op1vnp.GetLiberal(), op2vnp.GetLiberal(),
resultTypeVNPair.GetLiberal());
ValueNum normalCVN =
vnStore->EvalHWIntrinsicFunBinary(tree, func, op1vnp.GetConservative(),
op2vnp.GetConservative(), resultTypeVNPair.GetConservative());
ValueNum normalCVN;

if (op1vnp.BothEqual() && op2vnp.BothEqual())
{
normalCVN = normalLVN;
}
else
{
normalCVN = vnStore->EvalHWIntrinsicFunBinary(tree, func, op1vnp.GetConservative(),
op2vnp.GetConservative(),
resultTypeVNPair.GetConservative());
}

normalPair = ValueNumPair(normalLVN, normalCVN);
excSetPair = vnStore->VNPExcSetUnion(op1Xvnp, op2Xvnp);
Expand All @@ -14269,10 +14312,19 @@ void Compiler::fgValueNumberHWIntrinsic(GenTreeHWIntrinsic* tree)
ValueNum normalLVN =
vnStore->EvalHWIntrinsicFunTernary(tree, func, op1vnp.GetLiberal(), op2vnp.GetLiberal(),
op3vnp.GetLiberal(), resultTypeVNPair.GetLiberal());
ValueNum normalCVN =
vnStore->EvalHWIntrinsicFunTernary(tree, func, op1vnp.GetConservative(),
op2vnp.GetConservative(), op3vnp.GetConservative(),
resultTypeVNPair.GetConservative());
ValueNum normalCVN;

if (op1vnp.BothEqual() && op2vnp.BothEqual() && op3vnp.BothEqual())
{
normalCVN = normalLVN;
}
else
{
normalCVN =
vnStore->EvalHWIntrinsicFunTernary(tree, func, op1vnp.GetConservative(),
op2vnp.GetConservative(), op3vnp.GetConservative(),
resultTypeVNPair.GetConservative());
}

normalPair = ValueNumPair(normalLVN, normalCVN);

Expand Down
35 changes: 31 additions & 4 deletions src/coreclr/jit/valuenum.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1400,14 +1400,36 @@ class ValueNumStore

ValueNumPair EvalMathFuncUnary(var_types typ, NamedIntrinsic mthFunc, ValueNumPair arg0VNP)
{
return ValueNumPair(EvalMathFuncUnary(typ, mthFunc, arg0VNP.GetLiberal()),
EvalMathFuncUnary(typ, mthFunc, arg0VNP.GetConservative()));
ValueNum liberalFuncVN = EvalMathFuncUnary(typ, mthFunc, arg0VNP.GetLiberal());
ValueNum conservativeFuncVN;

if (arg0VNP.BothEqual())
{
conservativeFuncVN = liberalFuncVN;
}
else
{
conservativeFuncVN = EvalMathFuncUnary(typ, mthFunc, arg0VNP.GetConservative());
}

return ValueNumPair(liberalFuncVN, conservativeFuncVN);
}

ValueNumPair EvalMathFuncBinary(var_types typ, NamedIntrinsic mthFunc, ValueNumPair arg0VNP, ValueNumPair arg1VNP)
{
return ValueNumPair(EvalMathFuncBinary(typ, mthFunc, arg0VNP.GetLiberal(), arg1VNP.GetLiberal()),
EvalMathFuncBinary(typ, mthFunc, arg0VNP.GetConservative(), arg1VNP.GetConservative()));
ValueNum liberalFuncVN = EvalMathFuncBinary(typ, mthFunc, arg0VNP.GetLiberal(), arg1VNP.GetLiberal());
ValueNum conservativeFuncVN;

if (arg0VNP.BothEqual() && arg1VNP.BothEqual())
{
conservativeFuncVN = liberalFuncVN;
}
else
{
conservativeFuncVN = EvalMathFuncBinary(typ, mthFunc, arg0VNP.GetConservative(), arg1VNP.GetConservative());
}

return ValueNumPair(liberalFuncVN, conservativeFuncVN);
}

#if defined(FEATURE_HW_INTRINSICS)
Expand Down Expand Up @@ -1545,6 +1567,11 @@ class ValueNumStore
static bool isReservedVN(ValueNum);

private:
#if defined(FEATURE_HW_INTRINSICS)
bool IsVectorPerElementMask(
ValueNum vn, var_types simdBaseType, unsigned simdSize, SmallValueNumSet& knownMasks, unsigned depth);
#endif // FEATURE_HW_INTRINSICS

struct VNDefFuncAppFlexible
{
VNFunc m_func;
Expand Down
68 changes: 68 additions & 0 deletions src/tests/JIT/Regression_2/Runtime_134487/Runtime_134487.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,68 @@
// 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;
using System.Runtime.Intrinsics;
using Xunit;

public class Runtime_134487
{
[Fact]
public static void TestEntryPoint()
{
Assert.Equal(Vector128.Create(-1, 0, -1, 0),
MaskChain(Vector128.Create(1, 2, 3, 4), Vector128.Create(1, 0, 3, 0)));
}

[MethodImpl(MethodImplOptions.NoInlining | MethodImplOptions.AggressiveOptimization)]
private static Vector128<int> MaskChain(Vector128<int> a, Vector128<int> b)
{
Vector128<int> m = Vector128.Equals(a, b);
Vector128<int> k = Vector128.GreaterThan(a, b);

// Each step shares the previous mask VN along two paths. Keep the chain unrolled so
// the mask query encounters a DAG rather than a loop phi.
m ^= m & k;
m ^= m & k;
m ^= m & k;
m ^= m & k;
m ^= m & k;
m ^= m & k;
m ^= m & k;
m ^= m & k;
m ^= m & k;
m ^= m & k;
m ^= m & k;
m ^= m & k;
m ^= m & k;
m ^= m & k;
m ^= m & k;
m ^= m & k;
m ^= m & k;
m ^= m & k;
m ^= m & k;
m ^= m & k;
m ^= m & k;
m ^= m & k;
m ^= m & k;
m ^= m & k;
m ^= m & k;
m ^= m & k;
m ^= m & k;
m ^= m & k;
m ^= m & k;
m ^= m & k;
m ^= m & k;
m ^= m & k;
m ^= m & k;
m ^= m & k;
m ^= m & k;
m ^= m & k;
m ^= m & k;
m ^= m & k;
m ^= m & k;
m ^= m & k;

return Vector128.Equals(m, Vector128<int>.AllBitsSet);
}
}
12 changes: 12 additions & 0 deletions src/tests/JIT/Regression_2/Runtime_134487/Runtime_134487.csproj
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
<Project Sdk="Microsoft.NET.Sdk">
<PropertyGroup>
<Optimize>True</Optimize>
<DebugType>None</DebugType>
<CLRTestPriority>1</CLRTestPriority>
<RequiresProcessIsolation>true</RequiresProcessIsolation>
</PropertyGroup>
<ItemGroup>
<Compile Include="$(MSBuildProjectName).cs" />
<CLRTestEnvironmentVariable Include="DOTNET_EnableAVX512" Value="0" />
Comment thread
tannergooding marked this conversation as resolved.
</ItemGroup>
</Project>
Loading