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

Morph Vector.Create(0) to Vector.Zero #63821

Merged
merged 68 commits into from
Feb 17, 2022
Merged
Show file tree
Hide file tree
Changes from 67 commits
Commits
Show all changes
68 commits
Select commit Hold shift + click to select a range
7aca5dc
Initial work
TIHan Dec 16, 2021
2752913
Added a comma to display
TIHan Dec 16, 2021
526e6c8
Cleanup
TIHan Dec 16, 2021
d850426
Fixing build
TIHan Dec 17, 2021
0ee6450
More cleanup
TIHan Dec 17, 2021
cb5a82e
Update comment
TIHan Dec 17, 2021
6ae9a94
Update comment
TIHan Dec 17, 2021
5c1997f
Added CompareEqual Vector64/128 with Zero tests
TIHan Jan 4, 2022
b33c72c
Merge remote-tracking branch 'upstream/main' into vector-64-128-zero-…
TIHan Jan 4, 2022
fd19fdc
Do not contain op1 for now
TIHan Jan 4, 2022
178ff52
Wrong intrinsic id used
TIHan Jan 5, 2022
c0a23c0
Removing generated tests
TIHan Jan 6, 2022
ad780ea
Removing generated tests
TIHan Jan 6, 2022
83c26ab
Added CompareEqual tests
TIHan Jan 6, 2022
9f7e7da
Supporting containment for first operand
TIHan Jan 6, 2022
1e67415
Fix test build
TIHan Jan 6, 2022
25b9d92
Passing correct register
TIHan Jan 6, 2022
c0dc7e4
Check IsVectorZero before not allocing a register
TIHan Jan 7, 2022
a718944
Update comment
TIHan Jan 7, 2022
1f45fa2
Fixing test
TIHan Jan 7, 2022
cb872da
Minor format change
TIHan Jan 7, 2022
c0708d7
Fixed formatting
TIHan Jan 7, 2022
bf49d11
Renamed test
TIHan Jan 8, 2022
bc7a557
Adding AdvSimd_Arm64 tests:
TIHan Jan 10, 2022
5939f36
Adding support for rest of 'cmeq' and 'fcmeq' instructions
TIHan Jan 10, 2022
6cd0ea8
Removing github csproj
TIHan Jan 10, 2022
2b30421
Minor test fix
TIHan Jan 10, 2022
0828de6
Fixed tests
TIHan Jan 10, 2022
fa43d19
Fix print
TIHan Jan 10, 2022
911f929
Minor format change
TIHan Jan 10, 2022
b91be1e
Fixing test
TIHan Jan 11, 2022
760a08c
Initial commit for Vector.Create to Vector.Zero normalization
TIHan Jan 14, 2022
c1a90b4
Added some emitter tests
TIHan Jan 18, 2022
32f86c1
Feedback
TIHan Jan 19, 2022
b08f552
Update emitarm64.cpp
TIHan Jan 19, 2022
c89e47b
Feedback
TIHan Jan 19, 2022
956e50a
Merge branch 'vector-64-128-zero-arm64-opts1' of github.com:TIHan/run…
TIHan Jan 19, 2022
2b526c9
Merge remote-tracking branch 'upstream' into vector-64-128-zero-arm64…
TIHan Jan 19, 2022
d51a988
Merge branch 'vector-64-128-zero-arm64-opts1' into vector-64-128-256-…
TIHan Jan 19, 2022
77c3d25
Merge remote-tracking branch 'upstream' into vector-64-128-256-Create…
TIHan Jan 20, 2022
b1065e8
Handling variations of Vector.Create
TIHan Jan 20, 2022
451f8e3
Use Operands iterator instead of edges
TIHan Jan 20, 2022
e231131
Fix condition
TIHan Jan 20, 2022
64ad95f
Simplify
TIHan Jan 20, 2022
333fc67
format
TIHan Jan 20, 2022
bc02904
Fixed IsFloatPositiveZero
TIHan Jan 21, 2022
d8c39e3
Uncomment
TIHan Jan 21, 2022
a5e51d5
Merging
TIHan Jan 21, 2022
38d9e7e
Updated tests to include Vector64.Create/Vector128.Create for ARM64
TIHan Jan 21, 2022
fb6047c
Making implementation of IsFloatPositiveZero explicit
TIHan Jan 21, 2022
bc1b9f4
Update src/coreclr/jit/gentree.cpp
TIHan Jan 24, 2022
d7904b6
Feedback
TIHan Jan 24, 2022
dbe1990
Merged
TIHan Jan 24, 2022
5b7d991
Update comment
TIHan Jan 24, 2022
5a7f674
Update comment
TIHan Jan 24, 2022
377b794
Do not perform optimization when VN CSE phase
TIHan Jan 24, 2022
1cf0b32
use ResetHWIntrinsicId
TIHan Jan 25, 2022
84f51cd
Assert !optValnumCSE_phase
TIHan Jan 25, 2022
31cd50d
Simplify IsVectorZero
TIHan Jan 25, 2022
383f147
Simplify IsVectorZero
TIHan Jan 25, 2022
29fb977
Simplify some uses of Vector*_get_Zero
TIHan Jan 25, 2022
51eae5a
Added another test
TIHan Jan 25, 2022
7d06ebf
Fixed formatting
TIHan Jan 25, 2022
177cd53
Revert lowering removal
TIHan Jan 25, 2022
32ac1fc
Merge remote-tracking branch 'upstream/main' into vector-64-128-256-C…
TIHan Feb 7, 2022
36c4001
Update gentree.h
TIHan Feb 8, 2022
759e8c0
Feedback
TIHan Feb 16, 2022
4c0f768
Merge remote-tracking branch 'upstream/main' into vector-64-128-256-C…
TIHan Feb 17, 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
3 changes: 3 additions & 0 deletions src/coreclr/jit/compiler.h
Original file line number Diff line number Diff line change
Expand Up @@ -6410,6 +6410,9 @@ class Compiler
GenTree* fgOptimizeCast(GenTreeCast* cast);
GenTree* fgOptimizeEqualityComparisonWithConst(GenTreeOp* cmp);
GenTree* fgOptimizeRelationalComparisonWithConst(GenTreeOp* cmp);
#ifdef FEATURE_HW_INTRINSICS
GenTree* fgOptimizeHWIntrinsic(GenTreeHWIntrinsic* node);
#endif
GenTree* fgOptimizeCommutativeArithmetic(GenTreeOp* tree);
GenTree* fgOptimizeAddition(GenTreeOp* add);
GenTree* fgOptimizeMultiply(GenTreeOp* mul);
Expand Down
54 changes: 16 additions & 38 deletions src/coreclr/jit/gentree.h
Original file line number Diff line number Diff line change
Expand Up @@ -7651,16 +7651,23 @@ inline bool GenTree::IsSIMDZero() const
//
inline bool GenTree::IsFloatPositiveZero() const
{
return IsCnsFltOrDbl() && !IsCnsNonZeroFltOrDbl();
if (IsCnsFltOrDbl())
Copy link
Member

Choose a reason for hiding this comment

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

why is this change? Seems IsCnsNonZeroFltOrDbl already does this?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It does, but we decided that !IsCnsNonZeroFltOrDbl() is a bit confusing; an explicit implementation seemed to be clearer.

Copy link
Member

Choose a reason for hiding this comment

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

Perhaps add a comment explaining why you're not using !IsCns ...

{
// This implementation is almost identical to IsCnsNonZeroFltOrDbl
// but it is easier to parse out
// rather than using !IsCnsNonZeroFltOrDbl.
double constValue = AsDblCon()->gtDconVal;
return *(__int64*)&constValue == 0;
}

return false;
}

//-------------------------------------------------------------------
// IsVectorZero: returns true if this is an integral or floating-point (SIMD or HW intrinsic) vector
// with all its elements equal to zero.
// IsVectorZero: returns true if this node is a HWIntrinsic that is Vector*_get_Zero.
//
// Returns:
// True if this represents an integral or floating-point const (SIMD or HW intrinsic) vector with all its elements
// equal to zero.
// True if this represents a HWIntrinsic node that is Vector*_get_Zero.
//
// TODO: We already have IsSIMDZero() and IsIntegralConstVector(0),
// however, IsSIMDZero() does not cover hardware intrinsics, and IsIntegralConstVector(0) does not cover floating
Expand All @@ -7669,46 +7676,17 @@ inline bool GenTree::IsFloatPositiveZero() const
// separate ones; preferably this one.
inline bool GenTree::IsVectorZero() const
{
#ifdef FEATURE_SIMD
if (gtOper == GT_SIMD)
{
const GenTreeSIMD* node = AsSIMD();

if (node->GetSIMDIntrinsicId() == SIMDIntrinsicInit)
{
return (node->Op(1)->IsIntegralConst(0) || node->Op(1)->IsFloatPositiveZero());
}
}
#endif

#ifdef FEATURE_HW_INTRINSICS
if (gtOper == GT_HWINTRINSIC)
{
const GenTreeHWIntrinsic* node = AsHWIntrinsic();
const var_types simdBaseType = node->GetSimdBaseType();

if (varTypeIsIntegral(simdBaseType) || varTypeIsFloating(simdBaseType))
{
const NamedIntrinsic intrinsicId = node->GetHWIntrinsicId();
const GenTreeHWIntrinsic* node = AsHWIntrinsic();
const NamedIntrinsic intrinsicId = node->GetHWIntrinsicId();

if (node->GetOperandCount() == 0)
{
#if defined(TARGET_XARCH)
return (intrinsicId == NI_Vector128_get_Zero) || (intrinsicId == NI_Vector256_get_Zero);
#elif defined(TARGET_ARM64)
return (intrinsicId == NI_Vector64_get_Zero) || (intrinsicId == NI_Vector128_get_Zero);
#endif // !TARGET_XARCH && !TARGET_ARM64
}
else if ((node->GetOperandCount() == 1) &&
(node->Op(1)->IsIntegralConst(0) || node->Op(1)->IsFloatPositiveZero()))
tannergooding marked this conversation as resolved.
Show resolved Hide resolved
{
#if defined(TARGET_XARCH)
return (intrinsicId == NI_Vector128_Create) || (intrinsicId == NI_Vector256_Create);
return (intrinsicId == NI_Vector128_get_Zero) || (intrinsicId == NI_Vector256_get_Zero);
#elif defined(TARGET_ARM64)
return (intrinsicId == NI_Vector64_Create) || (intrinsicId == NI_Vector128_Create);
return (intrinsicId == NI_Vector64_get_Zero) || (intrinsicId == NI_Vector128_get_Zero);
#endif // !TARGET_XARCH && !TARGET_ARM64
}
}
}
#endif // FEATURE_HW_INTRINSICS

Expand Down
90 changes: 90 additions & 0 deletions src/coreclr/jit/morph.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -13433,6 +13433,89 @@ GenTree* Compiler::fgOptimizeRelationalComparisonWithConst(GenTreeOp* cmp)
return cmp;
}

#ifdef FEATURE_HW_INTRINSICS

//------------------------------------------------------------------------
// fgOptimizeHWIntrinsic: optimize a HW intrinsic node
//
// Arguments:
// node - HWIntrinsic node to examine
//
// Returns:
// The original node if no optimization happened or if tree bashing occured.
// An alternative tree if an optimization happened.
//
// Notes:
// Checks for HWIntrinsic nodes: Vector64.Create/Vector128.Create/Vector256.Create,
// and if the call is one of these, attempt to optimize.
// This is post-order, meaning that it will not morph the children.
//
GenTree* Compiler::fgOptimizeHWIntrinsic(GenTreeHWIntrinsic* node)
{
assert(!optValnumCSE_phase);

if (opts.OptimizationDisabled())
{
return node;
}

switch (node->GetHWIntrinsicId())
{
case NI_Vector128_Create:
#if defined(TARGET_XARCH)
case NI_Vector256_Create:
#elif defined(TARGET_ARM64)
case NI_Vector64_Create:
#endif
{
bool hwAllArgsAreConstZero = true;
for (GenTree* arg : node->Operands())
{
if (!arg->IsIntegralConst(0) && !arg->IsFloatPositiveZero())
{
hwAllArgsAreConstZero = false;
break;
}
}

if (hwAllArgsAreConstZero)
{
switch (node->GetHWIntrinsicId())
{
case NI_Vector128_Create:
{
node->ResetHWIntrinsicId(NI_Vector128_get_Zero);
break;
}
#if defined(TARGET_XARCH)
case NI_Vector256_Create:
{
node->ResetHWIntrinsicId(NI_Vector256_get_Zero);
break;
}
#elif defined(TARGET_ARM64)
case NI_Vector64_Create:
{
node->ResetHWIntrinsicId(NI_Vector64_get_Zero);
break;
}
#endif
default:
unreached();
}
}
break;
}

default:
break;
}

return node;
}

#endif

//------------------------------------------------------------------------
// fgOptimizeCommutativeArithmetic: Optimizes commutative operations.
//
Expand Down Expand Up @@ -14349,6 +14432,13 @@ GenTree* Compiler::fgMorphMultiOp(GenTreeMultiOp* multiOp)
}
#endif // defined(FEATURE_HW_INTRINSICS) && defined(TARGET_XARCH)

#ifdef FEATURE_HW_INTRINSICS
if (multiOp->OperIsHWIntrinsic() && !optValnumCSE_phase)
Copy link
Member

Choose a reason for hiding this comment

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

why is !optValnumCSE_phase check needed?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It was added because I was deleting nodes; however, I'm not deleting the nodes anymore and instead using ResetHWIntrinsic. I do feel safer having that guard there in-case more optimizations get added in fgOptimizeHWIntrinsic.

Read more in the discussion: #63821 (comment)

{
return fgOptimizeHWIntrinsic(multiOp->AsHWIntrinsic());
}
#endif

return multiOp;
}
#endif // defined(FEATURE_SIMD) || defined(FEATURE_HW_INTRINSICS)
Expand Down
102 changes: 102 additions & 0 deletions src/tests/JIT/Regression/JitBlue/Runtime_33972/Runtime_33972.cs
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,30 @@ static Vector64<float> AdvSimd_CompareEqual_Vector64_Single_Zero(Vector64<float>
return AdvSimd.CompareEqual(left, Vector64<float>.Zero);
}

[MethodImpl(MethodImplOptions.NoInlining)]
static Vector64<int> AdvSimd_CompareEqual_Vector64_Int32_CreateZero(Vector64<int> left)
{
return AdvSimd.CompareEqual(left, Vector64.Create(0));
}

[MethodImpl(MethodImplOptions.NoInlining)]
static Vector64<int> AdvSimd_CompareEqual_Vector64_Int32_CreateZeroZero(Vector64<int> left)
{
return AdvSimd.CompareEqual(left, Vector64.Create(0, 0));
}

[MethodImpl(MethodImplOptions.NoInlining)]
static Vector64<float> AdvSimd_CompareEqual_Vector64_Single_CreateZero(Vector64<float> left)
{
return AdvSimd.CompareEqual(left, Vector64.Create(0f));
}

[MethodImpl(MethodImplOptions.NoInlining)]
static Vector64<float> AdvSimd_CompareEqual_Vector64_Single_CreateZeroZero(Vector64<float> left)
{
return AdvSimd.CompareEqual(left, Vector64.Create(0f, 0f));
}

[MethodImpl(MethodImplOptions.NoInlining)]
static Vector128<byte> AdvSimd_CompareEqual_Vector128_Byte_Zero(Vector128<byte> left)
{
Expand Down Expand Up @@ -94,6 +118,37 @@ static Vector128<float> AdvSimd_CompareEqual_Vector128_Single_Zero(Vector128<flo
return AdvSimd.CompareEqual(left, Vector128<float>.Zero);
}

[MethodImpl(MethodImplOptions.NoInlining)]
static Vector128<int> AdvSimd_CompareEqual_Vector128_Int32_CreateZero(Vector128<int> left)
{
return AdvSimd.CompareEqual(left, Vector128.Create(0));
}

[MethodImpl(MethodImplOptions.NoInlining)]
static Vector128<int> AdvSimd_CompareEqual_Vector128_Int32_CreateZeroZeroZeroZero(Vector128<int> left)
{
return AdvSimd.CompareEqual(left, Vector128.Create(0, 0, 0, 0));
}

[MethodImpl(MethodImplOptions.NoInlining)]
static Vector128<float> AdvSimd_CompareEqual_Vector128_Single_CreateZero(Vector128<float> left)
{
return AdvSimd.CompareEqual(left, Vector128.Create(0f));
}

[MethodImpl(MethodImplOptions.NoInlining)]
static Vector128<float> AdvSimd_CompareEqual_Vector128_Single_CreateZeroZeroZeroZero(Vector128<float> left)
{
return AdvSimd.CompareEqual(left, Vector128.Create(0f, 0f, 0f, 0f));
}

[MethodImpl(MethodImplOptions.NoInlining)]
static Vector128<float> AdvSimd_CompareEqual_Vector128_Single_CreateZeroZeroZeroZero_AsVariable(Vector128<float> left)
{
var asVar = Vector128.Create(0f, 0f, 0f, 0f);
return AdvSimd.CompareEqual(left, asVar);
}

[MethodImpl(MethodImplOptions.NoInlining)]
static Vector128<double> AdvSimd_Arm64_CompareEqual_Vector128_Double_Zero(Vector128<double> left)
{
Expand Down Expand Up @@ -332,6 +387,8 @@ static int Tests_AdvSimd()

// Begin CompareEqual Tests

// Vector64

if (!ValidateResult_Vector64<byte>(AdvSimd_CompareEqual_Vector64_Byte_Zero(Vector64<byte>.Zero), Byte.MaxValue))
result = -1;

Expand All @@ -353,6 +410,22 @@ static int Tests_AdvSimd()
if (!ValidateResult_Vector64<float>(AdvSimd_CompareEqual_Vector64_Single_Zero(Vector64<float>.Zero), Single.NaN))
result = -1;

// Vector64.Create

if (!ValidateResult_Vector64<int>(AdvSimd_CompareEqual_Vector64_Int32_CreateZero(Vector64<int>.Zero), -1))
result = -1;

if (!ValidateResult_Vector64<float>(AdvSimd_CompareEqual_Vector64_Single_CreateZero(Vector64<float>.Zero), Single.NaN))
result = -1;

if (!ValidateResult_Vector64<int>(AdvSimd_CompareEqual_Vector64_Int32_CreateZeroZero(Vector64<int>.Zero), -1))
result = -1;

if (!ValidateResult_Vector64<float>(AdvSimd_CompareEqual_Vector64_Single_CreateZeroZero(Vector64<float>.Zero), Single.NaN))
result = -1;

// Vector128

if (!ValidateResult_Vector128<byte>(AdvSimd_CompareEqual_Vector128_Byte_Zero(Vector128<byte>.Zero), Byte.MaxValue))
result = -1;

Expand All @@ -374,6 +447,23 @@ static int Tests_AdvSimd()
if (!ValidateResult_Vector128<float>(AdvSimd_CompareEqual_Vector128_Single_Zero(Vector128<float>.Zero), Single.NaN))
result = -1;

// Vector128.Create

if (!ValidateResult_Vector128<int>(AdvSimd_CompareEqual_Vector128_Int32_CreateZero(Vector128<int>.Zero), -1))
result = -1;

if (!ValidateResult_Vector128<float>(AdvSimd_CompareEqual_Vector128_Single_CreateZero(Vector128<float>.Zero), Single.NaN))
result = -1;

if (!ValidateResult_Vector128<int>(AdvSimd_CompareEqual_Vector128_Int32_CreateZeroZeroZeroZero(Vector128<int>.Zero), -1))
result = -1;

if (!ValidateResult_Vector128<float>(AdvSimd_CompareEqual_Vector128_Single_CreateZeroZeroZeroZero(Vector128<float>.Zero), Single.NaN))
result = -1;

if (!ValidateResult_Vector128<float>(AdvSimd_CompareEqual_Vector128_Single_CreateZeroZeroZeroZero_AsVariable(Vector128<float>.Zero), Single.NaN))
result = -1;

// End CompareEqual Tests

return result;
Expand All @@ -385,6 +475,8 @@ static int Tests_AdvSimd_Swapped()

// Begin CompareEqual Tests

// Vector64

if (!ValidateResult_Vector64<byte>(AdvSimd_CompareEqual_Vector64_Byte_Zero_Swapped(Vector64<byte>.Zero), Byte.MaxValue))
result = -1;

Expand All @@ -406,6 +498,8 @@ static int Tests_AdvSimd_Swapped()
if (!ValidateResult_Vector64<float>(AdvSimd_CompareEqual_Vector64_Single_Zero_Swapped(Vector64<float>.Zero), Single.NaN))
result = -1;

// Vector128

if (!ValidateResult_Vector128<byte>(AdvSimd_CompareEqual_Vector128_Byte_Zero_Swapped(Vector128<byte>.Zero), Byte.MaxValue))
result = -1;

Expand Down Expand Up @@ -438,6 +532,8 @@ static int Tests_AdvSimd_Arm64()

// Begin CompareEqual Tests

// Vector128

if (!ValidateResult_Vector128<double>(AdvSimd_Arm64_CompareEqual_Vector128_Double_Zero(Vector128<double>.Zero), Double.NaN))
result = -1;

Expand All @@ -447,6 +543,8 @@ static int Tests_AdvSimd_Arm64()
if (!ValidateResult_Vector128<long>(AdvSimd_Arm64_CompareEqual_Vector128_Int64_Zero(Vector128<long>.Zero), -1))
result = -1;

// Vector64

if (!ValidateResult_Vector64<float>(AdvSimd_Arm64_CompareEqualScalar_Vector64_Single_Zero(Vector64<float>.Zero), Vector64.CreateScalar(Single.NaN)))
result = -1;

Expand All @@ -470,6 +568,8 @@ static int Tests_AdvSimd_Arm64_Swapped()

// Begin CompareEqual Tests

// Vector128

if (!ValidateResult_Vector128<double>(AdvSimd_Arm64_CompareEqual_Vector128_Double_Zero_Swapped(Vector128<double>.Zero), Double.NaN))
result = -1;

Expand All @@ -479,6 +579,8 @@ static int Tests_AdvSimd_Arm64_Swapped()
if (!ValidateResult_Vector128<long>(AdvSimd_Arm64_CompareEqual_Vector128_Int64_Zero_Swapped(Vector128<long>.Zero), -1))
result = -1;

// Vector64

if (!ValidateResult_Vector64<float>(AdvSimd_Arm64_CompareEqualScalar_Vector64_Single_Zero_Swapped(Vector64<float>.Zero), Vector64.CreateScalar(Single.NaN)))
result = -1;

Expand Down