From b27167e3e89d51cc6901275eb68289631ae72aa4 Mon Sep 17 00:00:00 2001 From: Mike Danes Date: Sat, 1 Dec 2018 15:35:55 +0200 Subject: Fix lclvar "cloning" in const division lowering The existing code was creating new uses of existing variables using the type of the division operation instead of the type of the existing variable use. There is one rare case when this does not work properly: * if the variable is "normalize on load" (that implies that it is also a small int type variable) * and if the variable is previously assigned to such that a subrange assertion is generated * and if morph decides not to insert the load normalization cast due to the subrange assertion * and if the variable is not enregistered Then the assignment stores 2 bytes to the stack location but the uses generated by the division lowering code read 4 bytes because they have TYP_INT. --- src/jit/lower.cpp | 46 +++++++++------------ src/jit/lower.h | 10 ++--- .../JitBlue/Github_19558/Github_19558.cs | 47 ++++++++++++++++++++++ .../JitBlue/Github_19558/Github_19558.csproj | 33 +++++++++++++++ 4 files changed, 105 insertions(+), 31 deletions(-) create mode 100644 tests/src/JIT/Regression/JitBlue/Github_19558/Github_19558.cs create mode 100644 tests/src/JIT/Regression/JitBlue/Github_19558/Github_19558.csproj diff --git a/src/jit/lower.cpp b/src/jit/lower.cpp index 9e4db125d8..f7bd292dfa 100644 --- a/src/jit/lower.cpp +++ b/src/jit/lower.cpp @@ -4567,13 +4567,11 @@ bool Lowering::LowerUnsignedDivOrMod(GenTreeOp* divMod) const bool requiresAdjustment = add; const bool requiresDividendMultiuse = requiresAdjustment || !isDiv; const unsigned curBBWeight = m_block->getBBWeight(comp); - unsigned dividendLclNum = BAD_VAR_NUM; if (requiresDividendMultiuse) { LIR::Use dividendUse(BlockRange(), &divMod->gtOp1, divMod); - dividendLclNum = ReplaceWithLclVar(dividendUse); - dividend = divMod->gtGetOp1(); + dividend = ReplaceWithLclVar(dividendUse); } // Insert a new GT_MULHI node before the existing GT_UDIV/GT_UMOD node. @@ -4588,8 +4586,8 @@ bool Lowering::LowerUnsignedDivOrMod(GenTreeOp* divMod) if (requiresAdjustment) { - GenTree* dividend = comp->gtNewLclvNode(dividendLclNum, type); - GenTree* sub = comp->gtNewOperNode(GT_SUB, type, dividend, mulhi); + dividend = comp->gtNewLclvNode(dividend->AsLclVar()->GetLclNum(), dividend->TypeGet()); + GenTree* sub = comp->gtNewOperNode(GT_SUB, type, dividend, mulhi); BlockRange().InsertBefore(divMod, dividend, sub); GenTree* one = comp->gtNewIconNode(1, TYP_INT); @@ -4597,11 +4595,11 @@ bool Lowering::LowerUnsignedDivOrMod(GenTreeOp* divMod) BlockRange().InsertBefore(divMod, one, rsz); LIR::Use mulhiUse(BlockRange(), &sub->gtOp.gtOp2, sub); - unsigned mulhiLclNum = ReplaceWithLclVar(mulhiUse); + mulhi = ReplaceWithLclVar(mulhiUse); - GenTree* mulhiCopy = comp->gtNewLclvNode(mulhiLclNum, type); - GenTree* add = comp->gtNewOperNode(GT_ADD, type, rsz, mulhiCopy); - BlockRange().InsertBefore(divMod, mulhiCopy, add); + mulhi = comp->gtNewLclvNode(mulhi->AsLclVar()->GetLclNum(), mulhi->TypeGet()); + GenTree* add = comp->gtNewOperNode(GT_ADD, type, rsz, mulhi); + BlockRange().InsertBefore(divMod, mulhi, add); mulhi = add; shift -= 1; @@ -4621,9 +4619,9 @@ bool Lowering::LowerUnsignedDivOrMod(GenTreeOp* divMod) GenTree* div = comp->gtNewOperNode(GT_RSZ, type, mulhi, shiftBy); // divisor UMOD dividend = dividend SUB (div MUL divisor) - GenTree* divisor = comp->gtNewIconNode(divisorValue, type); - GenTree* mul = comp->gtNewOperNode(GT_MUL, type, div, divisor); - GenTree* dividend = comp->gtNewLclvNode(dividendLclNum, type); + GenTree* divisor = comp->gtNewIconNode(divisorValue, type); + GenTree* mul = comp->gtNewOperNode(GT_MUL, type, div, divisor); + dividend = comp->gtNewLclvNode(dividend->AsLclVar()->GetLclNum(), dividend->TypeGet()); divMod->SetOper(GT_SUB); divMod->gtOp1 = dividend; @@ -4760,19 +4758,18 @@ GenTree* Lowering::LowerConstIntDivOrMod(GenTree* node) bool requiresShiftAdjust = shift != 0; bool requiresDividendMultiuse = requiresAddSubAdjust || !isDiv; unsigned curBBWeight = comp->compCurBB->getBBWeight(comp); - unsigned dividendLclNum = BAD_VAR_NUM; if (requiresDividendMultiuse) { LIR::Use dividendUse(BlockRange(), &mulhi->gtOp.gtOp2, mulhi); - dividendLclNum = ReplaceWithLclVar(dividendUse); + dividend = ReplaceWithLclVar(dividendUse); } GenTree* adjusted; if (requiresAddSubAdjust) { - dividend = comp->gtNewLclvNode(dividendLclNum, type); + dividend = comp->gtNewLclvNode(dividend->AsLclVar()->GetLclNum(), dividend->TypeGet()); adjusted = comp->gtNewOperNode(divisorValue > 0 ? GT_ADD : GT_SUB, type, mulhi, dividend); BlockRange().InsertBefore(divMod, dividend, adjusted); } @@ -4786,8 +4783,8 @@ GenTree* Lowering::LowerConstIntDivOrMod(GenTree* node) BlockRange().InsertBefore(divMod, shiftBy, signBit); LIR::Use adjustedUse(BlockRange(), &signBit->gtOp.gtOp1, signBit); - unsigned adjustedLclNum = ReplaceWithLclVar(adjustedUse); - adjusted = comp->gtNewLclvNode(adjustedLclNum, type); + adjusted = ReplaceWithLclVar(adjustedUse); + adjusted = comp->gtNewLclvNode(adjusted->AsLclVar()->GetLclNum(), adjusted->TypeGet()); BlockRange().InsertBefore(divMod, adjusted); if (requiresShiftAdjust) @@ -4807,7 +4804,7 @@ GenTree* Lowering::LowerConstIntDivOrMod(GenTree* node) { GenTree* div = comp->gtNewOperNode(GT_ADD, type, adjusted, signBit); - dividend = comp->gtNewLclvNode(dividendLclNum, type); + dividend = comp->gtNewLclvNode(dividend->AsLclVar()->GetLclNum(), dividend->TypeGet()); // divisor % dividend = dividend - divisor x div GenTree* divisor = comp->gtNewIconNode(divisorValue, type); @@ -4841,12 +4838,7 @@ GenTree* Lowering::LowerConstIntDivOrMod(GenTree* node) unsigned curBBWeight = comp->compCurBB->getBBWeight(comp); LIR::Use opDividend(BlockRange(), &divMod->gtOp.gtOp1, divMod); - ReplaceWithLclVar(opDividend); - - dividend = divMod->gtGetOp1(); - assert(dividend->OperGet() == GT_LCL_VAR); - - unsigned dividendLclNum = dividend->gtLclVar.gtLclNum; + dividend = ReplaceWithLclVar(opDividend); GenTree* adjustment = comp->gtNewOperNode(GT_RSH, type, dividend, comp->gtNewIconNode(type == TYP_INT ? 31 : 63)); @@ -4862,7 +4854,8 @@ GenTree* Lowering::LowerConstIntDivOrMod(GenTree* node) } GenTree* adjustedDividend = - comp->gtNewOperNode(GT_ADD, type, adjustment, comp->gtNewLclvNode(dividendLclNum, type)); + comp->gtNewOperNode(GT_ADD, type, adjustment, + comp->gtNewLclvNode(dividend->AsLclVar()->GetLclNum(), dividend->TypeGet())); GenTree* newDivMod; @@ -4888,7 +4881,8 @@ GenTree* Lowering::LowerConstIntDivOrMod(GenTree* node) // which simply discards the low log2(divisor) bits, that's just dividend & ~(divisor - 1) divisor->gtIntCon.SetIconValue(~(absDivisorValue - 1)); - newDivMod = comp->gtNewOperNode(GT_SUB, type, comp->gtNewLclvNode(dividendLclNum, type), + newDivMod = comp->gtNewOperNode(GT_SUB, type, + comp->gtNewLclvNode(dividend->AsLclVar()->GetLclNum(), dividend->TypeGet()), comp->gtNewOperNode(GT_AND, type, adjustedDividend, divisor)); } diff --git a/src/jit/lower.h b/src/jit/lower.h index 6dbad630d9..e29bb9c4d6 100644 --- a/src/jit/lower.h +++ b/src/jit/lower.h @@ -198,18 +198,18 @@ private: } // Replace the definition of the given use with a lclVar, allocating a new temp - // if 'tempNum' is BAD_VAR_NUM. - unsigned ReplaceWithLclVar(LIR::Use& use, unsigned tempNum = BAD_VAR_NUM) + // if 'tempNum' is BAD_VAR_NUM. Returns the LclVar node. + GenTreeLclVar* ReplaceWithLclVar(LIR::Use& use, unsigned tempNum = BAD_VAR_NUM) { GenTree* oldUseNode = use.Def(); if ((oldUseNode->gtOper != GT_LCL_VAR) || (tempNum != BAD_VAR_NUM)) { - unsigned newLclNum = use.ReplaceWithLclVar(comp, m_block->getBBWeight(comp), tempNum); + use.ReplaceWithLclVar(comp, m_block->getBBWeight(comp), tempNum); GenTree* newUseNode = use.Def(); ContainCheckRange(oldUseNode->gtNext, newUseNode); - return newLclNum; + return newUseNode->AsLclVar(); } - return oldUseNode->AsLclVarCommon()->gtLclNum; + return oldUseNode->AsLclVar(); } // return true if this call target is within range of a pc-rel call on the machine diff --git a/tests/src/JIT/Regression/JitBlue/Github_19558/Github_19558.cs b/tests/src/JIT/Regression/JitBlue/Github_19558/Github_19558.cs new file mode 100644 index 0000000000..f30480cb5c --- /dev/null +++ b/tests/src/JIT/Regression/JitBlue/Github_19558/Github_19558.cs @@ -0,0 +1,47 @@ +// 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. + +struct S0 +{ + public uint F1; + public long F2; + public ulong F3; + public ushort F5; + public bool F7; + public S0(uint f1) : this() + { + F1 = f1; + } +} + +public class Program +{ + static S0[] s_7 = new S0[] { new S0(0) }; + + public static int Main() + { + return (M9(0) == -1) ? 100 : 1; + } + + static short M9(short arg1) + { + long var0 = 0; + try + { + System.GC.KeepAlive(var0); + } + finally + { + var vr12 = new ulong[] { 0, 2271009908085114245UL }; + S0[] vr18 = new S0[] { new S0(32768) }; + uint vr19 = vr18[0].F1; + arg1 = (short)vr19; + arg1 %= -32767; + System.GC.KeepAlive(s_7[0]); + System.GC.KeepAlive(s_7[0]); + } + + return arg1; + } +} diff --git a/tests/src/JIT/Regression/JitBlue/Github_19558/Github_19558.csproj b/tests/src/JIT/Regression/JitBlue/Github_19558/Github_19558.csproj new file mode 100644 index 0000000000..42f8a01f39 --- /dev/null +++ b/tests/src/JIT/Regression/JitBlue/Github_19558/Github_19558.csproj @@ -0,0 +1,33 @@ + + + + + Debug + AnyCPU + 2.0 + {2649FAFE-07BF-4F93-8120-BA9A69285ABB} + Exe + {786C830F-07A1-408B-BD7F-6EE04809D6DB};{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC} + ..\..\ + + + + + + None + True + + + + False + + + + + + + + + + + \ No newline at end of file -- cgit v1.2.3