Skip to content

Commit

Permalink
Forbid - byref cnst -> + (byref -cnst) transformation. (#44266)
Browse files Browse the repository at this point in the history
* Add a repro test.

* Forbid the transformation for byrefs.

* Update src/coreclr/src/jit/morph.cpp

Co-authored-by: Andy Ayers <andya@microsoft.com>

* Update src/coreclr/src/jit/morph.cpp

* Fix the test return value.

WriteLine is just to make sure we don't delete the value.

* improve the test.

avoid a possible overflow and don't waste time on printing.

Co-authored-by: Andy Ayers <andya@microsoft.com>
  • Loading branch information
Sergey Andreenko and AndyAyersMS committed Nov 6, 2020
1 parent 667e71d commit 5ed389b
Show file tree
Hide file tree
Showing 3 changed files with 76 additions and 2 deletions.
5 changes: 3 additions & 2 deletions src/coreclr/src/jit/morph.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -13354,9 +13354,10 @@ GenTree* Compiler::fgMorphSmpOp(GenTree* tree, MorphAddrContext* mac)
/* Check for "op1 - cns2" , we change it to "op1 + (-cns2)" */

noway_assert(op2);
if (op2->IsCnsIntOrI())
if (op2->IsCnsIntOrI() && !op2->IsIconHandle())
{
/* Negate the constant and change the node to be "+" */
// Negate the constant and change the node to be "+",
// except when `op2` is a const byref.

op2->AsIntConCommon()->SetIconValue(-op2->AsIntConCommon()->IconValue());
op2->AsIntConRef().gtFieldSeq = FieldSeqStore::NotAField();
Expand Down
63 changes: 63 additions & 0 deletions src/tests/JIT/Regression/JitBlue/Runtime_44266/Runtime_44266.il
Original file line number Diff line number Diff line change
@@ -0,0 +1,63 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.

// This test shows an inlining of `byref LCL_VAR_ADDR - byref CNST_INT` method that returns a native int.
// However, Jit could try to optimize `-` as `+ -CNST_INT` that could lead to an incorrect `long + (-byref)`.

.assembly extern System.Console {}
.assembly extern legacy library mscorlib {}
.assembly byrefsubbyref1 { }
.class a extends [mscorlib]System.Object
{
.field static class ctest S_1
.method public static native int byrefsubbyref(class ctest& V_1, class ctest& V_2) aggressiveinlining
{
ldarg 0
ldarg 1
sub
ret
}

.method public static int32 main() cil managed
{
.entrypoint
.maxstack 2
.locals init (class ctest V_1,
class ctest V_2,
native int V_3)
newobj instance void ctest::.ctor()
stloc.0
newobj instance void ctest::.ctor()
dup
stsfld class ctest a::S_1
stloc.1

ldloca V_1
ldsflda class ctest a::S_1
call native int a::byrefsubbyref(class ctest&, class ctest&)
stloc V_3
ldloc V_3
call void a::KeepA(native int)
ldc.i4.s 100
ret
}

.method public hidebysig static void KeepA(native int a) cil managed noinlining
{
.maxstack 8
ret
}
}

.class private auto ansi ctest
extends [mscorlib]System.Object
{
.method public specialname rtspecialname
instance void .ctor() cil managed
{
.maxstack 1
ldarg.0
call instance void [mscorlib]System.Object::.ctor()
ret
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
<Project Sdk="Microsoft.NET.Sdk.IL">
<PropertyGroup>
<OutputType>Exe</OutputType>
<DebugType>None</DebugType>
<Optimize>True</Optimize>
</PropertyGroup>
<ItemGroup>
<Compile Include="$(MSBuildProjectName).il" />
</ItemGroup>
</Project>

0 comments on commit 5ed389b

Please sign in to comment.