From 6297d411861944cc29590e1ecbe79a57c9e6a7fd Mon Sep 17 00:00:00 2001 From: "github-actions[bot]" <41898282+github-actions[bot]@users.noreply.github.com> Date: Tue, 13 Sep 2022 21:11:36 -0700 Subject: [PATCH] [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 --- src/coreclr/jit/codegencommon.cpp | 65 +++++++------------ .../JitBlue/Runtime_75312/Runtime_75312.il | 52 +++++++++++++++ .../Runtime_75312/Runtime_75312.ilproj | 11 ++++ 3 files changed, 86 insertions(+), 42 deletions(-) create mode 100644 src/tests/JIT/Regression/JitBlue/Runtime_75312/Runtime_75312.il create mode 100644 src/tests/JIT/Regression/JitBlue/Runtime_75312/Runtime_75312.ilproj diff --git a/src/coreclr/jit/codegencommon.cpp b/src/coreclr/jit/codegencommon.cpp index 45cbd8ac6c073..7278506f86cf6 100644 --- a/src/coreclr/jit/codegencommon.cpp +++ b/src/coreclr/jit/codegencommon.cpp @@ -1370,65 +1370,46 @@ bool CodeGen::genCreateAddrMode( 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(cns + ixv)) + else if (FitsIn(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 0000000000000..ef3f9b67f8293 --- /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 0000000000000..5e9fc16ea3a67 --- /dev/null +++ b/src/tests/JIT/Regression/JitBlue/Runtime_75312/Runtime_75312.ilproj @@ -0,0 +1,11 @@ + + + Exe + true + None + True + + + + +