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

Do not retype SIMD nodes #70265

Merged
merged 2 commits into from
Jun 7, 2022
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
4 changes: 0 additions & 4 deletions src/coreclr/jit/codegenarm64.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2324,12 +2324,8 @@ void CodeGen::genSetRegToConst(regNumber targetReg, var_types targetType, GenTre
switch (tree->TypeGet())
{
#if defined(FEATURE_SIMD)
case TYP_LONG:
case TYP_DOUBLE:
case TYP_SIMD8:
{
// TODO-1stClassStructs: do not retype SIMD nodes

if (vecCon->IsAllBitsSet())
{
emit->emitIns_R_I(INS_mvni, attr, targetReg, 0, INS_OPTS_2S);
Expand Down
4 changes: 0 additions & 4 deletions src/coreclr/jit/codegenxarch.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -537,12 +537,8 @@ void CodeGen::genSetRegToConst(regNumber targetReg, var_types targetType, GenTre
switch (tree->TypeGet())
{
#if defined(FEATURE_SIMD)
case TYP_LONG:
case TYP_DOUBLE:
case TYP_SIMD8:
{
// TODO-1stClassStructs: do not retype SIMD nodes

simd8_t constValue = vecCon->gtSimd8Val;
CORINFO_FIELD_HANDLE hnd = emit->emitSimd8Const(constValue);

Expand Down
9 changes: 1 addition & 8 deletions src/coreclr/jit/gentree.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2856,10 +2856,7 @@ unsigned Compiler::gtHashValue(GenTree* tree)
}

case TYP_SIMD8:
case TYP_DOUBLE:
case TYP_LONG:
{
// TODO-1stClassStructs: do not retype SIMD nodes
add = genTreeHashAdd(ulo32(add), vecCon->gtSimd8Val.u32[1]);
add = genTreeHashAdd(ulo32(add), vecCon->gtSimd8Val.u32[0]);
break;
Expand All @@ -2873,7 +2870,6 @@ unsigned Compiler::gtHashValue(GenTree* tree)
}

add = genTreeHashAdd(ulo32(add), vecCon->GetSimdBaseType());
add = genTreeHashAdd(ulo32(add), vecCon->GetSimdSize());
break;
}

Expand Down Expand Up @@ -6928,7 +6924,7 @@ GenTree* Compiler::gtNewSconNode(int CPX, CORINFO_MODULE_HANDLE scpHandle)

GenTreeVecCon* Compiler::gtNewVconNode(var_types type, CorInfoType simdBaseJitType)
{
GenTreeVecCon* vecCon = new (this, GT_CNS_VEC) GenTreeVecCon(type, simdBaseJitType, genTypeSize(type));
GenTreeVecCon* vecCon = new (this, GT_CNS_VEC) GenTreeVecCon(type, simdBaseJitType);
return vecCon;
}

Expand Down Expand Up @@ -11131,11 +11127,8 @@ void Compiler::gtDispConst(GenTree* tree)
switch (vecCon->TypeGet())
{
#if defined(FEATURE_SIMD)
case TYP_LONG:
case TYP_DOUBLE:
case TYP_SIMD8:
{
// TODO-1stClassStructs: do not retype SIMD nodes
simd8_t simdVal = vecCon->gtSimd8Val;
printf("<0x%08x, 0x%08x>", simdVal.u32[0], simdVal.u32[1]);
break;
Expand Down
28 changes: 2 additions & 26 deletions src/coreclr/jit/gentree.h
Original file line number Diff line number Diff line change
Expand Up @@ -3270,7 +3270,6 @@ struct GenTreeVecCon : public GenTree
// size should be `gtType` and the handle should be looked up at callsites where required

unsigned char gtSimdBaseJitType; // SIMD vector base JIT type
unsigned char gtSimdSize; // SIMD vector size in bytes

public:
CorInfoType GetSimdBaseJitType() const
Expand All @@ -3286,17 +3285,6 @@ struct GenTreeVecCon : public GenTree

var_types GetSimdBaseType() const;

unsigned char GetSimdSize() const
{
return gtSimdSize;
}

void SetSimdSize(unsigned simdSize)
{
gtSimdSize = (unsigned char)simdSize;
assert(gtSimdSize == simdSize);
}

#if defined(FEATURE_HW_INTRINSICS)
static bool IsHWIntrinsicCreateConstant(GenTreeHWIntrinsic* node, simd32_t& simd32Val);

Expand All @@ -3308,11 +3296,8 @@ struct GenTreeVecCon : public GenTree
switch (gtType)
{
#if defined(FEATURE_SIMD)
case TYP_LONG:
case TYP_DOUBLE:
case TYP_SIMD8:
{
// TODO-1stClassStructs: do not retype SIMD nodes
return (gtSimd8Val.u64[0] == 0xFFFFFFFFFFFFFFFF);
}

Expand Down Expand Up @@ -3353,11 +3338,8 @@ struct GenTreeVecCon : public GenTree
switch (gtType)
{
#if defined(FEATURE_SIMD)
case TYP_LONG:
case TYP_DOUBLE:
case TYP_SIMD8:
{
// TODO-1stClassStructs: do not retype SIMD nodes
return (left->gtSimd8Val.u64[0] == right->gtSimd8Val.u64[0]);
}

Expand Down Expand Up @@ -3395,11 +3377,8 @@ struct GenTreeVecCon : public GenTree
switch (gtType)
{
#if defined(FEATURE_SIMD)
case TYP_LONG:
case TYP_DOUBLE:
case TYP_SIMD8:
{
// TODO-1stClassStructs: do not retype SIMD nodes
return (gtSimd8Val.u64[0] == 0x0000000000000000);
}

Expand Down Expand Up @@ -3428,14 +3407,11 @@ struct GenTreeVecCon : public GenTree
}
}

GenTreeVecCon(var_types type, CorInfoType simdBaseJitType, unsigned simdSize)
: GenTree(GT_CNS_VEC, type)
, gtSimdBaseJitType((unsigned char)simdBaseJitType)
, gtSimdSize((unsigned char)simdSize)
GenTreeVecCon(var_types type, CorInfoType simdBaseJitType)
: GenTree(GT_CNS_VEC, type), gtSimdBaseJitType((unsigned char)simdBaseJitType)
{
assert(varTypeIsSIMD(type));
assert(gtSimdBaseJitType == simdBaseJitType);
assert(gtSimdSize == simdSize);
}

#if DEBUGGABLE_GENTREE
Expand Down
3 changes: 0 additions & 3 deletions src/coreclr/jit/instr.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -843,11 +843,8 @@ CodeGen::OperandDesc CodeGen::genOperandDesc(GenTree* op)
switch (op->TypeGet())
{
#if defined(FEATURE_SIMD)
case TYP_LONG:
case TYP_DOUBLE:
case TYP_SIMD8:
{
// TODO-1stClassStructs: do not retype SIMD nodes
simd8_t constValue = op->AsVecCon()->gtSimd8Val;
return OperandDesc(emit->emitSimd8Const(constValue));
}
Expand Down
19 changes: 16 additions & 3 deletions src/coreclr/jit/morph.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -3404,11 +3404,24 @@ GenTreeCall* Compiler::fgMorphArgs(GenTreeCall* call)
argObj->gtType = structBaseType;
}
}
else
else if (argObj->OperIs(GT_LCL_FLD, GT_IND))
{
// Not a GT_LCL_VAR, so we can just change the type on the node
// We can just change the type on the node
argObj->gtType = structBaseType;
}
else
{
#ifdef FEATURE_SIMD
// We leave the SIMD8 <-> LONG (Windows x64) case to lowering. For SIMD8 <-> DOUBLE (Unix x64),
// we do not need to do anything as both types already use floating-point registers.
assert((argObj->TypeIs(TYP_SIMD8) &&
((structBaseType == TYP_LONG) || (structBaseType == TYP_DOUBLE))) ||
argObj->TypeIs(structBaseType));
#else // !FEATURE_SIMD
unreached();
#endif // !FEATURE_SIMD
}

assert(varTypeIsEnregisterable(argObj->TypeGet()) ||
(makeOutArgCopy && varTypeIsEnregisterable(structBaseType)));
}
Expand Down Expand Up @@ -9922,7 +9935,7 @@ GenTree* Compiler::getSIMDStructFromField(GenTree* tree,
{
ret = obj;
GenTreeVecCon* vecCon = obj->AsVecCon();
*simdSizeOut = vecCon->GetSimdSize();
*simdSizeOut = genTypeSize(vecCon);
*simdBaseJitTypeOut = vecCon->GetSimdBaseJitType();
}
}
Expand Down
58 changes: 0 additions & 58 deletions src/coreclr/jit/rationalize.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -670,16 +670,6 @@ Compiler::fgWalkResult Rationalizer::RewriteNode(GenTree** useEdge, Compiler::Ge
unsigned simdSize = simdNode->GetSimdSize();
var_types simdType = comp->getSIMDTypeForSize(simdSize);

// TODO-1stClassStructs: This should be handled more generally for enregistered or promoted
// structs that are passed or returned in a different register type than their enregistered
// type(s).
if (simdNode->gtType == TYP_I_IMPL && simdNode->GetSimdSize() == TARGET_POINTER_SIZE)
{
// This happens when it is consumed by a GT_RET_EXPR.
// It can only be a Vector2f or Vector2i.
assert(genTypeSize(simdNode->GetSimdBaseType()) == 4);
simdNode->gtType = TYP_SIMD8;
}
// Certain SIMD trees require rationalizing.
if (simdNode->AsSIMD()->GetSIMDIntrinsicId() == SIMDIntrinsicInitArray)
{
Expand All @@ -704,54 +694,6 @@ Compiler::fgWalkResult Rationalizer::RewriteNode(GenTree** useEdge, Compiler::Ge
break;
#endif // FEATURE_SIMD

#ifdef FEATURE_HW_INTRINSICS
case GT_HWINTRINSIC:
{
GenTreeHWIntrinsic* hwIntrinsicNode = node->AsHWIntrinsic();

if (!hwIntrinsicNode->isSIMD())
{
break;
}

// TODO-1stClassStructs: This should be handled more generally for enregistered or promoted
// structs that are passed or returned in a different register type than their enregistered
// type(s).
if ((hwIntrinsicNode->gtType == TYP_I_IMPL) && (hwIntrinsicNode->GetSimdSize() == TARGET_POINTER_SIZE))
{
#ifdef TARGET_ARM64
// Special case for GetElement/ToScalar because they take Vector64<T> and return T
// and T can be long or ulong.
if (!((hwIntrinsicNode->GetHWIntrinsicId() == NI_Vector64_GetElement) ||
(hwIntrinsicNode->GetHWIntrinsicId() == NI_Vector64_ToScalar)))
#endif
{
// This happens when it is consumed by a GT_RET_EXPR.
// It can only be a Vector2f or Vector2i.
assert(genTypeSize(hwIntrinsicNode->GetSimdBaseType()) == 4);
hwIntrinsicNode->gtType = TYP_SIMD8;
}
}
break;
}
#endif // FEATURE_HW_INTRINSICS

#if defined(FEATURE_SIMD)
case GT_CNS_VEC:
{
GenTreeVecCon* vecCon = node->AsVecCon();

// TODO-1stClassStructs: do not retype SIMD nodes

if ((vecCon->TypeIs(TYP_I_IMPL)) && (vecCon->GetSimdSize() == TARGET_POINTER_SIZE))
{
assert(genTypeSize(vecCon->GetSimdBaseType()) == 4);
vecCon->gtType = TYP_SIMD8;
}
break;
}
#endif // FEATURE_SIMD

default:
// Check that we don't have nodes not allowed in HIR here.
assert((node->DebugOperKind() & DBK_NOTHIR) == 0);
Expand Down
23 changes: 1 addition & 22 deletions src/coreclr/jit/valuenum.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -7959,16 +7959,6 @@ void Compiler::fgValueNumberTreeConst(GenTree* tree)
tree->gtVNPair.SetBoth(
vnStore->VNForHandle(ssize_t(tree->AsIntConCommon()->IconValue()), tree->GetIconHandleFlag()));
}
#ifdef FEATURE_SIMD
else if (tree->IsCnsVec())
{
// TODO-1stClassStructs: do not retype SIMD nodes
assert(varTypeIsLong(typ));

simd8_t simd8Val = tree->AsVecCon()->gtSimd8Val;
tree->gtVNPair.SetBoth(vnStore->VNForSimd8Con(simd8Val));
}
#endif // FEATURE_SIMD
else if ((typ == TYP_LONG) || (typ == TYP_ULONG))
{
tree->gtVNPair.SetBoth(vnStore->VNForLongCon(INT64(tree->AsIntConCommon()->LngValue())));
Expand Down Expand Up @@ -8061,18 +8051,7 @@ void Compiler::fgValueNumberTreeConst(GenTree* tree)

case TYP_DOUBLE:
{
#ifdef FEATURE_SIMD
if (tree->IsCnsVec())
{
// TODO-1stClassStructs: do not retype SIMD nodes
simd8_t simd8Val = tree->AsVecCon()->gtSimd8Val;
tree->gtVNPair.SetBoth(vnStore->VNForSimd8Con(simd8Val));
}
else
#endif // FEATURE_SIMD
{
tree->gtVNPair.SetBoth(vnStore->VNForDoubleCon(tree->AsDblCon()->gtDconVal));
}
tree->gtVNPair.SetBoth(vnStore->VNForDoubleCon(tree->AsDblCon()->gtDconVal));
break;
}

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

using System.Numerics;
using System.Runtime.CompilerServices;

public class Runtime_70124
{
public static int Main()
{
return Problem(Vector2.One, Vector2.One) != new Vector2(3, 3) ? 101 : 100;
}

[MethodImpl(MethodImplOptions.NoInlining)]
private static Vector2 Problem(Vector2 a, Vector2 b)
{
CallForVtor2(a + b);

return (a + b) + a;
}

[MethodImpl(MethodImplOptions.NoInlining)]
private static void CallForVtor2(Vector2 vtor) { }
}
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>