From 796dc17708417d9edeb6f2b1d26ca529227d8694 Mon Sep 17 00:00:00 2001 From: Carol Eidt Date: Tue, 13 Feb 2018 14:35:34 -0800 Subject: [PATCH] Correctly handle check for busy double reg Fixes the second case in DevDev bug 543057 --- src/jit/lsra.cpp | 48 +++++++++----- .../JitBlue/DevDiv_543057a/DevDiv_543057a.cs | 74 ++++++++++++++++++++++ .../JitBlue/DevDiv_543057a/DevDiv_543057a.csproj | 37 +++++++++++ 3 files changed, 144 insertions(+), 15 deletions(-) create mode 100644 tests/src/JIT/Regression/JitBlue/DevDiv_543057a/DevDiv_543057a.cs create mode 100644 tests/src/JIT/Regression/JitBlue/DevDiv_543057a/DevDiv_543057a.csproj diff --git a/src/jit/lsra.cpp b/src/jit/lsra.cpp index 129b857..276f3a4 100644 --- a/src/jit/lsra.cpp +++ b/src/jit/lsra.cpp @@ -3358,8 +3358,10 @@ bool LinearScan::isRefPositionActive(RefPosition* refPosition, LsraLocation refL // False - otherwise // // Notes: -// This helper is designed to be used only from allocateBusyReg(). -// The caller must have already checked for the case where 'refPosition' is a fixed ref. +// This helper is designed to be used only from allocateBusyReg(), where: +// - This register was *not* found when looking for a free register, and +// - The caller must have already checked for the case where 'refPosition' is a fixed ref +// (asserted at the beginning of this method). // bool LinearScan::isRegInUse(RegRecord* regRec, RefPosition* refPosition) { @@ -3370,27 +3372,43 @@ bool LinearScan::isRegInUse(RegRecord* regRec, RefPosition* refPosition) { if (!assignedInterval->isActive) { - // This can only happen if we have a recentRefPosition active at this location that hasn't yet been freed - // (Or, in the case of ARM, the other half of a double is either active or has an active recentRefPosition). + // This can only happen if we have a recentRefPosition active at this location that hasn't yet been freed. CLANG_FORMAT_COMMENT_ANCHOR; -#ifdef _TARGET_ARM_ - if (refPosition->getInterval()->registerType == TYP_DOUBLE) + if (isRefPositionActive(assignedInterval->recentRefPosition, refPosition->nodeLocation)) { - if (!isRefPositionActive(assignedInterval->recentRefPosition, refPosition->nodeLocation)) + return true; + } + else + { +#ifdef _TARGET_ARM_ + // In the case of TYP_DOUBLE, we may have the case where 'assignedInterval' is inactive, + // but the other half register is active. If so, it must be have an active recentRefPosition, + // as above. + if (refPosition->getInterval()->registerType == TYP_DOUBLE) { RegRecord* otherHalfRegRec = findAnotherHalfRegRec(regRec); - assert(otherHalfRegRec->assignedInterval->isActive || - isRefPositionActive(otherHalfRegRec->assignedInterval->recentRefPosition, - refPosition->nodeLocation)); + if (!otherHalfRegRec->assignedInterval->isActive) + { + if (isRefPositionActive(otherHalfRegRec->assignedInterval->recentRefPosition, + refPosition->nodeLocation)) + { + return true; + } + else + { + assert(!"Unexpected inactive assigned interval in isRegInUse"); + return true; + } + } } - } - else + else #endif - { - assert(isRefPositionActive(assignedInterval->recentRefPosition, refPosition->nodeLocation)); + { + assert(!"Unexpected inactive assigned interval in isRegInUse"); + return true; + } } - return true; } RefPosition* nextAssignedRef = assignedInterval->getNextRefPosition(); diff --git a/tests/src/JIT/Regression/JitBlue/DevDiv_543057a/DevDiv_543057a.cs b/tests/src/JIT/Regression/JitBlue/DevDiv_543057a/DevDiv_543057a.cs new file mode 100644 index 0000000..cfa907a --- /dev/null +++ b/tests/src/JIT/Regression/JitBlue/DevDiv_543057a/DevDiv_543057a.cs @@ -0,0 +1,74 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. +// See the LICENSE file in the project root for more information. +// + +// The bug captured by this test was a case where: +// - We have COMPlus_JitStressRegs=3, so we're limiting the available registers. +// - We have a DIV with two double operands that are casts from int lclVars, and it is passed to a call. +// - We have 4 float lclVars in registers: +// - One is active in a caller-save register (that will be x in our case) +// - One is active in a callee-save register (y) +// - One is inactive in a caller-save register (z) +// - One is inactive in a callee-save register (w) +// - When we allocate the def (target) register for the second cast, we spill the first one. +// - When we try to reload it, we were incorrectly returning false from 'isRegInUse()' +// for the inactive interval in the second half. + +using System; +using System.Runtime.CompilerServices; + +public class DevDiv_543057 + +{ + public const int Pass = 100; + public const int Fail = -1; + + [MethodImplAttribute(MethodImplOptions.NoInlining)] + public static float GetFloat(int i) + { + return (float)i; + } + [MethodImplAttribute(MethodImplOptions.NoInlining)] + public static int test(int i, int j) + { + // x and y will be preferenced to callee-save + // they need to have enough references to qualify for callee-save + // z and w will not + // x and z will be inactive at the call to Math.Ceiling + // y and w will be active + float x = GetFloat(1); + float y = GetFloat(2); + float z = GetFloat(3); + // We want w in f1. At this point z is likely to be in f0. + // So define 'result' first. + float result = x - y; + float w = x + y + z; + if (i != j) + { + // Here all our floats are in registers. z is going to be redefined, so it is inactive, + // and w is not used except in the else clause so it is also inactive. + z = (float) Math.Ceiling(((double)i) / ((double)j)); + // Now we use x and y so that they are live across the call to Math.Ceiling. + result = z + y + w; + } + else + { + // Here we need to use all of our float arguments so that they are all live. + y = x * y * z * w; + x = y * 2; + // Now x and y are going to be live across this call, to encourage them to get a callee-save reg. + Console.WriteLine("FAIL"); + // And use x a couple more times. + x *= y; + result += x + y; + } + Console.WriteLine("Result: " + result); + return Pass; + } + public static int Main() + { + return test(5, 6); + } +} + diff --git a/tests/src/JIT/Regression/JitBlue/DevDiv_543057a/DevDiv_543057a.csproj b/tests/src/JIT/Regression/JitBlue/DevDiv_543057a/DevDiv_543057a.csproj new file mode 100644 index 0000000..6d58ab0 --- /dev/null +++ b/tests/src/JIT/Regression/JitBlue/DevDiv_543057a/DevDiv_543057a.csproj @@ -0,0 +1,37 @@ + + + + + Debug + AnyCPU + $(MSBuildProjectName) + 2.0 + {95DFC527-4DC1-495E-97D7-E94EE1F7140D} + Exe + {786C830F-07A1-408B-BD7F-6EE04809D6DB};{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC} + ..\..\ + + + + + + + + + False + + + + + True + + + + + + + + + + + -- 2.7.4