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.

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

Jump to bottom

Expose cross-platform helpers for Vector64, Vector128, and Vector256 #53450

Merged
merged 22 commits into from
Sep 30, 2021
Merged

Expose cross-platform helpers for Vector64, Vector128, and Vector256 #53450

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
22 commits
Select commit Hold shift + click to select a range
df33419
Refactoring Vector<T> to be alphabetically ordered and reduce duplica…
tannergooding May 22, 2021
bcf2bbd
Adding a debug view to System.Numerics.Vector<T>
tannergooding May 22, 2021
7ed4a28
Do some basic code cleanup and refactoring of Vector64/128/256
tannergooding May 22, 2021
b577114
Updating Vector64/128/256 to expose cross platform helper methods
tannergooding May 23, 2021
4cdec92
Adding templated tests for Vector64
tannergooding May 23, 2021
2f9ce11
Regenerating templated tests for Vector64
tannergooding May 23, 2021
4940fcf
Adding templated tests for Vector128
tannergooding May 23, 2021
ac9c477
Regenerating templated tests for Vector128
tannergooding May 23, 2021
557128e
Adding templated tests for Vector256
tannergooding May 23, 2021
81a012e
Regenerating templated tests for Vector256
tannergooding May 23, 2021
3245883
Fix getSIMDStructFromField to account for accessing the private ulong…
tannergooding May 24, 2021
37c521e
Ensure helper intrinsics don't insert on importation
tannergooding May 24, 2021
6cddb02
Minor cleanup of impBaseIntrinsic for x86/x64
tannergooding May 25, 2021
8c28873
Intrinsify the Vector64/128/256 methods
tannergooding May 26, 2021
1b665fc
Make the internal helper named IsTypeSupported to not conflict with m…
tannergooding May 30, 2021
f917ea6
Ensure we lie about the type for TYP_SIMD32 bitwise ops when only AVX…
tannergooding May 31, 2021
10dfe0c
Use gtNewSimdZeroNode rather than gtNewSIMDVectorZero
tannergooding Jun 1, 2021
6d34d27
Applying formatting patch
tannergooding Jun 1, 2021
57ce0a9
Apply suggestions from code review
tannergooding Jun 29, 2021
f2529d7
Apply suggestions from code review
tannergooding Jun 30, 2021
54610f9
Split HardwareIntrinsic tests into 3 groups
tannergooding Jun 30, 2021
fc5c5bc
Update src/coreclr/vm/class.cpp
tannergooding Sep 30, 2021
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
Prev Previous commit
Next Next commit
Minor cleanup of impBaseIntrinsic for x86/x64
  • Loading branch information
@tannergooding
tannergooding committed Sep 29, 2021
commit 6cddb02a7e3b43afa8e5663643dc0432c6d9ff34
55 changes: 23 additions & 32 deletions src/coreclr/jit/hwintrinsicarm64.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -312,6 +312,11 @@ GenTree* Compiler::impSpecialIntrinsic(NamedIntrinsic intrinsic,
int numArgs = sig->numArgs;
var_types simdBaseType = JitType2PreciseVarType(simdBaseJitType);

if (!featureSIMD || !IsBaselineSimdIsaSupported())
{
return nullptr;
}

assert(numArgs >= 0);
assert(varTypeIsArithmetic(simdBaseType));

Expand Down Expand Up @@ -350,11 +355,6 @@ GenTree* Compiler::impSpecialIntrinsic(NamedIntrinsic intrinsic,
assert(!sig->hasThis());
assert(numArgs == 1);

if (!featureSIMD)
{
return nullptr;
}

// We fold away the cast here, as it only exists to satisfy
// the type system. It is safe to do this here since the retNode type
// and the signature return type are both the same TYP_SIMD.
Expand All @@ -371,11 +371,6 @@ GenTree* Compiler::impSpecialIntrinsic(NamedIntrinsic intrinsic,
// We shouldn't handle this as an intrinsic if the
// respective ISAs have been disabled by the user.

if (!compExactlyDependsOn(InstructionSet_AdvSimd))
{
break;
}

if (sig->numArgs == 1)
{
op1 = impPopStack().val;
Expand Down Expand Up @@ -422,11 +417,6 @@ GenTree* Compiler::impSpecialIntrinsic(NamedIntrinsic intrinsic,
assert(!sig->hasThis());
assert(numArgs == 2);

if (!featureSIMD || !compExactlyDependsOn(InstructionSet_AdvSimd))
{
return nullptr;
}

op2 = impPopStack().val;
op1 = impSIMDPopStack(getSIMDTypeForSize(simdSize));

Expand All @@ -435,10 +425,10 @@ GenTree* Compiler::impSpecialIntrinsic(NamedIntrinsic intrinsic,
break;
}

case NI_Vector64_get_Zero:
case NI_Vector64_get_AllBitsSet:
case NI_Vector128_get_Zero:
case NI_Vector128_get_AllBitsSet:
case NI_Vector64_get_Zero:
case NI_Vector128_get_Zero:
{
assert(!sig->hasThis());
assert(numArgs == 0);
Expand All @@ -447,13 +437,29 @@ GenTree* Compiler::impSpecialIntrinsic(NamedIntrinsic intrinsic,
break;
}

case NI_Vector128_GetUpper:
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think this is the most optimal way of implementing Vector128.GetUpper()...

Perhaps, something that does DUP Vd, Vn.D[1] should be used instead.
This would correspond to vector.AsUInt64().GetElement(1).As<T>()

Copy link
Member Author

Choose a reason for hiding this comment

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

Do you mind if I fix this in a follow up PR (just so we can go ahead and get this one resolved and avoid further downstream conflicts)?

Also noting, this might be a case where we want to recognize the ExtractVector128 pattern and replace it with DUP if we know its always going to be more efficient...

Copy link
Contributor

Choose a reason for hiding this comment

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

Sure, I meant to write that this work can wait when posted the message (but forgot).

{
// Converts to equivalent managed code:
// AdvSimd.ExtractVector128(vector, Vector128<T>.Zero, 8 / sizeof(T)).GetLower();
assert(numArgs == 1);
op1 = impPopStack().val;
GenTree* zero = gtNewSimdHWIntrinsicNode(retType, NI_Vector128_get_Zero, simdBaseJitType, simdSize);
ssize_t index = 8 / genTypeSize(simdBaseType);

retNode = gtNewSimdHWIntrinsicNode(TYP_SIMD16, op1, zero, gtNewIconNode(index), NI_AdvSimd_ExtractVector128,
simdBaseJitType, simdSize);
retNode = gtNewSimdHWIntrinsicNode(TYP_SIMD8, retNode, NI_Vector128_GetLower, simdBaseJitType, 8);
break;
}

case NI_Vector64_WithElement:
case NI_Vector128_WithElement:
{
assert(numArgs == 3);
GenTree* indexOp = impStackTop(1).val;
if (!indexOp->OperIsConst())
{
// TODO-XARCH-CQ: We should always import these like we do with GetElement
// If index is not constant use software fallback.
return nullptr;
}
Expand All @@ -476,21 +482,6 @@ GenTree* Compiler::impSpecialIntrinsic(NamedIntrinsic intrinsic,
break;
}

case NI_Vector128_GetUpper:
{
// Converts to equivalent managed code:
// AdvSimd.ExtractVector128(vector, Vector128<T>.Zero, 8 / sizeof(T)).GetLower();
assert(numArgs == 1);
op1 = impPopStack().val;
GenTree* zero = gtNewSimdHWIntrinsicNode(retType, NI_Vector128_get_Zero, simdBaseJitType, simdSize);
ssize_t index = 8 / genTypeSize(simdBaseType);

retNode = gtNewSimdHWIntrinsicNode(TYP_SIMD16, op1, zero, gtNewIconNode(index), NI_AdvSimd_ExtractVector128,
simdBaseJitType, simdSize);
retNode = gtNewSimdHWIntrinsicNode(TYP_SIMD8, retNode, NI_Vector128_GetLower, simdBaseJitType, 8);
break;
}

default:
{
return nullptr;
Expand Down
Loading