Skip to content

Commit

Permalink
Add back missing conv opcodes when compiling via System.Linq.Expressi…
Browse files Browse the repository at this point in the history
…ons (#76024)

* Add new convert tests

With expression funcs that return int32 instead of short/byte to prevent implicit conversions.

* Add back missing conv opcodes when compiling via System.Linq.Expressions

The conversion opcodes are still necessary when the sign of the value might change, in which case the conversion opcode will do a sign/zero extend to the full i32 storage used by the IL execution stack.

For example, before this change, conversions from ushort to short were ignored. Consider expressions converting the value `ushort.MaxValue` to short (testcase ConvertUShortToShortTest). `ushort.MaxValue` will be pushed to execution stack as a i32 ldc of value 0xffff. The conv.i2 opcode would change the value on the stack to 0xffffffff so it shouldn't be omitted.
  • Loading branch information
BrzVlad committed Sep 24, 2022
1 parent 901b0a1 commit 05aa1fd
Show file tree
Hide file tree
Showing 2 changed files with 80 additions and 26 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -673,11 +673,6 @@ private static void EmitNumericConversion(this ILGenerator il, Type typeFrom, Ty
}
else
{
if (tf == TypeCode.Byte)
{
return;
}

convCode = OpCodes.Conv_I1;
}

Expand All @@ -689,11 +684,6 @@ private static void EmitNumericConversion(this ILGenerator il, Type typeFrom, Ty
}
else
{
if (tf == TypeCode.SByte)
{
return;
}

convCode = OpCodes.Conv_U1;
}

Expand All @@ -704,14 +694,6 @@ private static void EmitNumericConversion(this ILGenerator il, Type typeFrom, Ty
case TypeCode.SByte:
case TypeCode.Byte:
return;
case TypeCode.Char:
case TypeCode.UInt16:
if (!isChecked)
{
return;
}

break;
}

convCode = isChecked
Expand All @@ -726,14 +708,6 @@ private static void EmitNumericConversion(this ILGenerator il, Type typeFrom, Ty
case TypeCode.Char:
case TypeCode.UInt16:
return;
case TypeCode.SByte:
case TypeCode.Int16:
if (!isChecked)
{
return;
}

break;
}

convCode = isChecked
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -182,6 +182,15 @@ public static void ConvertByteToSByteTest(bool useInterpreter)
}
}

[Theory, ClassData(typeof(CompilationTypes))]
public static void ConvertByteToSByteRetIntTest(bool useInterpreter)
{
foreach (byte value in new byte[] { 0, 1, byte.MaxValue })
{
VerifyByteToSByteRetInt(value, useInterpreter);
}
}

[Theory, ClassData(typeof(CompilationTypes))]
public static void ConvertByteToNullableSByteTest(bool useInterpreter)
{
Expand Down Expand Up @@ -4412,6 +4421,15 @@ public static void ConvertSByteToByteTest(bool useInterpreter)
}
}

[Theory, ClassData(typeof(CompilationTypes))]
public static void ConvertSByteToByteRetIntTest(bool useInterpreter)
{
foreach (sbyte value in new sbyte[] { 0, 1, -1, sbyte.MinValue, sbyte.MaxValue })
{
VerifySByteToByteRetInt(value, useInterpreter);
}
}

[Theory, ClassData(typeof(CompilationTypes))]
public static void ConvertSByteToNullableByteTest(bool useInterpreter)
{
Expand Down Expand Up @@ -5150,6 +5168,15 @@ public static void ConvertShortToUShortTest(bool useInterpreter)
}
}

[Theory, ClassData(typeof(CompilationTypes))]
public static void ConvertShortToUShortRetIntTest(bool useInterpreter)
{
foreach (short value in new short[] { 0, 1, -1, short.MinValue, short.MaxValue })
{
VerifyShortToUShortRetInt(value, useInterpreter);
}
}

[Theory, ClassData(typeof(CompilationTypes))]
public static void ConvertShortToNullableUShortTest(bool useInterpreter)
{
Expand Down Expand Up @@ -6608,6 +6635,15 @@ public static void ConvertUShortToShortTest(bool useInterpreter)
}
}

[Theory, ClassData(typeof(CompilationTypes))]
public static void ConvertUShortToShortRetIntTest(bool useInterpreter)
{
foreach (ushort value in new ushort[] { 0, 1, ushort.MaxValue })
{
VerifyUShortToShortRetInt(value, useInterpreter);
}
}

[Theory, ClassData(typeof(CompilationTypes))]
public static void ConvertUShortToNullableShortTest(bool useInterpreter)
{
Expand Down Expand Up @@ -7136,6 +7172,17 @@ private static void VerifyByteToSByte(byte value, bool useInterpreter)
Assert.Equal(unchecked((sbyte)value), f());
}

private static void VerifyByteToSByteRetInt(byte value, bool useInterpreter)
{
Expression<Func<int>> e =
Expression.Lambda<Func<int>>(
Expression.Convert(Expression.Convert(Expression.Constant(value, typeof(byte)), typeof(sbyte)), typeof(int)),
Enumerable.Empty<ParameterExpression>());
Func<int> f = e.Compile(useInterpreter);

Assert.Equal((int)unchecked((sbyte)value), f());
}

private static void VerifyByteToNullableSByte(byte value, bool useInterpreter)
{
Expression<Func<sbyte?>> e =
Expand Down Expand Up @@ -13178,6 +13225,17 @@ private static void VerifySByteToByte(sbyte value, bool useInterpreter)
Assert.Equal(unchecked((byte)value), f());
}

private static void VerifySByteToByteRetInt(sbyte value, bool useInterpreter)
{
Expression<Func<int>> e =
Expression.Lambda<Func<int>>(
Expression.Convert(Expression.Convert(Expression.Constant(value, typeof(sbyte)), typeof(byte)), typeof(int)),
Enumerable.Empty<ParameterExpression>());
Func<int> f = e.Compile(useInterpreter);

Assert.Equal((int)unchecked((byte)value), f());
}

private static void VerifySByteToNullableByte(sbyte value, bool useInterpreter)
{
Expression<Func<byte?>> e =
Expand Down Expand Up @@ -14122,6 +14180,17 @@ private static void VerifyShortToUShort(short value, bool useInterpreter)
Assert.Equal(unchecked((ushort)value), f());
}

private static void VerifyShortToUShortRetInt(short value, bool useInterpreter)
{
Expression<Func<int>> e =
Expression.Lambda<Func<int>>(
Expression.Convert(Expression.Convert(Expression.Constant(value, typeof(short)), typeof(ushort)), typeof(int)),
Enumerable.Empty<ParameterExpression>());
Func<int> f = e.Compile(useInterpreter);

Assert.Equal((int)unchecked((ushort)value), f());
}

private static void VerifyShortToNullableUShort(short value, bool useInterpreter)
{
Expression<Func<ushort?>> e =
Expand Down Expand Up @@ -16030,6 +16099,17 @@ private static void VerifyUShortToShort(ushort value, bool useInterpreter)
Assert.Equal(unchecked((short)value), f());
}

private static void VerifyUShortToShortRetInt(ushort value, bool useInterpreter)
{
Expression<Func<int>> e =
Expression.Lambda<Func<int>>(
Expression.Convert(Expression.Convert(Expression.Constant(value, typeof(ushort)), typeof(short)), typeof(int)),
Enumerable.Empty<ParameterExpression>());
Func<int> f = e.Compile(useInterpreter);

Assert.Equal((int)unchecked((short)value), f());
}

private static void VerifyUShortToNullableShort(ushort value, bool useInterpreter)
{
Expression<Func<short?>> e =
Expand Down

0 comments on commit 05aa1fd

Please sign in to comment.