Skip to content

Commit 9534947

Browse files
authored
JIT: Fix generating unrepresentable nullchecks on ARM64 (#71687)
The logic in lowering could in some struct and primitive type cases generate an unrepresentable NULLCHECK node for ARM64/LA64 when there was a contained address mode. We now transform unused indirections and create address modes in the opposite order on non-xarch to avoid these unrepresentable nullchecks. Fix #71684
1 parent a965218 commit 9534947

File tree

4 files changed

+77
-3
lines changed

4 files changed

+77
-3
lines changed

src/coreclr/jit/compiler.cpp

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -9992,16 +9992,21 @@ bool Compiler::lvaIsOSRLocal(unsigned varNum)
99929992
//
99939993
var_types Compiler::gtTypeForNullCheck(GenTree* tree)
99949994
{
9995-
if (varTypeIsArithmetic(tree))
9995+
static const var_types s_typesBySize[] = {TYP_UNDEF, TYP_BYTE, TYP_SHORT, TYP_UNDEF, TYP_INT,
9996+
TYP_UNDEF, TYP_UNDEF, TYP_UNDEF, TYP_LONG};
9997+
9998+
if (!varTypeIsStruct(tree))
99969999
{
999710000
#if defined(TARGET_XARCH)
999810001
// Just an optimization for XARCH - smaller mov
9999-
if (varTypeIsLong(tree))
10002+
if (genTypeSize(tree) == 8)
1000010003
{
1000110004
return TYP_INT;
1000210005
}
1000310006
#endif
10004-
return tree->TypeGet();
10007+
10008+
assert((genTypeSize(tree) < ARRAY_SIZE(s_typesBySize)) && (s_typesBySize[genTypeSize(tree)] != TYP_UNDEF));
10009+
return s_typesBySize[genTypeSize(tree)];
1000510010
}
1000610011
// for the rest: probe a single byte to avoid potential AVEs
1000710012
return TYP_BYTE;

src/coreclr/jit/lower.cpp

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7149,6 +7149,17 @@ void Lowering::LowerIndir(GenTreeIndir* ind)
71497149
// they only appear as the source of a block copy operation or a return node.
71507150
if (!ind->TypeIs(TYP_STRUCT) || ind->IsUnusedValue())
71517151
{
7152+
#ifndef TARGET_XARCH
7153+
// On non-xarch, whether or not we can contain an address mode will depend on the access width
7154+
// which may be changed when transforming an unused indir, so do that first.
7155+
// On xarch, it is the opposite: we transform to indir/nullcheck based on whether we contained the
7156+
// address mode, so in that case we must do this transformation last.
7157+
if (ind->OperIs(GT_NULLCHECK) || ind->IsUnusedValue())
7158+
{
7159+
TransformUnusedIndirection(ind, comp, m_block);
7160+
}
7161+
#endif
7162+
71527163
// TODO-Cleanup: We're passing isContainable = true but ContainCheckIndir rejects
71537164
// address containment in some cases so we end up creating trivial (reg + offfset)
71547165
// or (reg + reg) LEAs that are not necessary.
@@ -7165,10 +7176,12 @@ void Lowering::LowerIndir(GenTreeIndir* ind)
71657176
TryCreateAddrMode(ind->Addr(), isContainable, ind);
71667177
ContainCheckIndir(ind);
71677178

7179+
#ifdef TARGET_XARCH
71687180
if (ind->OperIs(GT_NULLCHECK) || ind->IsUnusedValue())
71697181
{
71707182
TransformUnusedIndirection(ind, comp, m_block);
71717183
}
7184+
#endif
71727185
}
71737186
else
71747187
{
Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,47 @@
1+
// Licensed to the .NET Foundation under one or more agreements.
2+
// The .NET Foundation licenses this file to you under the MIT license.
3+
4+
using System.Numerics;
5+
using System.Runtime.CompilerServices;
6+
using System.Runtime.Intrinsics;
7+
8+
class Runtime_71687
9+
{
10+
[MethodImpl(MethodImplOptions.NoInlining)]
11+
private static void Test<T>(ref T first, int i)
12+
{
13+
Consume(Unsafe.Add(ref first, i));
14+
}
15+
16+
// Must be inlined so we end up with null check above
17+
private static void Consume<T>(T value) { }
18+
19+
private static int Main()
20+
{
21+
Test(ref (new byte[10])[0], 5);
22+
Test(ref (new sbyte[10])[0], 5);
23+
Test(ref (new ushort[10])[0], 5);
24+
Test(ref (new short[10])[0], 5);
25+
Test(ref (new uint[10])[0], 5);
26+
Test(ref (new int[10])[0], 5);
27+
Test(ref (new ulong[10])[0], 5);
28+
Test(ref (new long[10])[0], 5);
29+
Test(ref (new float[10])[0], 5);
30+
Test(ref (new double[10])[0], 5);
31+
Test(ref (new object[10])[0], 5);
32+
Test(ref (new string[10])[0], 5);
33+
Test(ref (new Vector<float>[10])[0], 5);
34+
Test(ref (new Vector128<float>[10])[0], 5);
35+
Test(ref (new Vector256<float>[10])[0], 5);
36+
Test(ref (new Struct1[10])[0], 5);
37+
Test(ref (new Struct2[10])[0], 5);
38+
Test(ref (new Struct4[10])[0], 5);
39+
Test(ref (new Struct8[10])[0], 5);
40+
return 100;
41+
}
42+
43+
private struct Struct1 { public byte Field; }
44+
private struct Struct2 { public short Field; }
45+
private struct Struct4 { public int Field; }
46+
private struct Struct8 { public long Field; }
47+
}
Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
<Project Sdk="Microsoft.NET.Sdk">
2+
<PropertyGroup>
3+
<OutputType>Exe</OutputType>
4+
<Optimize>True</Optimize>
5+
</PropertyGroup>
6+
<ItemGroup>
7+
<Compile Include="$(MSBuildProjectName).cs" />
8+
</ItemGroup>
9+
</Project>

0 commit comments

Comments
 (0)