From 665ffb2fd55dd9a1a66886f9c5e245c8659ac4e2 Mon Sep 17 00:00:00 2001 From: Andy Ayers Date: Tue, 30 Oct 2018 11:24:37 -0700 Subject: [PATCH] JIT: Fix call flag propagation for GenTreeArrElem (dotnet/coreclr#20660) Closes dotnet/coreclr#20651. Also fix up some "near miss" cases for GenTreeField and GenTreeBoundsCheck, where we get lucky and the importer currently splits trees with temps so the currently ignored child nodes have no interesting side effects. Revise GenTreeField a bit to pull more of the initialization work into the constructor. Add a missing R2R field propagation for field nodes in GtClone (evidently also never hit in practice). Commit migrated from https://github.com/dotnet/coreclr/commit/626def86af589734999a0e62d3a83c2acb9de54a --- src/coreclr/src/jit/compiler.hpp | 13 ++------ src/coreclr/src/jit/gentree.cpp | 3 ++ src/coreclr/src/jit/gentree.h | 14 ++++++-- .../JitBlue/GitHub_20651/GitHub_20651.cs | 25 ++++++++++++++ .../JitBlue/GitHub_20651/GitHub_20651.csproj | 39 ++++++++++++++++++++++ 5 files changed, 81 insertions(+), 13 deletions(-) create mode 100644 src/coreclr/tests/src/JIT/Regression/JitBlue/GitHub_20651/GitHub_20651.cs create mode 100644 src/coreclr/tests/src/JIT/Regression/JitBlue/GitHub_20651/GitHub_20651.csproj diff --git a/src/coreclr/src/jit/compiler.hpp b/src/coreclr/src/jit/compiler.hpp index b4de24a..f8fc942 100644 --- a/src/coreclr/src/jit/compiler.hpp +++ b/src/coreclr/src/jit/compiler.hpp @@ -1208,19 +1208,10 @@ inline GenTree* Compiler::gtNewFieldRef(var_types typ, CORINFO_FIELD_HANDLE fldH { #if SMALL_TREE_NODES /* 'GT_FIELD' nodes may later get transformed into 'GT_IND' */ - assert(GenTree::s_gtNodeSizes[GT_IND] <= GenTree::s_gtNodeSizes[GT_FIELD]); - GenTree* tree = new (this, GT_FIELD) GenTreeField(typ); -#else - GenTree* tree = new (this, GT_FIELD) GenTreeField(typ); -#endif - tree->gtField.gtFldObj = obj; - tree->gtField.gtFldHnd = fldHnd; - tree->gtField.gtFldOffset = offset; +#endif // SMALL_TREE_NODES -#ifdef FEATURE_READYTORUN_COMPILER - tree->gtField.gtFieldLookup.addr = nullptr; -#endif + GenTree* tree = new (this, GT_FIELD) GenTreeField(typ, obj, fldHnd, offset); // If "obj" is the address of a local, note that a field of that struct local has been accessed. if (obj != nullptr && obj->OperGet() == GT_ADDR && varTypeIsStruct(obj->gtOp.gtOp1) && diff --git a/src/coreclr/src/jit/gentree.cpp b/src/coreclr/src/jit/gentree.cpp index 2fed901..c5cf957 100644 --- a/src/coreclr/src/jit/gentree.cpp +++ b/src/coreclr/src/jit/gentree.cpp @@ -6768,6 +6768,9 @@ GenTree* Compiler::gtClone(GenTree* tree, bool complexOK) copy = gtNewFieldRef(tree->TypeGet(), tree->gtField.gtFldHnd, objp, tree->gtField.gtFldOffset); copy->gtField.gtFldMayOverlap = tree->gtField.gtFldMayOverlap; +#ifdef FEATURE_READYTORUN_COMPILER + copy->gtField.gtFieldLookup = tree->gtField.gtFieldLookup; +#endif } else if (tree->OperIs(GT_ADD, GT_SUB)) { diff --git a/src/coreclr/src/jit/gentree.h b/src/coreclr/src/jit/gentree.h index 3aa7521..c6c5841 100644 --- a/src/coreclr/src/jit/gentree.h +++ b/src/coreclr/src/jit/gentree.h @@ -2937,9 +2937,17 @@ struct GenTreeField : public GenTree CORINFO_CONST_LOOKUP gtFieldLookup; #endif - GenTreeField(var_types type) : GenTree(GT_FIELD, type) + GenTreeField(var_types type, GenTree* obj, CORINFO_FIELD_HANDLE fldHnd, DWORD offs) + : GenTree(GT_FIELD, type), gtFldObj(obj), gtFldHnd(fldHnd), gtFldOffset(offs), gtFldMayOverlap(false) { - gtFldMayOverlap = false; + if (obj != nullptr) + { + gtFlags |= (obj->gtFlags & GTF_ALL_EFFECT); + } + +#ifdef FEATURE_READYTORUN_COMPILER + gtFieldLookup.addr = nullptr; +#endif } #if DEBUGGABLE_GENTREE GenTreeField() : GenTree() @@ -4323,6 +4331,7 @@ struct GenTreeBoundsChk : public GenTree : GenTree(oper, type), gtIndex(index), gtArrLen(arrLen), gtIndRngFailBB(nullptr), gtThrowKind(kind) { // Effects flags propagate upwards. + gtFlags |= (index->gtFlags & GTF_ALL_EFFECT); gtFlags |= (arrLen->gtFlags & GTF_ALL_EFFECT); gtFlags |= GTF_EXCEPT; } @@ -4369,6 +4378,7 @@ struct GenTreeArrElem : public GenTree var_types type, GenTree* arr, unsigned char rank, unsigned char elemSize, var_types elemType, GenTree** inds) : GenTree(GT_ARR_ELEM, type), gtArrObj(arr), gtArrRank(rank), gtArrElemSize(elemSize), gtArrElemType(elemType) { + gtFlags |= (arr->gtFlags & GTF_ALL_EFFECT); for (unsigned char i = 0; i < rank; i++) { gtArrInds[i] = inds[i]; diff --git a/src/coreclr/tests/src/JIT/Regression/JitBlue/GitHub_20651/GitHub_20651.cs b/src/coreclr/tests/src/JIT/Regression/JitBlue/GitHub_20651/GitHub_20651.cs new file mode 100644 index 0000000..81a532d --- /dev/null +++ b/src/coreclr/tests/src/JIT/Regression/JitBlue/GitHub_20651/GitHub_20651.cs @@ -0,0 +1,25 @@ +// 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. + +using System; + +class X +{ + static string s = "hello, world"; + + static string[,] G() + { + string[,] strings = new string[3,3]; + strings[0,0] = s; + return strings; + } + + // Ensure GTF_CALL flag is propagated to MD array accessor + public static int Main() + { + int c = G()[0,0].GetHashCode(); + int v = s.GetHashCode(); + return c == v ? 100 : -1; + } +} diff --git a/src/coreclr/tests/src/JIT/Regression/JitBlue/GitHub_20651/GitHub_20651.csproj b/src/coreclr/tests/src/JIT/Regression/JitBlue/GitHub_20651/GitHub_20651.csproj new file mode 100644 index 0000000..52a913f --- /dev/null +++ b/src/coreclr/tests/src/JIT/Regression/JitBlue/GitHub_20651/GitHub_20651.csproj @@ -0,0 +1,39 @@ + + + + + Debug + AnyCPU + $(MSBuildProjectName) + 2.0 + {95DFC527-4DC1-495E-97D7-E94EE1F7140D} + Exe + Properties + 512 + {786C830F-07A1-408B-BD7F-6EE04809D6DB};{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC} + $(ProgramFiles)\Common Files\microsoft shared\VSTT\11.0\UITestExtensionPackages + ..\..\ + 7a9bfb7d + 1 + + + + + + + False + + + + + True + + + + + + + + + + \ No newline at end of file -- 2.7.4