Skip to content

Commit

Permalink
Account for type mismatch of FIELD_LIST members in LSRA (#57450)
Browse files Browse the repository at this point in the history
* Add the test

* Fix the bug

Assertion propagation can replace a TYP_UBYTE field
in a field list with a TYP_INT constant, so LSRA
must use the actual field type when deciding whether
an internal register to allocate must be byteable.
  • Loading branch information
SingleAccretion committed Aug 16, 2021
1 parent a1eefbb commit ac7b120
Show file tree
Hide file tree
Showing 4 changed files with 96 additions and 8 deletions.
8 changes: 4 additions & 4 deletions src/coreclr/jit/lowerxarch.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -452,10 +452,10 @@ void Lowering::LowerPutArgStk(GenTreePutArgStk* putArgStk)
unsigned prevOffset = putArgStk->GetStackByteSize();
for (GenTreeFieldList::Use& use : fieldList->Uses())
{
GenTree* const fieldNode = use.GetNode();
const var_types fieldType = fieldNode->TypeGet();
const unsigned fieldOffset = use.GetOffset();
assert(fieldType != TYP_LONG);
GenTree* const fieldNode = use.GetNode();
const unsigned fieldOffset = use.GetOffset();

assert(!fieldNode->TypeIs(TYP_LONG));

// We can treat as a slot any field that is stored at a slot boundary, where the previous
// field is not in the same slot. (Note that we store the fields in reverse order.)
Expand Down
8 changes: 4 additions & 4 deletions src/coreclr/jit/lsraxarch.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1501,18 +1501,18 @@ int LinearScan::BuildPutArgStk(GenTreePutArgStk* putArgStk)
for (GenTreeFieldList::Use& use : putArgStk->gtOp1->AsFieldList()->Uses())
{
GenTree* const fieldNode = use.GetNode();
const var_types fieldType = fieldNode->TypeGet();
const unsigned fieldOffset = use.GetOffset();
const var_types fieldType = use.GetType();

#ifdef TARGET_X86
assert(fieldType != TYP_LONG);
#endif // TARGET_X86

#if defined(FEATURE_SIMD)
// Note that we need to check the GT_FIELD_LIST type, not 'fieldType'. This is because the
// GT_FIELD_LIST will be TYP_SIMD12 whereas the fieldType might be TYP_SIMD16 for lclVar, where
// Note that we need to check the field type, not the type of the node. This is because the
// field type will be TYP_SIMD12 whereas the node type might be TYP_SIMD16 for lclVar, where
// we "round up" to 16.
if ((use.GetType() == TYP_SIMD12) && (simdTemp == nullptr))
if ((fieldType == TYP_SIMD12) && (simdTemp == nullptr))
{
simdTemp = buildInternalFloatRegisterDefForNode(putArgStk);
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,75 @@
using System.Runtime.CompilerServices;
using System.Runtime.InteropServices;

public unsafe class FieldListByteNodeTypeMismatchX86
{
private static readonly byte* _addr = (byte*)Marshal.AllocHGlobal(20);
private static int _result = 100;

public static int Main()
{
int sum = 0;
Problem(&sum);

return _result;
}

[MethodImpl(MethodImplOptions.NoInlining)]
private static void Problem(int* sum)
{
// Just a loop with some computations so that we can use
// callee-saved registers (which happen to be non-byteable ones).
for (int i = 0; i < 10; i++)
{
var i1 = i ^ i;
var i2 = i | i;
var i3 = i & i;
var i4 = i1 + i + i2 - i3;
i4 = i2 ^ i4;

*sum += i4;
}

Sx2x1 s;
byte* j = Addr();

int o1 = j[-2];
int o2 = j[-1];
s.Short = j[0];
s.Byte = j[1];

o1 = o1 + o1;
o2 = o2 + o2;

if (s.Byte != 1)
{
return;
}

// Here assertion propagation will make s.Byte into CNS_INT(TYP_INT), yet the RA must
// still allocate a byteable register so that it can be saved to the stack correctly.
Call(s, 0, 0, o2, o1, j);
}

[MethodImpl(MethodImplOptions.NoInlining)]
private static byte* Addr()
{
_addr[11] = 1;
return _addr + 10;
}

[MethodImpl(MethodImplOptions.NoInlining)]
private static void Call(Sx2x1 s, int i3, int i4, int i5, int i6, byte* i1)
{
if (s.Byte != 1)
{
_result = -1;
}
}

private struct Sx2x1
{
public short Short;
public byte Byte;
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
<Project Sdk="Microsoft.NET.Sdk">
<PropertyGroup>
<OutputType>Exe</OutputType>
<AllowUnsafeBlocks>true</AllowUnsafeBlocks>
</PropertyGroup>
<PropertyGroup>
<DebugType>None</DebugType>
<Optimize>True</Optimize>
</PropertyGroup>
<ItemGroup>
<Compile Include="$(MSBuildProjectName).cs" />
</ItemGroup>
</Project>

0 comments on commit ac7b120

Please sign in to comment.