From 2ca7494048fbe2994f719e9000a9be8eae9a8fca Mon Sep 17 00:00:00 2001 From: Carol Eidt Date: Wed, 21 Nov 2018 14:12:03 -0800 Subject: [PATCH] Use LclFld for full-width cpblk of different types This issue arose because the source tree was a struct with a single double field, and the destination was a struct with a single long field. Copy prop replaced the lclVar in the source, which was under a `IND(ADDR)` with a lclVar that was not address taken, although it was marked `GTF_DONT_CSE`. Replacing the src tree with a `GT_LCL_FLD` addresses this issue. Also, ensure that the target is marked `DoNotEnregister` if it isn't referenced as the same size/type. Fix dotnet/coreclr#21080 Fix dotnet/coreclr#21064 Commit migrated from https://github.com/dotnet/coreclr/commit/0c08dc461f4dd7ac418a475264129d2c57536955 --- src/coreclr/src/jit/morph.cpp | 57 +++++++++++++++++++++++++++++++--------- src/coreclr/tests/issues.targets | 15 ----------- 2 files changed, 44 insertions(+), 28 deletions(-) diff --git a/src/coreclr/src/jit/morph.cpp b/src/coreclr/src/jit/morph.cpp index fb7ab00..5b3cc72 100644 --- a/src/coreclr/src/jit/morph.cpp +++ b/src/coreclr/src/jit/morph.cpp @@ -10309,13 +10309,11 @@ GenTree* Compiler::fgMorphCopyBlock(GenTree* tree) JITDUMP(requiresCopyBlock ? " this requires a CopyBlock.\n" : " using field by field assignments.\n"); - // Mark the dest/src structs as DoNotEnreg - // when they are not reg-sized non-field-addressed structs and we are using a CopyBlock - // or the struct is not promoted + // Mark the dest/src structs as DoNotEnreg when they are not being fully referenced as the same type. // if (!destDoFldAsg && (destLclVar != nullptr) && !destSingleLclVarAsg) { - if (!destLclVar->lvRegStruct) + if (!destLclVar->lvRegStruct || (destLclVar->lvType != dest->TypeGet())) { // Mark it as DoNotEnregister. lvaSetVarDoNotEnregister(destLclNum DEBUGARG(DNER_BlockOp)); @@ -10526,6 +10524,8 @@ GenTree* Compiler::fgMorphCopyBlock(GenTree* tree) _AssignFields: + // We may have allocated a temp above, and that may have caused the lvaTable to be expanded. + // So, beyond this point we cannot rely on the old values of 'srcLclVar' and 'destLclVar'. for (unsigned i = 0; i < fieldCnt; ++i) { FieldSeqNode* curFieldSeq = nullptr; @@ -10627,10 +10627,10 @@ GenTree* Compiler::fgMorphCopyBlock(GenTree* tree) if (srcSingleLclVarAsg) { noway_assert(fieldCnt == 1); - noway_assert(srcLclVar != nullptr); + noway_assert(srcLclNum != BAD_VAR_NUM); noway_assert(addrSpill == nullptr); - src = gtNewLclvNode(srcLclNum, srcLclVar->TypeGet()); + src = gtNewLclvNode(srcLclNum, lvaGetDesc(srcLclNum)->TypeGet()); } else { @@ -10648,13 +10648,44 @@ GenTree* Compiler::fgMorphCopyBlock(GenTree* tree) CORINFO_CLASS_HANDLE classHnd = lvaTable[destLclNum].lvVerTypeInfo.GetClassHandle(); CORINFO_FIELD_HANDLE fieldHnd = info.compCompHnd->getFieldInClass(classHnd, lvaTable[fieldLclNum].lvFldOrdinal); - curFieldSeq = GetFieldSeqStore()->CreateSingleton(fieldHnd); + curFieldSeq = GetFieldSeqStore()->CreateSingleton(fieldHnd); + var_types destType = lvaGetDesc(fieldLclNum)->lvType; - src = gtNewOperNode(GT_ADD, TYP_BYREF, src, - new (this, GT_CNS_INT) - GenTreeIntCon(TYP_I_IMPL, lvaTable[fieldLclNum].lvFldOffset, curFieldSeq)); - - src = gtNewIndir(lvaTable[fieldLclNum].TypeGet(), src); + bool done = false; + if (lvaGetDesc(fieldLclNum)->lvFldOffset == 0) + { + // If this is a full-width use of the src via a different type, we need to create a GT_LCL_FLD. + // (Note that if it was the same type, 'srcSingleLclVarAsg' would be true.) + if (srcLclNum != BAD_VAR_NUM) + { + noway_assert(srcLclVarTree != nullptr); + assert(destType != TYP_STRUCT); + unsigned destSize = genTypeSize(destType); + srcLclVar = lvaGetDesc(srcLclNum); + unsigned srcSize = + (srcLclVar->lvType == TYP_STRUCT) ? srcLclVar->lvExactSize : genTypeSize(srcLclVar); + if (destSize == srcSize) + { + srcLclVarTree->gtFlags |= GTF_VAR_CAST; + srcLclVarTree->ChangeOper(GT_LCL_FLD); + srcLclVarTree->gtType = destType; + srcLclVarTree->AsLclFld()->gtFieldSeq = curFieldSeq; + src = srcLclVarTree; + done = true; + } + } + } + else // if (lvaGetDesc(fieldLclNum)->lvFldOffset != 0) + { + src = gtNewOperNode(GT_ADD, TYP_BYREF, src, + new (this, GT_CNS_INT) + GenTreeIntCon(TYP_I_IMPL, lvaGetDesc(fieldLclNum)->lvFldOffset, + curFieldSeq)); + } + if (!done) + { + src = gtNewIndir(destType, src); + } } } @@ -10667,7 +10698,7 @@ GenTree* Compiler::fgMorphCopyBlock(GenTree* tree) // exposed. Neither liveness nor SSA are able to track this kind of indirect assignments. if (addrSpill && !destDoFldAsg && destLclNum != BAD_VAR_NUM) { - noway_assert(lvaTable[destLclNum].lvAddrExposed); + noway_assert(lvaGetDesc(destLclNum)->lvAddrExposed); } #if LOCAL_ASSERTION_PROP diff --git a/src/coreclr/tests/issues.targets b/src/coreclr/tests/issues.targets index 41b66b6..5fe1694 100644 --- a/src/coreclr/tests/issues.targets +++ b/src/coreclr/tests/issues.targets @@ -199,9 +199,6 @@ 19537 - - 21064 - @@ -245,18 +242,6 @@ 19441 - - 21064 - - - 21064 - - - 21064 - - - 21064 - -- 2.7.4