diff options
author | github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> | 2022-09-14 07:11:36 +0300 |
---|---|---|
committer | GitHub <noreply@github.com> | 2022-09-14 07:11:36 +0300 |
commit | 6297d411861944cc29590e1ecbe79a57c9e6a7fd (patch) | |
tree | 8d3d7a1fc509d793be6f31de576d0ed5867cbddd | |
parent | adbfdfa3050d069155d51286b91776574c40d0a9 (diff) |
[release/7.0] JIT: fix incorrect scale in genCreateAddrMode + no-opt (#75560)
* Fix invalid addressing mode scale in debug mode
* Add a test
* Update Runtime_75312.il
* Update Runtime_75312.ilproj
* Drop unneeded initialization
* Slightly better fix
* Test an even better fix?
* Refactoring
* address feedback
Co-authored-by: EgorBo <egorbo@gmail.com>
3 files changed, 86 insertions, 42 deletions
diff --git a/src/coreclr/jit/codegencommon.cpp b/src/coreclr/jit/codegencommon.cpp index 45cbd8ac6c0..7278506f86c 100644 --- a/src/coreclr/jit/codegencommon.cpp +++ b/src/coreclr/jit/codegencommon.cpp @@ -1370,65 +1370,46 @@ FOUND_AM: if (rv2) { - /* Make sure a GC address doesn't end up in 'rv2' */ - + // Make sure a GC address doesn't end up in 'rv2' if (varTypeIsGC(rv2->TypeGet())) { noway_assert(rv1 && !varTypeIsGC(rv1->TypeGet())); - - tmp = rv1; - rv1 = rv2; - rv2 = tmp; - + std::swap(rv1, rv2); rev = !rev; } - /* Special case: constant array index (that is range-checked) */ - + // Special case: constant array index (that is range-checked) if (fold) { - ssize_t tmpMul; - GenTree* index; + // By default, assume index is rv2 and indexScale is mul (or 1 if mul is zero) + GenTree* index = rv2; + ssize_t indexScale = mul == 0 ? 1 : mul; - if ((rv2->gtOper == GT_MUL || rv2->gtOper == GT_LSH) && (rv2->AsOp()->gtOp2->IsCnsIntOrI())) + if (rv2->OperIs(GT_MUL, GT_LSH) && (rv2->gtGetOp2()->IsCnsIntOrI())) { - /* For valuetype arrays where we can't use the scaled address - mode, rv2 will point to the scaled index. So we have to do - more work */ - - tmpMul = compiler->optGetArrayRefScaleAndIndex(rv2, &index DEBUGARG(false)); - if (mul) - { - tmpMul *= mul; - } + indexScale *= compiler->optGetArrayRefScaleAndIndex(rv2, &index DEBUGARG(false)); } - else - { - /* May be a simple array. rv2 will points to the actual index */ - index = rv2; - tmpMul = mul; + // "index * 0" means index is zero + if (indexScale == 0) + { + mul = 0; + rv2 = nullptr; } - - /* Get hold of the array index and see if it's a constant */ - if (index->IsIntCnsFitsInI32()) + else if (index->IsIntCnsFitsInI32()) { - /* Get hold of the index value */ - ssize_t ixv = index->AsIntConCommon()->IconValue(); - - /* Scale the index if necessary */ - if (tmpMul) + ssize_t constantIndex = index->AsIntConCommon()->IconValue() * indexScale; + if (constantIndex == 0) { - ixv *= tmpMul; + // while scale is a non-zero constant, the actual index is zero so drop it + mul = 0; + rv2 = nullptr; } - - if (FitsIn<INT32>(cns + ixv)) + else if (FitsIn<INT32>(cns + constantIndex)) { - /* Add the scaled index to the offset value */ - - cns += ixv; - - /* There is no scaled operand any more */ + // Add the constant index to the accumulated offset value + cns += constantIndex; + // and get rid of index mul = 0; rv2 = nullptr; } diff --git a/src/tests/JIT/Regression/JitBlue/Runtime_75312/Runtime_75312.il b/src/tests/JIT/Regression/JitBlue/Runtime_75312/Runtime_75312.il new file mode 100644 index 00000000000..ef3f9b67f82 --- /dev/null +++ b/src/tests/JIT/Regression/JitBlue/Runtime_75312/Runtime_75312.il @@ -0,0 +1,52 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +.assembly extern System.Runtime {} +.assembly extern System.Runtime.Extensions {} +.assembly extern System.Console {} +.assembly Runtime_75312 {} + +.class public abstract auto ansi sealed beforefieldinit Runtime_75312 + extends [System.Runtime]System.Object +{ + .method private hidebysig static + int32 Main () cil managed + { + .entrypoint + .maxstack 2 + .locals init ( + [0] int64 a + ) + ldc.i8 1234605616436508552 + stloc.0 + ldc.i4 1146447579 + ldloca.s 0 + conv.u + newobj instance void [System.Runtime]System.IntPtr::.ctor(void*) + call int32 [System.Runtime]System.IntPtr::op_Explicit(native int) + call int32 Runtime_75312::Test(int32) + sub + ret + } + + .method private hidebysig static int32 Test (int32 lcl) cil managed noinlining nooptimization + { + .maxstack 8 + + // return *(int*)(arg0 + ((3 * 0) << 2) + 1); + // to avoid constant folding in Roslyn (even for Debug) it's written in IL + + ldarg.0 + ldc.i4.3 + ldc.i4.0 + mul + ldc.i4.2 + shl + add + ldc.i4.1 + add + conv.i + ldind.i4 + ret + } +} diff --git a/src/tests/JIT/Regression/JitBlue/Runtime_75312/Runtime_75312.ilproj b/src/tests/JIT/Regression/JitBlue/Runtime_75312/Runtime_75312.ilproj new file mode 100644 index 00000000000..5e9fc16ea3a --- /dev/null +++ b/src/tests/JIT/Regression/JitBlue/Runtime_75312/Runtime_75312.ilproj @@ -0,0 +1,11 @@ +<Project Sdk="Microsoft.NET.Sdk.IL"> + <PropertyGroup> + <OutputType>Exe</OutputType> + <CLRTestTargetUnsupported Condition="'$(TargetBits)' != '32'">true</CLRTestTargetUnsupported> + <DebugType>None</DebugType> + <Optimize>True</Optimize> + </PropertyGroup> + <ItemGroup> + <Compile Include="$(MSBuildProjectName).il" /> + </ItemGroup> +</Project> |