Skip to content
Open
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
18 changes: 15 additions & 3 deletions src/coreclr/jit/morph.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -8572,14 +8572,26 @@ GenTree* Compiler::fgOptimizeCast(GenTreeCast* cast)
}

// For indir-like nodes, we may be able to change their type to satisfy (and discard) the cast.
if (varTypeIsSmall(castToType) && (genTypeSize(castToType) == genTypeSize(src)) &&
src->OperIs(GT_IND, GT_LCL_FLD))
// Look through COMMAs so we can fold `CAST<small>(COMMA(side-effects, IND<small-same-size>))`
// into `COMMA(side-effects, IND<castToType>)` -- e.g. `(sbyte)span[i]` should lower to a
// single sign-extending load instead of `movzx`/`movsx`.
GenTree* effectiveSrc = src->gtEffectiveVal();
if (varTypeIsSmall(castToType) && (genTypeSize(castToType) == genTypeSize(effectiveSrc)) &&
effectiveSrc->OperIs(GT_IND, GT_LCL_FLD))
{
// We're changing the type here so we need to update the VN;
// in other cases we discard the cast without modifying src
// so the VN doesn't change.

src->ChangeType(castToType);
effectiveSrc->ChangeType(castToType);

// Propagate the new type up through any COMMA wrappers so that the
// tree's type accurately describes the value being produced.
for (GenTree* cur = src; cur != effectiveSrc; cur = cur->AsOp()->gtOp2)
{
cur->ChangeType(castToType);
}

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Doesn't ChangeType do this already if called on the right node?


src->SetVNsFromNode(cast);

return src;
Comment on lines +8586 to 8597
Expand Down
109 changes: 109 additions & 0 deletions src/tests/JIT/Regression/JitBlue/Runtime_69472/Runtime_69472.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,109 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.

namespace Runtime_69472;

using System;
using System.Runtime.CompilerServices;
using Xunit;

// Validates the fgOptimizeCast extension that looks through GT_COMMA wrappers
// to fold `CAST<small>(COMMA(side-effects, IND<small-same-size>))` into
// `COMMA(side-effects, IND<castToType>)`. The common shape is
// `(sbyte)span[i]` which previously emitted a zero-extending load followed by
// a sign-extending cast and should now collapse to a single `movsx` (or
// equivalently `movzx`) load.
Comment on lines +13 to +15
//
// Correctness must hold across signed/unsigned source-and-target combinations
// and across all relevant integer values for the small type.

public class Runtime_69472
{
[MethodImpl(MethodImplOptions.NoInlining)]
private static int SbyteFromByteArr(byte[] a, int i) => (sbyte)a[i];

[MethodImpl(MethodImplOptions.NoInlining)]
private static int ByteFromSbyteArr(sbyte[] a, int i) => (byte)a[i];

[MethodImpl(MethodImplOptions.NoInlining)]
private static int ShortFromUshortArr(ushort[] a, int i) => (short)a[i];

[MethodImpl(MethodImplOptions.NoInlining)]
private static int UshortFromShortArr(short[] a, int i) => (ushort)a[i];

[MethodImpl(MethodImplOptions.NoInlining)]
private static int SbyteFromReadOnlySpan(ReadOnlySpan<byte> s, int i) => (sbyte)s[i];

[MethodImpl(MethodImplOptions.NoInlining)]
private static unsafe int SbyteFromPointer(byte* p) => (sbyte)*p;

[MethodImpl(MethodImplOptions.NoInlining)]
private static int SbyteFromByteField(Boxed b) => (sbyte)b._value;

private sealed class Boxed
{
public byte _value;
}

[Fact]
public static int TestEntryPoint()
{
bool ok = true;

for (int v = 0; v < 256; v++)
{
byte b = (byte)v;
sbyte sb = unchecked((sbyte)v);

byte[] ba = new[] { b };
sbyte[] sba = new[] { sb };

int expSbyteFromByte = unchecked((sbyte)b);
int expByteFromSbyte = (byte)sb;

if (SbyteFromByteArr(ba, 0) != expSbyteFromByte)
{
ok = false;
}
if (ByteFromSbyteArr(sba, 0) != expByteFromSbyte)
{
ok = false;
}
if (SbyteFromReadOnlySpan(ba, 0) != expSbyteFromByte)
{
ok = false;
}
if (SbyteFromByteField(new Boxed { _value = b }) != expSbyteFromByte)
{
ok = false;
}
unsafe
{
byte bb = b;
if (SbyteFromPointer(&bb) != expSbyteFromByte)
{
ok = false;
}
}
}

for (int v = 0; v <= 0xFFFF; v++)
{
ushort uw = (ushort)v;
short sw = unchecked((short)v);
ushort[] ua = new[] { uw };
short[] sa = new[] { sw };

Comment on lines +90 to +96
if (ShortFromUshortArr(ua, 0) != unchecked((short)uw))
{
ok = false;
}
if (UshortFromShortArr(sa, 0) != (ushort)sw)
{
ok = false;
}
}

return ok ? 100 : 1;
}
}
1 change: 1 addition & 0 deletions src/tests/JIT/Regression/Regression_ro_2.csproj
Original file line number Diff line number Diff line change
Expand Up @@ -102,6 +102,7 @@
<Compile Include="JitBlue\Runtime_128801\Runtime_128801.cs" />
<Compile Include="JitBlue\Runtime_129076\Runtime_129076.cs" />
<Compile Include="JitBlue\Runtime_129176\Runtime_129176.cs" />
<Compile Include="JitBlue\Runtime_69472\Runtime_69472.cs" />

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Should this be a regression test?

</ItemGroup>
<Import Project="$(TestSourceDir)MergedTestRunner.targets" />
</Project>
Loading