Skip to content

Commit

Permalink
Do not use LEAs in HIR (#69992)
Browse files Browse the repository at this point in the history
* Make LEAs backend-only

In the frontend, for better or worse, the canonical form
of address arithmetic is explicit pointer arithmetic.

* Add a test
  • Loading branch information
SingleAccretion authored Jun 7, 2022
1 parent 50d289c commit 019a009
Show file tree
Hide file tree
Showing 6 changed files with 70 additions and 35 deletions.
2 changes: 1 addition & 1 deletion src/coreclr/jit/gtlist.h
Original file line number Diff line number Diff line change
Expand Up @@ -152,7 +152,7 @@ GTNODE(INDEX , GenTreeIndex ,0,GTK_BINOP|GTK_EXOP|DBK_NOTLIR) /
GTNODE(INDEX_ADDR , GenTreeIndexAddr ,0,GTK_BINOP|GTK_EXOP) // Addr of SZ-array-element; used when aiming to minimize compile times.

GTNODE(MKREFANY , GenTreeOp ,0,GTK_BINOP|DBK_NOTLIR)
GTNODE(LEA , GenTreeAddrMode ,0,GTK_BINOP|GTK_EXOP)
GTNODE(LEA , GenTreeAddrMode ,0,GTK_BINOP|GTK_EXOP|DBK_NOTHIR)

#if !defined(TARGET_64BIT)
// A GT_LONG node simply represents the long value produced by the concatenation
Expand Down
12 changes: 4 additions & 8 deletions src/coreclr/jit/morph.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2033,8 +2033,8 @@ void CallArgs::AddFinalArgsAndDetermineABIInfo(Compiler* comp, GenTreeCall* call
}
noway_assert(cloned != nullptr);

GenTree* newArg = new (comp, GT_ADDR)
GenTreeAddrMode(TYP_BYREF, cloned, nullptr, 0, comp->eeGetEEInfo()->offsetOfWrapperDelegateIndirectCell);
GenTree* offsetNode = comp->gtNewIconNode(comp->eeGetEEInfo()->offsetOfWrapperDelegateIndirectCell, TYP_I_IMPL);
GenTree* newArg = comp->gtNewOperNode(GT_ADD, TYP_BYREF, cloned, offsetNode);

// Append newArg as the last arg
PushBack(comp, NewCallArg::Primitive(newArg).WellKnown(WellKnownArg::WrapperDelegateCell));
Expand Down Expand Up @@ -17529,12 +17529,8 @@ bool Compiler::fgMorphCombineSIMDFieldAssignments(BasicBlock* block, Statement*
{
setLclRelatedToSIMDIntrinsic(simdStructNode);
}
GenTree* copyBlkAddr = copyBlkDst;
if (copyBlkAddr->gtOper == GT_LEA)
{
copyBlkAddr = copyBlkAddr->AsAddrMode()->Base();
}
GenTreeLclVarCommon* localDst = copyBlkAddr->IsLocalAddrExpr();

GenTreeLclVarCommon* localDst = copyBlkDst->IsLocalAddrExpr();
if (localDst != nullptr)
{
setLclRelatedToSIMDIntrinsic(localDst);
Expand Down
36 changes: 22 additions & 14 deletions src/coreclr/jit/simd.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1701,7 +1701,7 @@ bool Compiler::areArgumentsContiguous(GenTree* op1, GenTree* op2)
}

//--------------------------------------------------------------------------------------------------------
// createAddressNodeForSIMDInit: Generate the address node(GT_LEA) if we want to intialize vector2, vector3 or vector4
// createAddressNodeForSIMDInit: Generate the address node if we want to intialize vector2, vector3 or vector4
// from first argument's address.
//
// Arguments:
Expand All @@ -1715,15 +1715,13 @@ bool Compiler::areArgumentsContiguous(GenTree* op1, GenTree* op2)
// TODO-CQ:
// 1. Currently just support for GT_FIELD and GT_INDEX, because we can only verify the GT_INDEX node or GT_Field
// are located contiguously or not. In future we should support more cases.
// 2. Though it happens to just work fine front-end phases are not aware of GT_LEA node. Therefore, convert these
// to use GT_ADDR.
//
GenTree* Compiler::createAddressNodeForSIMDInit(GenTree* tree, unsigned simdSize)
{
assert(tree->OperGet() == GT_FIELD || tree->OperGet() == GT_INDEX);
GenTree* byrefNode = nullptr;
GenTree* startIndex = nullptr;
unsigned offset = 0;
var_types baseType = tree->gtType;
GenTree* byrefNode = nullptr;
unsigned offset = 0;
var_types baseType = tree->gtType;

if (tree->OperGet() == GT_FIELD)
{
Expand Down Expand Up @@ -1781,8 +1779,9 @@ GenTree* Compiler::createAddressNodeForSIMDInit(GenTree* tree, unsigned simdSize
{
unreached();
}
GenTree* address =
new (this, GT_LEA) GenTreeAddrMode(TYP_BYREF, byrefNode, startIndex, genTypeSize(tree->TypeGet()), offset);

GenTree* address = gtNewOperNode(GT_ADD, TYP_BYREF, byrefNode, gtNewIconNode(offset, TYP_I_IMPL));

return address;
}

Expand Down Expand Up @@ -2227,12 +2226,21 @@ GenTree* Compiler::impSIMDIntrinsic(OPCODE opcode,
assert(op1->TypeGet() == simdType);

// copy vector (op1) to array (op2) starting at index (op3)
simdTree = op1;
simdTree = op1;
copyBlkDst = op2;
if (op3 != nullptr)
{
#ifdef TARGET_64BIT
// Upcast the index: it is safe to use a zero-extending cast since we've bounds checked it above.
op3 = gtNewCastNode(TYP_I_IMPL, op3, /* fromUnsigned */ true, TYP_I_IMPL);
#endif // !TARGET_64BIT
GenTree* elemSizeNode = gtNewIconNode(genTypeSize(simdBaseType), TYP_I_IMPL);
GenTree* indexOffs = gtNewOperNode(GT_MUL, TYP_I_IMPL, op3, elemSizeNode);
copyBlkDst = gtNewOperNode(GT_ADD, TYP_BYREF, copyBlkDst, indexOffs);
}

// TODO-Cleanup: Though it happens to just work fine front-end phases are not aware of GT_LEA node.
// Therefore, convert these to use GT_ADDR .
copyBlkDst = new (this, GT_LEA)
GenTreeAddrMode(TYP_BYREF, op2, op3, genTypeSize(simdBaseType), OFFSETOF__CORINFO_Array__data);
copyBlkDst = gtNewOperNode(GT_ADD, TYP_BYREF, copyBlkDst,
gtNewIconNode(OFFSETOF__CORINFO_Array__data, TYP_I_IMPL));
doCopyBlk = true;
}
}
Expand Down
13 changes: 1 addition & 12 deletions src/coreclr/jit/valuenum.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -8785,17 +8785,6 @@ void Compiler::fgValueNumberTree(GenTree* tree)
{
assert(oper != GT_ASG); // We handled assignments earlier.
assert(GenTree::OperIsBinary(oper));
// Standard binary operator.
ValueNumPair op2VNPair;
if (tree->AsOp()->gtOp2 == nullptr)
{
// Handle any GT_LEA nodes as they can have a nullptr for op2.
op2VNPair.SetBoth(ValueNumStore::VNForNull());
}
else
{
op2VNPair = tree->AsOp()->gtOp2->gtVNPair;
}

// Handle a few special cases: if we add a field offset constant to a PtrToXXX, we will get back a
// new
Expand All @@ -8807,7 +8796,7 @@ void Compiler::fgValueNumberTree(GenTree* tree)

ValueNumPair op2vnp;
ValueNumPair op2Xvnp;
vnStore->VNPUnpackExc(op2VNPair, &op2vnp, &op2Xvnp);
vnStore->VNPUnpackExc(tree->AsOp()->gtOp2->gtVNPair, &op2vnp, &op2Xvnp);
ValueNumPair excSetPair = vnStore->VNPExcSetUnion(op1Xvnp, op2Xvnp);

ValueNum newVN = ValueNumStore::NoVN;
Expand Down
32 changes: 32 additions & 0 deletions src/tests/JIT/Regression/JitBlue/Runtime_64375/Runtime_64375.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
// 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 unsafe class Runtime_64375
{
public static int Main()
{
var a = new StructWithFloats { FloatOne = 1, FloatThree = 2 };

return Problem(&a) ? 101 : 100;
}

[MethodImpl(MethodImplOptions.NoInlining)]
private static bool Problem(StructWithFloats* p1)
{
var a = new Vector2(p1->FloatOne, p1->FloatTwo);
var b = new Vector2(p1->FloatThree, p1->FloatFour);

return a == b;
}

struct StructWithFloats
{
public float FloatOne;
public float FloatTwo;
public float FloatThree;
public float FloatFour;
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
<Project Sdk="Microsoft.NET.Sdk">
<PropertyGroup>
<OutputType>Exe</OutputType>
<Optimize>True</Optimize>
<AllowUnsafeBlocks>true</AllowUnsafeBlocks>
</PropertyGroup>
<ItemGroup>
<Compile Include="$(MSBuildProjectName).cs" />
</ItemGroup>
</Project>

0 comments on commit 019a009

Please sign in to comment.