From 95268ce168ef2432d696d6a1ad01bacf040ffb1b Mon Sep 17 00:00:00 2001 From: SingleAccretion Date: Sat, 22 Oct 2022 22:53:50 +0300 Subject: [PATCH 01/13] Introduce GT_FIELD_ADDR --- src/coreclr/jit/compiler.h | 4 ++++ src/coreclr/jit/compiler.hpp | 1 + src/coreclr/jit/gentree.cpp | 27 ++++++++++++--------------- src/coreclr/jit/gentree.h | 5 +++-- src/coreclr/jit/gtlist.h | 3 ++- src/coreclr/jit/gtstructs.h | 2 +- 6 files changed, 23 insertions(+), 19 deletions(-) diff --git a/src/coreclr/jit/compiler.h b/src/coreclr/jit/compiler.h index 05dfb0bfe30111..2aa7ff203096d4 100644 --- a/src/coreclr/jit/compiler.h +++ b/src/coreclr/jit/compiler.h @@ -2598,6 +2598,9 @@ class Compiler GenTreeField* gtNewFieldRef(var_types type, CORINFO_FIELD_HANDLE fldHnd, GenTree* obj = nullptr, DWORD offset = 0); + GenTreeField* gtNewFieldAddrNode( + var_types type, CORINFO_FIELD_HANDLE fldHnd, GenTree* obj = nullptr, DWORD offset = 0); + GenTreeIndexAddr* gtNewIndexAddr(GenTree* arrayOp, GenTree* indexOp, var_types elemType, @@ -10744,6 +10747,7 @@ class GenTreeVisitor case GT_RETURNTRAP: case GT_NOP: case GT_FIELD: + case GT_FIELD_ADDR: case GT_RETURN: case GT_RETFILT: case GT_RUNTIMELOOKUP: diff --git a/src/coreclr/jit/compiler.hpp b/src/coreclr/jit/compiler.hpp index 274514bbf9831f..f683770e8f8ccf 100644 --- a/src/coreclr/jit/compiler.hpp +++ b/src/coreclr/jit/compiler.hpp @@ -4130,6 +4130,7 @@ void GenTree::VisitOperands(TVisitor visitor) // Unary operators with an optional operand case GT_NOP: case GT_FIELD: + case GT_FIELD_ADDR: case GT_RETURN: case GT_RETFILT: if (this->AsUnOp()->gtOp1 == nullptr) diff --git a/src/coreclr/jit/gentree.cpp b/src/coreclr/jit/gentree.cpp index a513dc7eeeca88..c046a06819579a 100644 --- a/src/coreclr/jit/gentree.cpp +++ b/src/coreclr/jit/gentree.cpp @@ -265,6 +265,7 @@ void GenTree::InitNodeSize() GenTree::s_gtNodeSizes[GT_ARR_OFFSET] = TREE_NODE_SZ_LARGE; GenTree::s_gtNodeSizes[GT_RET_EXPR] = TREE_NODE_SZ_LARGE; GenTree::s_gtNodeSizes[GT_FIELD] = TREE_NODE_SZ_LARGE; + GenTree::s_gtNodeSizes[GT_FIELD_ADDR] = TREE_NODE_SZ_LARGE; GenTree::s_gtNodeSizes[GT_CMPXCHG] = TREE_NODE_SZ_LARGE; GenTree::s_gtNodeSizes[GT_QMARK] = TREE_NODE_SZ_LARGE; GenTree::s_gtNodeSizes[GT_STORE_DYN_BLK] = TREE_NODE_SZ_LARGE; @@ -2319,9 +2320,6 @@ bool GenTree::Compare(GenTree* op1, GenTree* op2, bool swapOK) genTreeOps oper; unsigned kind; -// printf("tree1:\n"); gtDispTree(op1); -// printf("tree2:\n"); gtDispTree(op2); - AGAIN: if (op1 == nullptr) @@ -2491,6 +2489,7 @@ bool GenTree::Compare(GenTree* op1, GenTree* op2, bool swapOK) break; case GT_FIELD: + case GT_FIELD_ADDR: if (op1->AsField()->gtFldHnd != op2->AsField()->gtFldHnd) { return false; @@ -2976,6 +2975,7 @@ unsigned Compiler::gtHashValue(GenTree* tree) break; case GT_FIELD: + case GT_FIELD_ADDR: hash = genTreeHashAdd(hash, tree->AsField()->gtFldHnd); break; @@ -6540,19 +6540,13 @@ ExceptionSetFlags GenTree::OperExceptions(Compiler* comp) return ExceptionSetFlags::None; case GT_FIELD: - { - GenTree* fldObj = this->AsField()->GetFldObj(); - - if (fldObj != nullptr) + case GT_FIELD_ADDR: + if (AsField()->IsInstance() && comp->fgAddrCouldBeNull(AsField()->GetFldObj())) { - if (comp->fgAddrCouldBeNull(fldObj)) - { - return ExceptionSetFlags::NullReferenceException; - } + return ExceptionSetFlags::NullReferenceException; } return ExceptionSetFlags::None; - } case GT_BOUNDS_CHECK: case GT_INDEX_ADDR: @@ -8529,8 +8523,10 @@ GenTree* Compiler::gtCloneExpr( break; case GT_FIELD: - copy = new (this, GT_FIELD) GenTreeField(tree->TypeGet(), tree->AsField()->GetFldObj(), - tree->AsField()->gtFldHnd, tree->AsField()->gtFldOffset); + case GT_FIELD_ADDR: + copy = new (this, tree->OperGet()) + GenTreeField(tree->OperGet(), tree->TypeGet(), tree->AsField()->GetFldObj(), + tree->AsField()->gtFldHnd, tree->AsField()->gtFldOffset); copy->AsField()->gtFldMayOverlap = tree->AsField()->gtFldMayOverlap; #ifdef FEATURE_READYTORUN copy->AsField()->gtFieldLookup = tree->AsField()->gtFieldLookup; @@ -9327,6 +9323,7 @@ GenTreeUseEdgeIterator::GenTreeUseEdgeIterator(GenTree* node) // Unary operators with an optional operand case GT_NOP: case GT_FIELD: + case GT_FIELD_ADDR: case GT_RETURN: case GT_RETFILT: if (m_node->AsUnOp()->gtOp1 == nullptr) @@ -11761,7 +11758,7 @@ void Compiler::gtDispTree(GenTree* tree, #endif // FEATURE_ARG_SPLIT #endif // FEATURE_PUT_STRUCT_ARG_STK - if (tree->OperIs(GT_FIELD)) + if (tree->OperIs(GT_FIELD, GT_FIELD_ADDR)) { printf(" %s", eeGetFieldName(tree->AsField()->gtFldHnd), 0); } diff --git a/src/coreclr/jit/gentree.h b/src/coreclr/jit/gentree.h index 6bc30c038f5a83..2744aa44508149 100644 --- a/src/coreclr/jit/gentree.h +++ b/src/coreclr/jit/gentree.h @@ -1743,6 +1743,7 @@ struct GenTree case GT_RETFILT: case GT_NOP: case GT_FIELD: + case GT_FIELD_ADDR: return true; case GT_RETURN: return gtType == TYP_VOID; @@ -4030,8 +4031,8 @@ struct GenTreeField : public GenTreeUnOp CORINFO_CONST_LOOKUP gtFieldLookup; #endif - GenTreeField(var_types type, GenTree* obj, CORINFO_FIELD_HANDLE fldHnd, DWORD offs) - : GenTreeUnOp(GT_FIELD, type, obj), gtFldHnd(fldHnd), gtFldOffset(offs), gtFldMayOverlap(false) + GenTreeField(genTreeOps oper, var_types type, GenTree* obj, CORINFO_FIELD_HANDLE fldHnd, DWORD offs) + : GenTreeUnOp(oper, type, obj), gtFldHnd(fldHnd), gtFldOffset(offs), gtFldMayOverlap(false) { #ifdef FEATURE_READYTORUN gtFieldLookup.addr = nullptr; diff --git a/src/coreclr/jit/gtlist.h b/src/coreclr/jit/gtlist.h index 65a77bf6a3e0aa..cde991961ab81c 100644 --- a/src/coreclr/jit/gtlist.h +++ b/src/coreclr/jit/gtlist.h @@ -93,7 +93,8 @@ GTNODE(NULLCHECK , GenTreeIndir ,0,GTK_UNOP|GTK_NOVALUE) GTNODE(ARR_LENGTH , GenTreeArrLen ,0,GTK_UNOP|GTK_EXOP) // single-dimension (SZ) array length GTNODE(MDARR_LENGTH , GenTreeMDArr ,0,GTK_UNOP|GTK_EXOP) // multi-dimension (MD) array length of a specific dimension GTNODE(MDARR_LOWER_BOUND, GenTreeMDArr ,0,GTK_UNOP|GTK_EXOP) // multi-dimension (MD) array lower bound of a specific dimension -GTNODE(FIELD , GenTreeField ,0,GTK_UNOP|GTK_EXOP|DBK_NOTLIR) // Member-field +GTNODE(FIELD , GenTreeField ,0,GTK_UNOP|GTK_EXOP|DBK_NOTLIR) // Field load +GTNODE(FIELD_ADDR , GenTreeField ,0,GTK_UNOP|GTK_EXOP|DBK_NOTLIR) // Field address GTNODE(ALLOCOBJ , GenTreeAllocObj ,0,GTK_UNOP|GTK_EXOP|DBK_NOTLIR) // object allocator GTNODE(INIT_VAL , GenTreeOp ,0,GTK_UNOP) // Initialization value for an initBlk diff --git a/src/coreclr/jit/gtstructs.h b/src/coreclr/jit/gtstructs.h index 7d50adbca39f97..ae62bde014a5a0 100644 --- a/src/coreclr/jit/gtstructs.h +++ b/src/coreclr/jit/gtstructs.h @@ -66,7 +66,7 @@ GTSTRUCT_3(LclVar , GT_LCL_VAR, GT_LCL_VAR_ADDR, GT_STORE_LCL_VAR) GTSTRUCT_3(LclFld , GT_LCL_FLD, GT_STORE_LCL_FLD, GT_LCL_FLD_ADDR) GTSTRUCT_1(Cast , GT_CAST) GTSTRUCT_1(Box , GT_BOX) -GTSTRUCT_1(Field , GT_FIELD) +GTSTRUCT_2(Field , GT_FIELD, GT_FIELD_ADDR) GTSTRUCT_1(Call , GT_CALL) GTSTRUCT_1(FieldList , GT_FIELD_LIST) GTSTRUCT_1(Colon , GT_COLON) From f2424bdb739247edf94e2a0cc9aafcca5aadb3c2 Mon Sep 17 00:00:00 2001 From: SingleAccretion Date: Sun, 23 Oct 2022 02:07:33 +0300 Subject: [PATCH 02/13] Add FIELD_ADDR to fgAddrCouldBeNull --- src/coreclr/jit/flowgraph.cpp | 132 ++++++++++++++++------------------ 1 file changed, 60 insertions(+), 72 deletions(-) diff --git a/src/coreclr/jit/flowgraph.cpp b/src/coreclr/jit/flowgraph.cpp index 413165417b76a0..7974607091d96d 100644 --- a/src/coreclr/jit/flowgraph.cpp +++ b/src/coreclr/jit/flowgraph.cpp @@ -936,98 +936,86 @@ GenTreeCall* Compiler::fgGetSharedCCtor(CORINFO_CLASS_HANDLE cls) // bool Compiler::fgAddrCouldBeNull(GenTree* addr) { - addr = addr->gtEffectiveVal(); - if (addr->IsIconHandle()) - { - return false; - } - else if (addr->OperIs(GT_CNS_STR, GT_CLS_VAR_ADDR)) - { - return false; - } - else if (addr->OperIs(GT_INDEX_ADDR)) - { - return !addr->AsIndexAddr()->IsNotNull(); - } - else if (addr->OperIs(GT_ARR_ADDR)) - { - return (addr->gtFlags & GTF_ARR_ADDR_NONNULL) == 0; - } - else if (addr->gtOper == GT_LCL_VAR) - { - unsigned varNum = addr->AsLclVarCommon()->GetLclNum(); - - if (lvaIsImplicitByRefLocal(varNum)) - { + switch (addr->OperGet()) + { + case GT_CNS_INT: + return !addr->IsIconHandle(); + + case GT_CNS_STR: + case GT_ADDR: + case GT_FIELD_ADDR: + case GT_CLS_VAR_ADDR: + // A GT_ADDR node, by itself, never requires null checking. The expression whose address is being + // taken is either a local or static variable, whose address is necessarily non-null, or else it is + // a field dereference, which will do its own bounds checking if necessary. return false; - } - } - else if (addr->gtOper == GT_ADDR) - { - if (addr->AsOp()->gtOp1->gtOper == GT_CNS_INT) - { - GenTree* cns1Tree = addr->AsOp()->gtOp1; - if (!cns1Tree->IsIconHandle()) - { - // Indirection of some random constant... - // It is safest just to return true - return true; - } - } - return false; // we can't have a null address - } - else if (addr->gtOper == GT_ADD) - { - if (addr->AsOp()->gtOp1->gtOper == GT_CNS_INT) - { - GenTree* cns1Tree = addr->AsOp()->gtOp1; - if (!cns1Tree->IsIconHandle()) + case GT_INDEX_ADDR: + return !addr->AsIndexAddr()->IsNotNull(); + + case GT_ARR_ADDR: + return (addr->gtFlags & GTF_ARR_ADDR_NONNULL) == 0; + + case GT_LCL_VAR: + return !lvaIsImplicitByRefLocal(addr->AsLclVar()->GetLclNum()); + + case GT_COMMA: + return fgAddrCouldBeNull(addr->AsOp()->gtOp2); + + case GT_ADD: + if (addr->AsOp()->gtOp1->gtOper == GT_CNS_INT) { - if (!fgIsBigOffset(cns1Tree->AsIntCon()->gtIconVal)) + GenTree* cns1Tree = addr->AsOp()->gtOp1; + if (!cns1Tree->IsIconHandle()) { - // Op1 was an ordinary small constant - return fgAddrCouldBeNull(addr->AsOp()->gtOp2); + if (!fgIsBigOffset(cns1Tree->AsIntCon()->gtIconVal)) + { + // Op1 was an ordinary small constant + return fgAddrCouldBeNull(addr->AsOp()->gtOp2); + } + } + else // Op1 was a handle represented as a constant + { + // Is Op2 also a constant? + if (addr->AsOp()->gtOp2->gtOper == GT_CNS_INT) + { + GenTree* cns2Tree = addr->AsOp()->gtOp2; + // Is this an addition of a handle and constant + if (!cns2Tree->IsIconHandle()) + { + if (!fgIsBigOffset(cns2Tree->AsIntCon()->gtIconVal)) + { + // Op2 was an ordinary small constant + return false; // we can't have a null address + } + } + } } } - else // Op1 was a handle represented as a constant + else { - // Is Op2 also a constant? + // Op1 is not a constant. What about Op2? if (addr->AsOp()->gtOp2->gtOper == GT_CNS_INT) { GenTree* cns2Tree = addr->AsOp()->gtOp2; - // Is this an addition of a handle and constant + // Is this an addition of a small constant if (!cns2Tree->IsIconHandle()) { if (!fgIsBigOffset(cns2Tree->AsIntCon()->gtIconVal)) { // Op2 was an ordinary small constant - return false; // we can't have a null address + return fgAddrCouldBeNull(addr->AsOp()->gtOp1); } } } } - } - else - { - // Op1 is not a constant - // What about Op2? - if (addr->AsOp()->gtOp2->gtOper == GT_CNS_INT) - { - GenTree* cns2Tree = addr->AsOp()->gtOp2; - // Is this an addition of a small constant - if (!cns2Tree->IsIconHandle()) - { - if (!fgIsBigOffset(cns2Tree->AsIntCon()->gtIconVal)) - { - // Op2 was an ordinary small constant - return fgAddrCouldBeNull(addr->AsOp()->gtOp1); - } - } - } - } + break; + + default: + break; } - return true; // default result: addr could be null + + return true; // default result: addr could be null. } //------------------------------------------------------------------------------ From 327520513c6790941ee3bed23a0749e91598a4b7 Mon Sep 17 00:00:00 2001 From: SingleAccretion Date: Sat, 22 Oct 2022 22:55:14 +0300 Subject: [PATCH 03/13] Add gtNewFieldAddrNode; move gtNewFieldRef --- src/coreclr/jit/compiler.h | 6 ++- src/coreclr/jit/compiler.hpp | 55 ------------------------- src/coreclr/jit/gentree.cpp | 80 ++++++++++++++++++++++++++++++++++++ 3 files changed, 84 insertions(+), 57 deletions(-) diff --git a/src/coreclr/jit/compiler.h b/src/coreclr/jit/compiler.h index 2aa7ff203096d4..5a26fc292b1ec6 100644 --- a/src/coreclr/jit/compiler.h +++ b/src/coreclr/jit/compiler.h @@ -2598,8 +2598,10 @@ class Compiler GenTreeField* gtNewFieldRef(var_types type, CORINFO_FIELD_HANDLE fldHnd, GenTree* obj = nullptr, DWORD offset = 0); - GenTreeField* gtNewFieldAddrNode( - var_types type, CORINFO_FIELD_HANDLE fldHnd, GenTree* obj = nullptr, DWORD offset = 0); + GenTreeField* gtNewFieldAddrNode(var_types type, + CORINFO_FIELD_HANDLE fldHnd, + GenTree* obj = nullptr, + DWORD offset = 0); GenTreeIndexAddr* gtNewIndexAddr(GenTree* arrayOp, GenTree* indexOp, diff --git a/src/coreclr/jit/compiler.hpp b/src/coreclr/jit/compiler.hpp index f683770e8f8ccf..cd93aa400cafc3 100644 --- a/src/coreclr/jit/compiler.hpp +++ b/src/coreclr/jit/compiler.hpp @@ -1069,61 +1069,6 @@ inline GenTree* Compiler::gtNewRuntimeLookup(CORINFO_GENERIC_HANDLE hnd, CorInfo return node; } -//------------------------------------------------------------------------ -// gtNewFieldRef: a helper for creating GT_FIELD nodes. -// -// Normalizes struct types (for SIMD vectors). Sets GTF_GLOB_REF for fields -// that may be pointing into globally visible memory. -// -// Arguments: -// type - type for the field node -// fldHnd - the field handle -// obj - the instance, an address -// offset - the field offset -// -// Return Value: -// The created node. -// -inline GenTreeField* Compiler::gtNewFieldRef(var_types type, CORINFO_FIELD_HANDLE fldHnd, GenTree* obj, DWORD offset) -{ - // GT_FIELD nodes are transformed into GT_IND nodes. - assert(GenTree::s_gtNodeSizes[GT_IND] <= GenTree::s_gtNodeSizes[GT_FIELD]); - - if (type == TYP_STRUCT) - { - CORINFO_CLASS_HANDLE structHnd; - eeGetFieldType(fldHnd, &structHnd); - type = impNormStructType(structHnd); - } - - GenTreeField* fieldNode = new (this, GT_FIELD) GenTreeField(type, 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->OperIs(GT_ADDR) && varTypeIsStruct(obj->AsUnOp()->gtOp1) && - obj->AsUnOp()->gtOp1->OperIs(GT_LCL_VAR)) - { - LclVarDsc* varDsc = lvaGetDesc(obj->AsUnOp()->gtOp1->AsLclVarCommon()); - - varDsc->lvFieldAccessed = 1; - - if (lvaIsImplicitByRefLocal(lvaGetLclNum(varDsc))) - { - // These structs are passed by reference and can easily become global references if those - // references are exposed. We clear out address-exposure information for these parameters - // when they are converted into references in fgRetypeImplicitByRefArgs() so we do not have - // the necessary information in morph to know if these indirections are actually global - // references, so we have to be conservative here. - fieldNode->gtFlags |= GTF_GLOB_REF; - } - } - else - { - fieldNode->gtFlags |= GTF_GLOB_REF; - } - - return fieldNode; -} - inline GenTreeIndexAddr* Compiler::gtNewIndexAddr(GenTree* arrayOp, GenTree* indexOp, var_types elemType, diff --git a/src/coreclr/jit/gentree.cpp b/src/coreclr/jit/gentree.cpp index c046a06819579a..0c84a9cd48d76d 100644 --- a/src/coreclr/jit/gentree.cpp +++ b/src/coreclr/jit/gentree.cpp @@ -7377,6 +7377,86 @@ GenTreeRetExpr* Compiler::gtNewInlineCandidateReturnExpr(GenTreeCall* inlineCand return node; } +//------------------------------------------------------------------------ +// gtNewFieldRef: Create a new GT_FIELD node. +// +// Normalizes struct types (for SIMD vectors). Sets GTF_GLOB_REF for fields +// that may be pointing into globally visible memory. +// +// Arguments: +// type - type for the field node +// fldHnd - the field handle +// obj - the instance, an address +// offset - the field offset +// +// Return Value: +// The created node. +// +GenTreeField* Compiler::gtNewFieldRef(var_types type, CORINFO_FIELD_HANDLE fldHnd, GenTree* obj, DWORD offset) +{ + // GT_FIELD nodes are transformed into GT_IND nodes. + assert(GenTree::s_gtNodeSizes[GT_IND] <= GenTree::s_gtNodeSizes[GT_FIELD]); + + if (type == TYP_STRUCT) + { + CORINFO_CLASS_HANDLE structHnd; + eeGetFieldType(fldHnd, &structHnd); + type = impNormStructType(structHnd); + } + + GenTreeField* fieldNode = new (this, GT_FIELD) GenTreeField(GT_FIELD, type, 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->OperIs(GT_ADDR) && varTypeIsStruct(obj->AsUnOp()->gtOp1) && + obj->AsUnOp()->gtOp1->OperIs(GT_LCL_VAR)) + { + LclVarDsc* varDsc = lvaGetDesc(obj->AsUnOp()->gtOp1->AsLclVarCommon()); + + varDsc->lvFieldAccessed = 1; + + if (lvaIsImplicitByRefLocal(lvaGetLclNum(varDsc))) + { + // These structs are passed by reference and can easily become global references if those + // references are exposed. We clear out address-exposure information for these parameters + // when they are converted into references in fgRetypeImplicitByRefArgs() so we do not have + // the necessary information in morph to know if these indirections are actually global + // references, so we have to be conservative here. + fieldNode->gtFlags |= GTF_GLOB_REF; + } + } + else + { + fieldNode->gtFlags |= GTF_GLOB_REF; + } + + return fieldNode; +} + +//------------------------------------------------------------------------ +// gtNewFieldRef: Create a new GT_FIELD_ADDR node. +// +// Arguments: +// type - type for the address node +// fldHnd - the field handle +// obj - the instance, an address +// offset - the field offset +// +// Return Value: +// The created node. +// +GenTreeField* Compiler::gtNewFieldAddrNode(var_types type, CORINFO_FIELD_HANDLE fldHnd, GenTree* obj, DWORD offset) +{ + assert(varTypeIsI(genActualType(type))); + + GenTreeField* fieldNode = new (this, GT_FIELD_ADDR) GenTreeField(GT_FIELD_ADDR, type, obj, fldHnd, offset); + + // TODO-ADDR: add GTF_EXCEPT handling here and delete it from callers. + // TODO-ADDR: delete this zero-diff quirk. + fieldNode->gtFlags |= GTF_GLOB_REF; + + return fieldNode; +} + /***************************************************************************** * * Create a node that will assign 'src' to 'dst'. From 042342a3e7fc46c14c029e8ed4460a7eed5024a4 Mon Sep 17 00:00:00 2001 From: SingleAccretion Date: Sun, 23 Oct 2022 00:07:54 +0300 Subject: [PATCH 04/13] Implement Windows x86 TLS via FIELD_ADDR Tested manually to work as well as it did before. --- src/coreclr/jit/compiler.cpp | 4 +- src/coreclr/jit/compiler.h | 3 +- src/coreclr/jit/compiler.hpp | 6 +- src/coreclr/jit/gentree.h | 47 +++-- src/coreclr/jit/importer.cpp | 41 ++-- src/coreclr/jit/morph.cpp | 328 +++++++++++++++++--------------- src/coreclr/jit/rationalize.cpp | 3 - 7 files changed, 234 insertions(+), 198 deletions(-) diff --git a/src/coreclr/jit/compiler.cpp b/src/coreclr/jit/compiler.cpp index 9184f102ce7e4e..6be6f06c1714b2 100644 --- a/src/coreclr/jit/compiler.cpp +++ b/src/coreclr/jit/compiler.cpp @@ -9311,9 +9311,9 @@ void cTreeFlags(Compiler* comp, GenTree* tree) { chars += printf("[IND_TGT_HEAP]"); } - if (tree->gtFlags & GTF_IND_TLS_REF) + if (tree->gtFlags & GTF_IND_REQ_ADDR_IN_REG) { - chars += printf("[IND_TLS_REF]"); + chars += printf("[IND_REQ_ADDR_IN_REG]"); } if (tree->gtFlags & GTF_IND_ASG_LHS) { diff --git a/src/coreclr/jit/compiler.h b/src/coreclr/jit/compiler.h index 5a26fc292b1ec6..78516c0b8d7e28 100644 --- a/src/coreclr/jit/compiler.h +++ b/src/coreclr/jit/compiler.h @@ -2625,7 +2625,7 @@ class Compiler GenTreeMDArr* gtNewMDArrLowerBound(GenTree* arrayOp, unsigned dim, unsigned rank, BasicBlock* block); - GenTreeIndir* gtNewIndir(var_types typ, GenTree* addr); + GenTreeIndir* gtNewIndir(var_types typ, GenTree* addr, GenTreeFlags indirFlags = GTF_EMPTY); GenTree* gtNewNullCheck(GenTree* addr, BasicBlock* basicBlock); @@ -5681,6 +5681,7 @@ class Compiler private: GenTree* fgMorphField(GenTree* tree, MorphAddrContext* mac); + GenTree* fgMorphExpandTlsFieldAddr(GenTree* tree); bool fgCanFastTailCall(GenTreeCall* call, const char** failReason); #if FEATURE_FASTTAILCALL bool fgCallHasMustCopyByrefParameter(GenTreeCall* callee); diff --git a/src/coreclr/jit/compiler.hpp b/src/coreclr/jit/compiler.hpp index cd93aa400cafc3..b89ebaf08715e0 100644 --- a/src/coreclr/jit/compiler.hpp +++ b/src/coreclr/jit/compiler.hpp @@ -1212,10 +1212,14 @@ inline GenTreeMDArr* Compiler::gtNewMDArrLowerBound(GenTree* arrayOp, unsigned d // Return Value: // New GT_IND node -inline GenTreeIndir* Compiler::gtNewIndir(var_types typ, GenTree* addr) +inline GenTreeIndir* Compiler::gtNewIndir(var_types typ, GenTree* addr, GenTreeFlags indirFlags) { + assert((indirFlags & ~GTF_IND_FLAGS) == GTF_EMPTY); + GenTree* indir = gtNewOperNode(GT_IND, typ, addr); + indir->gtFlags |= indirFlags; indir->SetIndirExceptionFlags(this); + return indir->AsIndir(); } diff --git a/src/coreclr/jit/gentree.h b/src/coreclr/jit/gentree.h index 2744aa44508149..75087d2b8c6b25 100644 --- a/src/coreclr/jit/gentree.h +++ b/src/coreclr/jit/gentree.h @@ -489,33 +489,29 @@ enum GenTreeFlags : unsigned int GTF_MEMORYBARRIER_LOAD = 0x40000000, // GT_MEMORYBARRIER -- Load barrier + GTF_FLD_TLS = 0x80000000, // GT_FIELD_ADDR -- field address is a Windows x86 TLS reference GTF_FLD_VOLATILE = 0x40000000, // GT_FIELD -- same as GTF_IND_VOLATILE - GTF_FLD_INITCLASS = 0x20000000, // GT_FIELD -- field access requires preceding class/static init helper + GTF_FLD_INITCLASS = 0x20000000, // GT_FIELD/GT_FIELD_ADDR -- field access requires preceding class/static init helper GTF_FLD_TGT_HEAP = 0x10000000, // GT_FIELD -- same as GTF_IND_TGT_HEAP GTF_INX_RNGCHK = 0x80000000, // GT_INDEX_ADDR -- this array address should be range-checked GTF_INX_ADDR_NONNULL = 0x40000000, // GT_INDEX_ADDR -- this array address is not null - GTF_IND_TGT_NOT_HEAP = 0x80000000, // GT_IND -- the target is not on the heap - GTF_IND_VOLATILE = 0x40000000, // GT_IND -- the load or store must use volatile semantics (this is a nop on X86) + GTF_IND_TGT_NOT_HEAP = 0x80000000, // GT_IND -- the target is not on the heap + GTF_IND_VOLATILE = 0x40000000, // GT_IND -- the load or store must use volatile semantics (this is a nop on X86) GTF_IND_NONFAULTING = 0x20000000, // Operations for which OperIsIndir() is true -- An indir that cannot fault. - // Same as GTF_ARRLEN_NONFAULTING. - GTF_IND_TGT_HEAP = 0x10000000, // GT_IND -- the target is on the heap - GTF_IND_TLS_REF = 0x08000000, // GT_IND -- the target is accessed via TLS - GTF_IND_ASG_LHS = 0x04000000, // GT_IND -- this GT_IND node is (the effective val) of the LHS of an - // assignment; don't evaluate it independently. - GTF_IND_REQ_ADDR_IN_REG = GTF_IND_ASG_LHS, // GT_IND -- requires its addr operand to be evaluated - // into a register. This flag is useful in cases where it - // is required to generate register indirect addressing mode. - // One such case is virtual stub calls on xarch. This is only - // valid in the backend, where GTF_IND_ASG_LHS is not necessary - // (all such indirections will be lowered to GT_STOREIND). - GTF_IND_UNALIGNED = 0x02000000, // GT_IND -- the load or store is unaligned (we assume worst case - // alignment of 1 byte) - GTF_IND_INVARIANT = 0x01000000, // GT_IND -- the target is invariant (a prejit indirection) - GTF_IND_NONNULL = 0x00400000, // GT_IND -- the indirection never returns null (zero) - - GTF_IND_FLAGS = GTF_IND_VOLATILE | GTF_IND_NONFAULTING | GTF_IND_TLS_REF | GTF_IND_UNALIGNED | GTF_IND_INVARIANT | + GTF_IND_TGT_HEAP = 0x10000000, // GT_IND -- the target is on the heap + GTF_IND_REQ_ADDR_IN_REG = 0x08000000, // GT_IND -- requires its addr operand to be evaluated into a register. + // This flag is useful in cases where it is required to generate register + // indirect addressing mode. One such case is virtual stub calls on xarch. + GTF_IND_ASG_LHS = 0x04000000, // GT_IND -- this GT_IND node is (the effective val) of the LHS of an + // assignment; don't evaluate it independently. + GTF_IND_UNALIGNED = 0x02000000, // GT_IND -- the load or store is unaligned (we assume worst case + // alignment of 1 byte) + GTF_IND_INVARIANT = 0x01000000, // GT_IND -- the target is invariant (a prejit indirection) + GTF_IND_NONNULL = 0x00400000, // GT_IND -- the indirection never returns null (zero) + + GTF_IND_FLAGS = GTF_IND_VOLATILE | GTF_IND_NONFAULTING | GTF_IND_UNALIGNED | GTF_IND_INVARIANT | GTF_IND_NONNULL | GTF_IND_TGT_NOT_HEAP | GTF_IND_TGT_HEAP, GTF_ADDRMODE_NO_CSE = 0x80000000, // GT_ADD/GT_MUL/GT_LSH -- Do not CSE this node only, forms complex @@ -4057,6 +4053,17 @@ struct GenTreeField : public GenTreeUnOp { return (gtFlags & GTF_FLD_VOLATILE) != 0; } + + bool IsStatic() const + { + return GetFldObj() == nullptr; + } + + bool IsTlsStatic() const + { + assert(((gtFlags & GTF_FLD_TLS) == 0) || IsStatic()); + return (gtFlags & GTF_FLD_TLS) != 0; + } }; // There was quite a bit of confusion in the code base about which of gtOp1 and gtOp2 was the diff --git a/src/coreclr/jit/importer.cpp b/src/coreclr/jit/importer.cpp index e6041c12c5aae3..199e196b5c0102 100644 --- a/src/coreclr/jit/importer.cpp +++ b/src/coreclr/jit/importer.cpp @@ -9559,22 +9559,27 @@ void Compiler::impImportBlockCode(BasicBlock* block) case CORINFO_FIELD_STATIC_TLS: #ifdef TARGET_X86 // Legacy TLS access is implemented as intrinsic on x86 only + op1 = gtNewFieldAddrNode(TYP_I_IMPL, resolvedToken.hField, nullptr, fieldInfo.offset); + op1->gtFlags |= GTF_FLD_TLS; // fgMorphExpandTlsField will handle the transformation. - /* Create the data member node */ - op1 = gtNewFieldRef(lclTyp, resolvedToken.hField, NULL, fieldInfo.offset); - op1->gtFlags |= GTF_IND_TLS_REF; // fgMorphField will handle the transformation - - if (isLoadAddress) + if (!isLoadAddress) { - op1 = gtNewOperNode(GT_ADDR, (var_types)TYP_I_IMPL, op1); + if (varTypeIsStruct(lclTyp)) + { + op1 = gtNewObjNode(fieldInfo.structType, op1); + op1->gtFlags |= GTF_IND_NONFAULTING; + } + else + { + op1 = gtNewIndir(lclTyp, op1, GTF_IND_NONFAULTING); + op1->gtFlags |= GTF_GLOB_REF; + } } break; #else fieldInfo.fieldAccessor = CORINFO_FIELD_STATIC_ADDR_HELPER; - FALLTHROUGH; #endif - case CORINFO_FIELD_STATIC_ADDR_HELPER: case CORINFO_FIELD_INSTANCE_HELPER: case CORINFO_FIELD_INSTANCE_ADDR_HELPER: @@ -9827,19 +9832,25 @@ void Compiler::impImportBlockCode(BasicBlock* block) case CORINFO_FIELD_STATIC_TLS: #ifdef TARGET_X86 - // Legacy TLS access is implemented as intrinsic on x86 only - - /* Create the data member node */ - op1 = gtNewFieldRef(lclTyp, resolvedToken.hField, NULL, fieldInfo.offset); - op1->gtFlags |= GTF_IND_TLS_REF; // fgMorphField will handle the transformation + // Legacy TLS access is implemented as intrinsic on x86 only. + op1 = gtNewFieldAddrNode(TYP_I_IMPL, resolvedToken.hField, nullptr, fieldInfo.offset); + op1->gtFlags |= GTF_FLD_TLS; // fgMorphExpandTlsField will handle the transformation. + if (varTypeIsStruct(lclTyp)) + { + op1 = gtNewObjNode(fieldInfo.structType, op1); + op1->gtFlags |= GTF_IND_NONFAULTING; + } + else + { + op1 = gtNewIndir(lclTyp, op1, GTF_IND_NONFAULTING); + op1->gtFlags |= GTF_GLOB_REF; + } break; #else fieldInfo.fieldAccessor = CORINFO_FIELD_STATIC_ADDR_HELPER; - FALLTHROUGH; #endif - case CORINFO_FIELD_STATIC_ADDR_HELPER: case CORINFO_FIELD_INSTANCE_HELPER: case CORINFO_FIELD_INSTANCE_ADDR_HELPER: diff --git a/src/coreclr/jit/morph.cpp b/src/coreclr/jit/morph.cpp index f9110161d9f191..86e11dbb3bc79c 100644 --- a/src/coreclr/jit/morph.cpp +++ b/src/coreclr/jit/morph.cpp @@ -5073,11 +5073,6 @@ GenTree* Compiler::fgMorphField(GenTree* tree, MorphAddrContext* mac) // Is this an instance data member? if (objRef != nullptr) { - if (tree->gtFlags & GTF_IND_TLS_REF) - { - NO_WAY("instance field can not be a TLS ref."); - } - /* We'll create the expression "*(objRef + mem_offs)" */ noway_assert(varTypeIsGC(objRef->TypeGet()) || objRef->TypeGet() == TYP_I_IMPL); @@ -5322,187 +5317,201 @@ GenTree* Compiler::fgMorphField(GenTree* tree, MorphAddrContext* mac) } #endif } - else /* This is a static data member */ + else { - if (tree->gtFlags & GTF_IND_TLS_REF) + // Normal static field reference + // + // If we can we access the static's address directly + // then pFldAddr will be NULL and + // fldAddr will be the actual address of the static field + // + void** pFldAddr = nullptr; + void* fldAddr = info.compCompHnd->getFieldAddress(symHnd, (void**)&pFldAddr); + + // We should always be able to access this static field address directly + // + assert(pFldAddr == nullptr); + + // For boxed statics, this direct address will be for the box. We have already added + // the indirection for the field itself and attached the sequence, in importation. + bool isBoxedStatic = gtIsStaticFieldPtrToBoxedStruct(tree->TypeGet(), symHnd); + if (!isBoxedStatic) { - // Thread Local Storage static field reference - // - // Field ref is a TLS 'Thread-Local-Storage' reference - // - // Build this tree: IND(*) # - // | - // ADD(I_IMPL) - // / \. - // / CNS(fldOffset) - // / - // / - // / - // IND(I_IMPL) == [Base of this DLL's TLS] - // | - // ADD(I_IMPL) - // / \. - // / CNS(IdValue*4) or MUL - // / / \. - // IND(I_IMPL) / CNS(4) - // | / - // CNS(TLS_HDL,0x2C) IND - // | - // CNS(pIdAddr) - // - // # Denotes the original node - // - void** pIdAddr = nullptr; - unsigned IdValue = info.compCompHnd->getFieldThreadLocalStoreID(symHnd, (void**)&pIdAddr); + // Only simple statics get importred as GT_FIELDs. + fieldSeq = GetFieldSeqStore()->Create(symHnd, reinterpret_cast(fldAddr), + FieldSeq::FieldKind::SimpleStatic); + } - // - // If we can we access the TLS DLL index ID value directly - // then pIdAddr will be NULL and - // IdValue will be the actual TLS DLL index ID - // - GenTree* dllRef = nullptr; - if (pIdAddr == nullptr) + // TODO-CQ: enable this optimization for 32 bit targets. + bool isStaticReadOnlyInited = false; +#ifdef TARGET_64BIT + if (tree->TypeIs(TYP_REF) && !isBoxedStatic) + { + bool pIsSpeculative = true; + if (info.compCompHnd->getStaticFieldCurrentClass(symHnd, &pIsSpeculative) != NO_CLASS_HANDLE) { - if (IdValue != 0) - { - dllRef = gtNewIconNode(IdValue * 4, TYP_I_IMPL); - } - } - else - { - dllRef = gtNewIndOfIconHandleNode(TYP_I_IMPL, (size_t)pIdAddr, GTF_ICON_CONST_PTR, true); - - // Next we multiply by 4 - dllRef = gtNewOperNode(GT_MUL, TYP_I_IMPL, dllRef, gtNewIconNode(4, TYP_I_IMPL)); + isStaticReadOnlyInited = !pIsSpeculative; } + } +#endif // TARGET_64BIT -#define WIN32_TLS_SLOTS (0x2C) // Offset from fs:[0] where the pointer to the slots resides + GenTreeFlags handleKind = GTF_EMPTY; + if (isBoxedStatic) + { + handleKind = GTF_ICON_STATIC_BOX_PTR; + } + else if (isStaticReadOnlyInited) + { + handleKind = GTF_ICON_CONST_PTR; + } + else + { + handleKind = GTF_ICON_STATIC_HDL; + } + GenTreeIntCon* addr = gtNewIconHandleNode((size_t)fldAddr, handleKind, fieldSeq); + INDEBUG(addr->gtTargetHandle = reinterpret_cast(symHnd)); - // Mark this ICON as a TLS_HDL, codegen will use FS:[cns] + // Translate GTF_FLD_INITCLASS to GTF_ICON_INITCLASS, if we need to. + if (((tree->gtFlags & GTF_FLD_INITCLASS) != 0) && !isStaticReadOnlyInited) + { + tree->gtFlags &= ~GTF_FLD_INITCLASS; + addr->gtFlags |= GTF_ICON_INITCLASS; + } - GenTree* tlsRef = gtNewIconHandleNode(WIN32_TLS_SLOTS, GTF_ICON_TLS_HDL); + tree->SetOper(GT_IND); + tree->AsOp()->gtOp1 = addr; - // Translate GTF_FLD_INITCLASS to GTF_ICON_INITCLASS - if ((tree->gtFlags & GTF_FLD_INITCLASS) != 0) - { - tree->gtFlags &= ~GTF_FLD_INITCLASS; - tlsRef->gtFlags |= GTF_ICON_INITCLASS; - } + if (isBoxedStatic) + { + // The box for the static cannot be null, and is logically invariant, since it + // represents (a base for) the static's address. + tree->gtFlags |= (GTF_IND_INVARIANT | GTF_IND_NONFAULTING | GTF_IND_NONNULL); + } + else if (isStaticReadOnlyInited) + { + JITDUMP("Marking initialized static read-only field '%s' as invariant.\n", eeGetFieldName(symHnd)); - tlsRef = gtNewOperNode(GT_IND, TYP_I_IMPL, tlsRef); + // Static readonly field is not null at this point (see getStaticFieldCurrentClass impl). + tree->gtFlags |= (GTF_IND_INVARIANT | GTF_IND_NONFAULTING | GTF_IND_NONNULL); + } - if (dllRef != nullptr) - { - /* Add the dllRef */ - tlsRef = gtNewOperNode(GT_ADD, TYP_I_IMPL, tlsRef, dllRef); - } + return fgMorphSmpOp(tree, /* mac */ nullptr); + } - /* indirect to have tlsRef point at the base of the DLLs Thread Local Storage */ - tlsRef = gtNewOperNode(GT_IND, TYP_I_IMPL, tlsRef); + noway_assert(tree->OperIs(GT_IND)); - // Add the TLS static field offset to the address. - assert(!fldMayOverlap); - fieldSeq = GetFieldSeqStore()->Create(symHnd, fldOffset, FieldSeq::FieldKind::SimpleStatic); - tlsRef = gtNewOperNode(GT_ADD, TYP_I_IMPL, tlsRef, gtNewIconNode(fldOffset, fieldSeq)); + // Pass down the current mac; if non null we are computing an address + GenTree* result = fgMorphSmpOp(tree, mac); - // Final indirect to get to actual value of TLS static field + JITDUMP("\nFinal value of Compiler::fgMorphField after calling fgMorphSmpOp:\n"); + DISPTREE(result); - tree->SetOper(GT_IND); - tree->AsOp()->gtOp1 = tlsRef; + return result; +} - noway_assert(tree->gtFlags & GTF_IND_TLS_REF); - } - else - { - // Normal static field reference - // - // If we can we access the static's address directly - // then pFldAddr will be NULL and - // fldAddr will be the actual address of the static field - // - void** pFldAddr = nullptr; - void* fldAddr = info.compCompHnd->getFieldAddress(symHnd, (void**)&pFldAddr); +//------------------------------------------------------------------------ +// fgMorphExpandTlsFieldAddr: Expand a TLS field address. +// +// Arguments: +// tree - The GT_FIELD_ADDR tree +// +// Return Value: +// The expanded tree - a GT_ADD. +// +// Notes: +// This expands ".tls"-style statics, produced by the C++/CLI compiler +// for "__declspec(thread)" variables. An overview of the underlying +// (native) mechanism can be found here: http://www.nynaeve.net/?p=180. +// +GenTree* Compiler::fgMorphExpandTlsFieldAddr(GenTree* tree) +{ + // Note we do not support "FIELD"s for TLS statics, for simplicity. + assert(tree->OperIs(GT_FIELD_ADDR) && tree->AsField()->IsTlsStatic()); - // We should always be able to access this static field address directly - // - assert(pFldAddr == nullptr); + CORINFO_FIELD_HANDLE fieldHandle = tree->AsField()->gtFldHnd; + int fieldOffset = tree->AsField()->gtFldOffset; - // For boxed statics, this direct address will be for the box. We have already added - // the indirection for the field itself and attached the sequence, in importation. - bool isBoxedStatic = gtIsStaticFieldPtrToBoxedStruct(tree->TypeGet(), symHnd); - if (!isBoxedStatic) - { - // Only simple statics get importred as GT_FIELDs. - fieldSeq = GetFieldSeqStore()->Create(symHnd, reinterpret_cast(fldAddr), - FieldSeq::FieldKind::SimpleStatic); - } + // Thread Local Storage static field reference + // + // Field ref is a TLS 'Thread-Local-Storage' reference + // + // Build this tree: ADD(I_IMPL) # + // / \. + // / CNS(fldOffset) + // / + // / + // / + // IND(I_IMPL) == [Base of this DLL's TLS] + // | + // ADD(I_IMPL) + // / \. + // / CNS(IdValue*4) or MUL + // / / \. + // IND(I_IMPL) / CNS(4) + // | / + // CNS(TLS_HDL,0x2C) IND + // | + // CNS(pIdAddr) + // + // # Denotes the original node + // + void** pIdAddr = nullptr; + unsigned IdValue = info.compCompHnd->getFieldThreadLocalStoreID(fieldHandle, (void**)&pIdAddr); - // TODO-CQ: enable this optimization for 32 bit targets. - bool isStaticReadOnlyInited = false; -#ifdef TARGET_64BIT - if (tree->TypeIs(TYP_REF) && !isBoxedStatic) - { - bool pIsSpeculative = true; - if (info.compCompHnd->getStaticFieldCurrentClass(symHnd, &pIsSpeculative) != NO_CLASS_HANDLE) - { - isStaticReadOnlyInited = !pIsSpeculative; - } - } -#endif // TARGET_64BIT + // + // If we can we access the TLS DLL index ID value directly + // then pIdAddr will be NULL and + // IdValue will be the actual TLS DLL index ID + // + GenTree* dllRef = nullptr; + if (pIdAddr == nullptr) + { + if (IdValue != 0) + { + dllRef = gtNewIconNode(IdValue * 4, TYP_I_IMPL); + } + } + else + { + dllRef = gtNewIndOfIconHandleNode(TYP_I_IMPL, (size_t)pIdAddr, GTF_ICON_CONST_PTR, true); - GenTreeFlags handleKind = GTF_EMPTY; - if (isBoxedStatic) - { - handleKind = GTF_ICON_STATIC_BOX_PTR; - } - else if (isStaticReadOnlyInited) - { - handleKind = GTF_ICON_CONST_PTR; - } - else - { - handleKind = GTF_ICON_STATIC_HDL; - } - GenTreeIntCon* addr = gtNewIconHandleNode((size_t)fldAddr, handleKind, fieldSeq); - INDEBUG(addr->gtTargetHandle = reinterpret_cast(symHnd)); + // Next we multiply by 4 + dllRef = gtNewOperNode(GT_MUL, TYP_I_IMPL, dllRef, gtNewIconNode(4, TYP_I_IMPL)); + } - // Translate GTF_FLD_INITCLASS to GTF_ICON_INITCLASS, if we need to. - if (((tree->gtFlags & GTF_FLD_INITCLASS) != 0) && !isStaticReadOnlyInited) - { - tree->gtFlags &= ~GTF_FLD_INITCLASS; - addr->gtFlags |= GTF_ICON_INITCLASS; - } +#define WIN32_TLS_SLOTS (0x2C) // Offset from fs:[0] where the pointer to the slots resides - tree->SetOper(GT_IND); - tree->AsOp()->gtOp1 = addr; + // Mark this ICON as a TLS_HDL, codegen will use FS:[cns] + GenTree* tlsRef = gtNewIconHandleNode(WIN32_TLS_SLOTS, GTF_ICON_TLS_HDL); - if (isBoxedStatic) - { - // The box for the static cannot be null, and is logically invariant, since it - // represents (a base for) the static's address. - tree->gtFlags |= (GTF_IND_INVARIANT | GTF_IND_NONFAULTING | GTF_IND_NONNULL); - } - else if (isStaticReadOnlyInited) - { - JITDUMP("Marking initialized static read-only field '%s' as invariant.\n", eeGetFieldName(symHnd)); + // Translate GTF_FLD_INITCLASS to GTF_ICON_INITCLASS + if ((tree->gtFlags & GTF_FLD_INITCLASS) != 0) + { + tree->gtFlags &= ~GTF_FLD_INITCLASS; + tlsRef->gtFlags |= GTF_ICON_INITCLASS; + } - // Static readonly field is not null at this point (see getStaticFieldCurrentClass impl). - tree->gtFlags |= (GTF_IND_INVARIANT | GTF_IND_NONFAULTING | GTF_IND_NONNULL); - } + tlsRef = gtNewOperNode(GT_IND, TYP_I_IMPL, tlsRef); - return fgMorphSmpOp(tree, /* mac */ nullptr); - } + if (dllRef != nullptr) + { + // Add the dllRef. + tlsRef = gtNewOperNode(GT_ADD, TYP_I_IMPL, tlsRef, dllRef); } - noway_assert(tree->OperIs(GT_IND)); + // indirect to have tlsRef point at the base of the DLLs Thread Local Storage. + tlsRef = gtNewOperNode(GT_IND, TYP_I_IMPL, tlsRef); - // Pass down the current mac; if non null we are computing an address - GenTree* result = fgMorphSmpOp(tree, mac); + // Add the TLS static field offset to the address. + assert(!tree->AsField()->gtFldMayOverlap); + FieldSeq* fieldSeq = GetFieldSeqStore()->Create(fieldHandle, fieldOffset, FieldSeq::FieldKind::SimpleStatic); + GenTree* offsetNode = gtNewIconNode(fieldOffset, fieldSeq); - JITDUMP("\nFinal value of Compiler::fgMorphField after calling fgMorphSmpOp:\n"); - DISPTREE(result); + tree->ChangeOper(GT_ADD); + tree->AsOp()->gtOp1 = tlsRef; + tree->AsOp()->gtOp2 = offsetNode; - return result; + return tree; } //------------------------------------------------------------------------------ @@ -9875,6 +9884,13 @@ GenTree* Compiler::fgMorphSmpOp(GenTree* tree, MorphAddrContext* mac, bool* optA case GT_FIELD: return fgMorphField(tree, mac); + case GT_FIELD_ADDR: + tree = fgMorphExpandTlsFieldAddr(tree); + oper = tree->OperGet(); + op1 = tree->AsOp()->gtOp1; + op2 = tree->AsOp()->gtOp2; + break; + case GT_INDEX_ADDR: return fgMorphIndexAddr(tree->AsIndexAddr()); diff --git a/src/coreclr/jit/rationalize.cpp b/src/coreclr/jit/rationalize.cpp index 7e2b2376391f8d..ad8139085f2ca0 100644 --- a/src/coreclr/jit/rationalize.cpp +++ b/src/coreclr/jit/rationalize.cpp @@ -53,9 +53,6 @@ void Rationalizer::RewriteIndir(LIR::Use& use) GenTreeIndir* indir = use.Def()->AsIndir(); assert(indir->OperIs(GT_IND, GT_BLK, GT_OBJ)); - // Clear the `GTF_IND_ASG_LHS` flag, which overlaps with `GTF_IND_REQ_ADDR_IN_REG`. - indir->gtFlags &= ~GTF_IND_ASG_LHS; - if (indir->OperIs(GT_IND)) { if (varTypeIsSIMD(indir)) From ab0774b3ddf7306ff030b1a930e56aa5110ee4ec Mon Sep 17 00:00:00 2001 From: SingleAccretion Date: Sun, 23 Oct 2022 15:01:41 +0300 Subject: [PATCH 05/13] Silence the IR checker --- src/coreclr/jit/morph.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/coreclr/jit/morph.cpp b/src/coreclr/jit/morph.cpp index 86e11dbb3bc79c..1fdb3cce1bac2b 100644 --- a/src/coreclr/jit/morph.cpp +++ b/src/coreclr/jit/morph.cpp @@ -5491,7 +5491,7 @@ GenTree* Compiler::fgMorphExpandTlsFieldAddr(GenTree* tree) tlsRef->gtFlags |= GTF_ICON_INITCLASS; } - tlsRef = gtNewOperNode(GT_IND, TYP_I_IMPL, tlsRef); + tlsRef = gtNewIndir(TYP_I_IMPL, tlsRef, GTF_IND_NONFAULTING | GTF_IND_INVARIANT); if (dllRef != nullptr) { From 0693d119a21f01ae0c3096683112bcb15aa2afb0 Mon Sep 17 00:00:00 2001 From: SingleAccretion Date: Sun, 23 Oct 2022 16:05:05 +0300 Subject: [PATCH 06/13] Minor code cleanup --- src/coreclr/jit/importer.cpp | 42 +++++++++++------------------------- 1 file changed, 12 insertions(+), 30 deletions(-) diff --git a/src/coreclr/jit/importer.cpp b/src/coreclr/jit/importer.cpp index 199e196b5c0102..6584e6126c81e1 100644 --- a/src/coreclr/jit/importer.cpp +++ b/src/coreclr/jit/importer.cpp @@ -4485,7 +4485,6 @@ GenTree* Compiler::impImportStaticFieldAccess(CORINFO_RESOLVED_TOKEN* pResolvedT } else // We need the value of a static field { - // In future, it may be better to just create the right tree here instead of folding it later. op1 = gtNewFieldRef(lclTyp, pResolvedToken->hField); if (pFieldInfo->fieldFlags & CORINFO_FLG_FIELD_INITCLASS) @@ -4518,9 +4517,7 @@ GenTree* Compiler::impImportStaticFieldAccess(CORINFO_RESOLVED_TOKEN* pResolvedT if (isBoxedStatic) { - op1 = gtNewOperNode(GT_IND, TYP_REF, op1); - op1->gtFlags |= (GTF_IND_INVARIANT | GTF_IND_NONFAULTING | GTF_IND_NONNULL); - + op1 = gtNewIndir(TYP_REF, op1, GTF_IND_INVARIANT | GTF_IND_NONFAULTING | GTF_IND_NONNULL); op1 = gtNewOperNode(GT_ADD, TYP_BYREF, op1, gtNewIconNode(TARGET_POINTER_SIZE, outerFldSeq)); } @@ -9390,12 +9387,10 @@ void Compiler::impImportBlockCode(BasicBlock* block) int aflags = isLoadAddress ? CORINFO_ACCESS_ADDRESS : CORINFO_ACCESS_GET; GenTree* obj = nullptr; - typeInfo* tiObj = nullptr; CORINFO_CLASS_HANDLE objType = nullptr; // used for fields - if (opcode == CEE_LDFLD || opcode == CEE_LDFLDA) + if ((opcode == CEE_LDFLD) || (opcode == CEE_LDFLDA)) { - tiObj = &impStackTop().seTypeInfo; StackEntry se = impPopStack(); objType = se.seTypeInfo.GetClassHandle(); obj = se.val; @@ -9657,8 +9652,7 @@ void Compiler::impImportBlockCode(BasicBlock* block) if (!usesHelper) { - assert((op1->OperGet() == GT_FIELD) || (op1->OperGet() == GT_IND) || - (op1->OperGet() == GT_OBJ)); + assert(op1->OperIs(GT_FIELD, GT_IND, GT_OBJ)); op1->gtFlags |= GTF_IND_VOLATILE; } } @@ -9667,15 +9661,13 @@ void Compiler::impImportBlockCode(BasicBlock* block) { if (!usesHelper) { - assert((op1->OperGet() == GT_FIELD) || (op1->OperGet() == GT_IND) || - (op1->OperGet() == GT_OBJ)); + assert(op1->OperIs(GT_FIELD, GT_IND, GT_OBJ)); op1->gtFlags |= GTF_IND_UNALIGNED; } } } - /* Check if the class needs explicit initialization */ - + // Check if the class needs explicit initialization. if (fieldInfo.fieldFlags & CORINFO_FLG_FIELD_INITCLASS) { GenTree* helperNode = impInitClass(&resolvedToken); @@ -9710,21 +9702,17 @@ void Compiler::impImportBlockCode(BasicBlock* block) JITDUMP(" %08X", resolvedToken.token); - int aflags = CORINFO_ACCESS_SET; - GenTree* obj = nullptr; - typeInfo* tiObj = nullptr; - typeInfo tiVal; + int aflags = CORINFO_ACCESS_SET; + GenTree* obj = nullptr; - /* Pull the value from the stack */ + // Pull the value from the stack. StackEntry se = impPopStack(); op2 = se.val; - tiVal = se.seTypeInfo; - clsHnd = tiVal.GetClassHandle(); + clsHnd = se.seTypeInfo.GetClassHandle(); if (opcode == CEE_STFLD) { - tiObj = &impStackTop().seTypeInfo; - obj = impPopStack().val; + obj = impPopStack().val; if (impIsThis(obj)) { @@ -9948,14 +9936,8 @@ void Compiler::impImportBlockCode(BasicBlock* block) } #endif - // We can generate an assignment to a TYP_FLOAT from a TYP_DOUBLE - // We insert a cast to the dest 'op1' type - // - if ((op1->TypeGet() != op2->TypeGet()) && varTypeIsFloating(op1->gtType) && - varTypeIsFloating(op2->gtType)) - { - op2 = gtNewCastNode(op1->TypeGet(), op2, false, op1->TypeGet()); - } + // Insert an implicit FLOAT<->DOUBLE cast if needed. + op2 = impImplicitR4orR8Cast(op2, op1->TypeGet()); op1 = gtNewAssignNode(op1, op2); } From 3d9b5bc3505747678f1ab146cca2d05c3949739a Mon Sep 17 00:00:00 2001 From: SingleAccretion Date: Sun, 23 Oct 2022 01:56:28 +0300 Subject: [PATCH 07/13] Prepare morph for instance FIELD_ADDRs --- src/coreclr/jit/compiler.h | 2 + src/coreclr/jit/gentree.h | 10 +- src/coreclr/jit/morph.cpp | 583 +++++++++++++++++++------------------ 3 files changed, 315 insertions(+), 280 deletions(-) diff --git a/src/coreclr/jit/compiler.h b/src/coreclr/jit/compiler.h index 78516c0b8d7e28..f37ed5f4655935 100644 --- a/src/coreclr/jit/compiler.h +++ b/src/coreclr/jit/compiler.h @@ -5681,7 +5681,9 @@ class Compiler private: GenTree* fgMorphField(GenTree* tree, MorphAddrContext* mac); + GenTree* fgMorphExpandInstanceField(GenTree* tree, MorphAddrContext* mac); GenTree* fgMorphExpandTlsFieldAddr(GenTree* tree); + GenTree* fgMorphExpandStaticField(GenTree* tree); bool fgCanFastTailCall(GenTreeCall* call, const char** failReason); #if FEATURE_FASTTAILCALL bool fgCallHasMustCopyByrefParameter(GenTreeCall* callee); diff --git a/src/coreclr/jit/gentree.h b/src/coreclr/jit/gentree.h index 75087d2b8c6b25..6b051beec07fae 100644 --- a/src/coreclr/jit/gentree.h +++ b/src/coreclr/jit/gentree.h @@ -4054,9 +4054,14 @@ struct GenTreeField : public GenTreeUnOp return (gtFlags & GTF_FLD_VOLATILE) != 0; } + bool IsInstance() const + { + return GetFldObj() != nullptr; + } + bool IsStatic() const { - return GetFldObj() == nullptr; + return !IsInstance(); } bool IsTlsStatic() const @@ -7016,8 +7021,7 @@ struct GenTreeIndir : public GenTreeOp void SetAddr(GenTree* addr) { - assert(addr != nullptr); - assert(addr->TypeIs(TYP_I_IMPL, TYP_BYREF)); + assert(varTypeIsI(addr)); gtOp1 = addr; } diff --git a/src/coreclr/jit/morph.cpp b/src/coreclr/jit/morph.cpp index 1fdb3cce1bac2b..550869fbd47ba1 100644 --- a/src/coreclr/jit/morph.cpp +++ b/src/coreclr/jit/morph.cpp @@ -4204,7 +4204,7 @@ void Compiler::fgMakeOutgoingStructArgCopy(GenTreeCall* call, CallArg* arg) GenTree* argNode = call->gtArgs.MakeTmpArgNode(this, arg); // Change the expression to "(tmp=val),tmp" - argNode = gtNewOperNode(GT_COMMA, argNode->TypeGet(), copyBlk, argNode); + argNode = gtNewOperNode(GT_COMMA, argNode->TypeGet(), copyBlk, argNode); #endif // !FEATURE_FIXED_OUT_ARGS @@ -4731,7 +4731,7 @@ GenTree* Compiler::fgMorphLocal(GenTreeLclVarCommon* lclNode) #ifdef TARGET_X86 expandedTree = fgMorphExpandStackArgForVarArgs(lclNode); #else - expandedTree = fgMorphExpandImplicitByRefArg(lclNode); + expandedTree = fgMorphExpandImplicitByRefArg(lclNode); #endif if (expandedTree != nullptr) @@ -5021,30 +5021,34 @@ unsigned Compiler::fgGetBigOffsetMorphingTemp(var_types type) return lclNum; } -/***************************************************************************** - * - * Transform the given GT_FIELD tree for code generation. - */ - +//------------------------------------------------------------------------ +// fgMorphField: Fully morph a FIELD/FIELD_ADDR tree. +// +// Expands the field node into explicit additions and indirections. +// +// Arguments: +// tree - The FIELD/FIELD_ADDR tree +// mac - The morphing context, used to elide adding null checks +// +// Return Value: +// The fully morphed "tree". +// GenTree* Compiler::fgMorphField(GenTree* tree, MorphAddrContext* mac) { - assert(tree->gtOper == GT_FIELD); + assert(tree->OperIs(GT_FIELD, GT_FIELD_ADDR)); - CORINFO_FIELD_HANDLE symHnd = tree->AsField()->gtFldHnd; - unsigned fldOffset = tree->AsField()->gtFldOffset; - GenTree* objRef = tree->AsField()->GetFldObj(); - bool fldMayOverlap = tree->AsField()->gtFldMayOverlap; - FieldSeq* fieldSeq = nullptr; + GenTreeField* fieldNode = tree->AsField(); + GenTree* objRef = fieldNode->GetFldObj(); - // Reset the flag because we may reuse the node. - tree->AsField()->gtFldMayOverlap = false; - - noway_assert(((objRef != nullptr) && (objRef->IsLocalAddrExpr() != nullptr)) || - ((tree->gtFlags & GTF_GLOB_REF) != 0)); + if (tree->OperIs(GT_FIELD)) + { + noway_assert(((objRef != nullptr) && (objRef->IsLocalAddrExpr() != nullptr)) || + ((tree->gtFlags & GTF_GLOB_REF) != 0)); + } #ifdef FEATURE_SIMD // if this field belongs to simd struct, translate it to simd intrinsic. - if (mac == nullptr) + if ((mac == nullptr) && tree->OperIs(GT_FIELD)) { if (IsBaselineSimdIsaSupported()) { @@ -5066,33 +5070,83 @@ GenTree* Compiler::fgMorphField(GenTree* tree, MorphAddrContext* mac) } #endif - // Create a default MorphAddrContext early so it doesn't go out of scope - // before it is used. - MorphAddrContext defMAC(MACK_Ind); + MorphAddrContext indMAC(MACK_Ind); + MorphAddrContext addrMAC(MACK_Addr); + bool isAddr = tree->OperIs(GT_FIELD_ADDR); - // Is this an instance data member? - if (objRef != nullptr) + if (fieldNode->IsInstance()) { - /* We'll create the expression "*(objRef + mem_offs)" */ + // NULL mac means we encounter the GT_FIELD/GT_FIELD_ADDR first (and don't know our parent). + if (mac == nullptr) + { + // FIELD denotes a dereference of the field, equivalent to a MACK_Ind with zero offset. + mac = tree->OperIs(GT_FIELD) ? &indMAC : &addrMAC; + } - noway_assert(varTypeIsGC(objRef->TypeGet()) || objRef->TypeGet() == TYP_I_IMPL); + tree = fgMorphExpandInstanceField(tree, mac); + } + else if (fieldNode->IsTlsStatic()) + { + tree = fgMorphExpandTlsFieldAddr(tree); + } + else + { + tree = fgMorphExpandStaticField(tree); + } - /* - Now we have a tree like this: + // Pass down the current mac; if non null we are computing an address + GenTree* result = fgMorphTree(tree, mac); + DBEXEC(result == fieldNode, result->gtDebugFlags &= ~GTF_DEBUG_NODE_MORPHED); + + // Quirk: preserve previous behavior with this NO_CSE. + if (isAddr && tree->OperIs(GT_COMMA)) + { + tree->SetDoNotCSE(); + } + + JITDUMP("\nFinal value of Compiler::fgMorphField after calling fgMorphSmpOp:\n"); + DISPTREE(result); + + return result; +} + +//------------------------------------------------------------------------ +// fgMorphExpandInstanceField: Expand an instance field reference. +// +// Expands the field node into explicit additions and indirections, adding +// explicit null checks if necessary. +// +// Arguments: +// tree - The FIELD/FIELD_ADDR tree +// mac - The morphing context, used to elide adding null checks +// +// Return Value: +// The expanded "tree" of an arbitrary shape. +// +GenTree* Compiler::fgMorphExpandInstanceField(GenTree* tree, MorphAddrContext* mac) +{ + assert(tree->OperIs(GT_FIELD, GT_FIELD_ADDR) && tree->AsField()->IsInstance()); + + GenTree* objRef = tree->AsField()->GetFldObj(); + CORINFO_FIELD_HANDLE fieldHandle = tree->AsField()->gtFldHnd; + unsigned fieldOffset = tree->AsField()->gtFldOffset; + + noway_assert(varTypeIsI(genActualType(objRef))); + + /* Now we have a tree like this: +--------------------+ - | GT_FIELD | tree + | GT_FIELD[_ADDR] | tree +----------+---------+ | +--------------+-------------+ |tree->AsField()->GetFldObj()| +--------------+-------------+ - We want to make it like this (when fldOffset is <= MAX_UNCHECKED_OFFSET_FOR_NULL_OBJECT): +--------------------+ - | GT_IND/GT_OBJ | tree + | GT_IND/GT_OBJ | tree (for FIELD) +---------+----------+ | | @@ -5103,37 +5157,37 @@ GenTree* Compiler::fgMorphField(GenTree* tree, MorphAddrContext* mac) / \ / \ / \ - +-------------------+ +----------------------+ - | objRef | | fldOffset | - | | | (when fldOffset !=0) | - +-------------------+ +----------------------+ + +-------------------+ +----------------------+ + | objRef | | fldOffset | + | | | (when fldOffset !=0) | + +-------------------+ +----------------------+ or this (when fldOffset is > MAX_UNCHECKED_OFFSET_FOR_NULL_OBJECT): +--------------------+ - | GT_IND/GT_OBJ | tree + | GT_IND/GT_OBJ | tree (for FIELD) +----------+---------+ | +----------+---------+ - | GT_COMMA | comma2 + | GT_COMMA | comma2 +----------+---------+ | / \ / \ / \ / \ - +---------+----------+ +---------+----------+ - comma | GT_COMMA | | "+" (i.e. GT_ADD) | addr - +---------+----------+ +---------+----------+ - | | - / \ / \ - / \ / \ - / \ / \ - +-----+-----+ +-----+-----+ +---------+ +-----------+ - asg | GT_ASG | ind | GT_IND | | tmpLcl | | fldOffset | - +-----+-----+ +-----+-----+ +---------+ +-----------+ + +---------+----------+ +---------+----------+ + comma | GT_COMMA | | "+" (i.e. GT_ADD) | addr + +---------+----------+ +---------+----------+ + | | + / \ / \ + / \ / \ + / \ / \ + +-----+-----+ +-----+-----+ +---------+ +-----------+ + asg | GT_ASG | ind | GT_IND | | tmpLcl | | fldOffset | + +-----+-----+ +-----+-----+ +---------+ +-----------+ | | / \ | / \ | @@ -5142,287 +5196,172 @@ GenTree* Compiler::fgMorphField(GenTree* tree, MorphAddrContext* mac) | tmpLcl | | objRef | | tmpLcl | +-----------+ +-----------+ +-----------+ + */ + + var_types objRefType = objRef->TypeGet(); + GenTree* addr = nullptr; + GenTree* comma = nullptr; + + // This flag is set to enable the "conservative" style of explicit null-check insertion. + // This means that we insert an explicit null check whenever we create byref by adding a + // constant offset to a ref, in a MACK_Addr context (meaning that the byref is not immediately + // dereferenced). The alternative is "aggressive", which would not insert such checks (for + // small offsets); in this plan, we would transfer some null-checking responsibility to + // callee's of methods taking byref parameters. They would have to add explicit null checks + // when creating derived byrefs from argument byrefs by adding constants to argument byrefs, in + // contexts where the resulting derived byref is not immediately dereferenced (or if the offset is too + // large). To make the "aggressive" scheme work, however, we'd also have to add explicit derived-from-null + // checks for byref parameters to "external" methods implemented in C++, and in P/Invoke stubs. + // This is left here to point out how to implement it. + CLANG_FORMAT_COMMENT_ANCHOR; - */ +#define CONSERVATIVE_NULL_CHECK_BYREF_CREATION 1 - var_types objRefType = objRef->TypeGet(); - GenTree* addr = nullptr; - GenTree* comma = nullptr; + bool addExplicitNullCheck = false; - // NULL mac means we encounter the GT_FIELD first. This denotes a dereference of the field, - // and thus is equivalent to a MACK_Ind with zero offset. - if (mac == nullptr) + if (fgAddrCouldBeNull(objRef)) + { + if (!mac->m_allConstantOffsets || fgIsBigOffset(mac->m_totalOffset + fieldOffset)) { - mac = &defMAC; + addExplicitNullCheck = true; } - - // This flag is set to enable the "conservative" style of explicit null-check insertion. - // This means that we insert an explicit null check whenever we create byref by adding a - // constant offset to a ref, in a MACK_Addr context (meaning that the byref is not immediately - // dereferenced). The alternative is "aggressive", which would not insert such checks (for - // small offsets); in this plan, we would transfer some null-checking responsibility to - // callee's of methods taking byref parameters. They would have to add explicit null checks - // when creating derived byrefs from argument byrefs by adding constants to argument byrefs, in - // contexts where the resulting derived byref is not immediately dereferenced (or if the offset is too - // large). To make the "aggressive" scheme work, however, we'd also have to add explicit derived-from-null - // checks for byref parameters to "external" methods implemented in C++, and in P/Invoke stubs. - // This is left here to point out how to implement it. - CLANG_FORMAT_COMMENT_ANCHOR; - -#define CONSERVATIVE_NULL_CHECK_BYREF_CREATION 1 - - bool addExplicitNullCheck = false; - - // Implicit byref locals and string literals are never null. - if (fgAddrCouldBeNull(objRef)) + else { - // If the objRef is a GT_ADDR node, it, itself, never requires null checking. The expression - // whose address is being taken is either a local or static variable, whose address is necessarily - // non-null, or else it is a field dereference, which will do its own bounds checking if necessary. - if (objRef->gtOper != GT_ADDR && (mac->m_kind == MACK_Addr || mac->m_kind == MACK_Ind)) - { - if (!mac->m_allConstantOffsets || fgIsBigOffset(mac->m_totalOffset + fldOffset)) - { - addExplicitNullCheck = true; - } - else - { - // In R2R mode the field offset for some fields may change when the code - // is loaded. So we can't rely on a zero offset here to suppress the null check. - // - // See GitHub issue #16454. - bool fieldHasChangeableOffset = false; + // In R2R mode the field offset for some fields may change when the code + // is loaded. So we can't rely on a zero offset here to suppress the null check. + // + // See GitHub issue #16454. + bool fieldHasChangeableOffset = false; #ifdef FEATURE_READYTORUN - fieldHasChangeableOffset = (tree->AsField()->gtFieldLookup.addr != nullptr); + fieldHasChangeableOffset = (tree->AsField()->gtFieldLookup.addr != nullptr); #endif #if CONSERVATIVE_NULL_CHECK_BYREF_CREATION - addExplicitNullCheck = (mac->m_kind == MACK_Addr) && - ((mac->m_totalOffset + fldOffset > 0) || fieldHasChangeableOffset); + addExplicitNullCheck = + (mac->m_kind == MACK_Addr) && ((mac->m_totalOffset + fieldOffset > 0) || fieldHasChangeableOffset); #else - addExplicitNullCheck = (objRef->gtType == TYP_BYREF && mac->m_kind == MACK_Addr && - ((mac->m_totalOffset + fldOffset > 0) || fieldHasChangeableOffset)); + addExplicitNullCheck = (objRef->gtType == TYP_BYREF && mac->m_kind == MACK_Addr && + ((mac->m_totalOffset + fldOffset > 0) || fieldHasChangeableOffset)); #endif - } - } } + } - if (addExplicitNullCheck) - { -#ifdef DEBUG - if (verbose) - { - printf("Before explicit null check morphing:\n"); - gtDispTree(tree); - } -#endif - - // - // Create the "comma" subtree - // - GenTree* asg = nullptr; + if (addExplicitNullCheck) + { + JITDUMP("Before explicit null check morphing:\n"); + DISPTREE(tree); - unsigned lclNum; + // Create the "comma" subtree. + GenTree* asg = nullptr; - if (!objRef->OperIs(GT_LCL_VAR) || lvaIsLocalImplicitlyAccessedByRef(objRef->AsLclVar()->GetLclNum())) - { - lclNum = fgGetBigOffsetMorphingTemp(genActualType(objRef->TypeGet())); + unsigned lclNum; - // Create the "asg" node - asg = gtNewTempAssign(lclNum, objRef); - } - else - { - lclNum = objRef->AsLclVarCommon()->GetLclNum(); - } - - GenTree* lclVar = gtNewLclvNode(lclNum, objRefType); - GenTree* nullchk = gtNewNullCheck(lclVar, compCurBB); - - if (asg != nullptr) - { - // Create the "comma" node. - comma = gtNewOperNode(GT_COMMA, TYP_VOID, asg, nullchk); - } - else - { - comma = nullchk; - } + if (!objRef->OperIs(GT_LCL_VAR) || lvaIsLocalImplicitlyAccessedByRef(objRef->AsLclVar()->GetLclNum())) + { + lclNum = fgGetBigOffsetMorphingTemp(genActualType(objRef->TypeGet())); - addr = gtNewLclvNode(lclNum, objRefType); // Use "tmpLcl" to create "addr" node. + // Create the "asg" node + asg = gtNewTempAssign(lclNum, objRef); } else { - addr = objRef; - } - -#ifdef FEATURE_READYTORUN - if (tree->AsField()->gtFieldLookup.addr != nullptr) - { - GenTree* offsetNode = nullptr; - if (tree->AsField()->gtFieldLookup.accessType == IAT_PVALUE) - { - offsetNode = gtNewIndOfIconHandleNode(TYP_I_IMPL, (size_t)tree->AsField()->gtFieldLookup.addr, - GTF_ICON_CONST_PTR, true); -#ifdef DEBUG - offsetNode->gtGetOp1()->AsIntCon()->gtTargetHandle = (size_t)symHnd; -#endif - } - else - { - noway_assert(!"unexpected accessType for R2R field access"); - } - - var_types addType = (objRefType == TYP_I_IMPL) ? TYP_I_IMPL : TYP_BYREF; - addr = gtNewOperNode(GT_ADD, addType, addr, offsetNode); + lclNum = objRef->AsLclVarCommon()->GetLclNum(); } -#endif - // We only need to attach the field offset information for class fields. - if ((objRefType == TYP_REF) && !fldMayOverlap) - { - fieldSeq = GetFieldSeqStore()->Create(symHnd, fldOffset, FieldSeq::FieldKind::Instance); - } + GenTree* lclVar = gtNewLclvNode(lclNum, objRefType); + GenTree* nullchk = gtNewNullCheck(lclVar, compCurBB); - // Add the member offset to the object's address. - if (fldOffset != 0) + if (asg != nullptr) { - addr = gtNewOperNode(GT_ADD, (objRefType == TYP_I_IMPL) ? TYP_I_IMPL : TYP_BYREF, addr, - gtNewIconNode(fldOffset, fieldSeq)); + // Create the "comma" node. + comma = gtNewOperNode(GT_COMMA, TYP_VOID, asg, nullchk); } - - // Now let's set the "tree" as a GT_IND tree. - - tree->SetOper(GT_IND); - tree->AsOp()->gtOp1 = addr; - - if (addExplicitNullCheck) + else { - // - // Create "comma2" node and link it to "tree". - // - GenTree* comma2 = gtNewOperNode(GT_COMMA, addr->TypeGet(), comma, addr); - tree->AsOp()->gtOp1 = comma2; + comma = nullchk; } -#ifdef DEBUG - if (verbose) - { - if (addExplicitNullCheck) - { - printf("After adding explicit null check:\n"); - gtDispTree(tree); - } - } -#endif + addr = gtNewLclvNode(lclNum, objRefType); // Use "tmpLcl" to create "addr" node. } else { - // Normal static field reference - // - // If we can we access the static's address directly - // then pFldAddr will be NULL and - // fldAddr will be the actual address of the static field - // - void** pFldAddr = nullptr; - void* fldAddr = info.compCompHnd->getFieldAddress(symHnd, (void**)&pFldAddr); - - // We should always be able to access this static field address directly - // - assert(pFldAddr == nullptr); - - // For boxed statics, this direct address will be for the box. We have already added - // the indirection for the field itself and attached the sequence, in importation. - bool isBoxedStatic = gtIsStaticFieldPtrToBoxedStruct(tree->TypeGet(), symHnd); - if (!isBoxedStatic) - { - // Only simple statics get importred as GT_FIELDs. - fieldSeq = GetFieldSeqStore()->Create(symHnd, reinterpret_cast(fldAddr), - FieldSeq::FieldKind::SimpleStatic); - } - - // TODO-CQ: enable this optimization for 32 bit targets. - bool isStaticReadOnlyInited = false; -#ifdef TARGET_64BIT - if (tree->TypeIs(TYP_REF) && !isBoxedStatic) - { - bool pIsSpeculative = true; - if (info.compCompHnd->getStaticFieldCurrentClass(symHnd, &pIsSpeculative) != NO_CLASS_HANDLE) - { - isStaticReadOnlyInited = !pIsSpeculative; - } - } -#endif // TARGET_64BIT + addr = objRef; + } - GenTreeFlags handleKind = GTF_EMPTY; - if (isBoxedStatic) - { - handleKind = GTF_ICON_STATIC_BOX_PTR; - } - else if (isStaticReadOnlyInited) +#ifdef FEATURE_READYTORUN + if (tree->AsField()->gtFieldLookup.addr != nullptr) + { + GenTree* offsetNode = nullptr; + if (tree->AsField()->gtFieldLookup.accessType == IAT_PVALUE) { - handleKind = GTF_ICON_CONST_PTR; + offsetNode = gtNewIndOfIconHandleNode(TYP_I_IMPL, (size_t)tree->AsField()->gtFieldLookup.addr, + GTF_ICON_CONST_PTR, true); +#ifdef DEBUG + offsetNode->gtGetOp1()->AsIntCon()->gtTargetHandle = (size_t)fieldHandle; +#endif } else { - handleKind = GTF_ICON_STATIC_HDL; + noway_assert(!"unexpected accessType for R2R field access"); } - GenTreeIntCon* addr = gtNewIconHandleNode((size_t)fldAddr, handleKind, fieldSeq); - INDEBUG(addr->gtTargetHandle = reinterpret_cast(symHnd)); - - // Translate GTF_FLD_INITCLASS to GTF_ICON_INITCLASS, if we need to. - if (((tree->gtFlags & GTF_FLD_INITCLASS) != 0) && !isStaticReadOnlyInited) - { - tree->gtFlags &= ~GTF_FLD_INITCLASS; - addr->gtFlags |= GTF_ICON_INITCLASS; - } - - tree->SetOper(GT_IND); - tree->AsOp()->gtOp1 = addr; - if (isBoxedStatic) - { - // The box for the static cannot be null, and is logically invariant, since it - // represents (a base for) the static's address. - tree->gtFlags |= (GTF_IND_INVARIANT | GTF_IND_NONFAULTING | GTF_IND_NONNULL); - } - else if (isStaticReadOnlyInited) - { - JITDUMP("Marking initialized static read-only field '%s' as invariant.\n", eeGetFieldName(symHnd)); + addr = gtNewOperNode(GT_ADD, (objRefType == TYP_I_IMPL) ? TYP_I_IMPL : TYP_BYREF, addr, offsetNode); + } +#endif - // Static readonly field is not null at this point (see getStaticFieldCurrentClass impl). - tree->gtFlags |= (GTF_IND_INVARIANT | GTF_IND_NONFAULTING | GTF_IND_NONNULL); - } + // We only need to attach the field offset information for class fields. + FieldSeq* fieldSeq = nullptr; + if ((objRefType == TYP_REF) && !tree->AsField()->gtFldMayOverlap) + { + fieldSeq = GetFieldSeqStore()->Create(fieldHandle, fieldOffset, FieldSeq::FieldKind::Instance); + } - return fgMorphSmpOp(tree, /* mac */ nullptr); + // Add the member offset to the object's address. + if (fieldOffset != 0) + { + addr = gtNewOperNode(GT_ADD, (objRefType == TYP_I_IMPL) ? TYP_I_IMPL : TYP_BYREF, addr, + gtNewIconNode(fieldOffset, fieldSeq)); } - noway_assert(tree->OperIs(GT_IND)); + if (addExplicitNullCheck) + { + // Create the "comma2" tree. + addr = gtNewOperNode(GT_COMMA, addr->TypeGet(), comma, addr); + } - // Pass down the current mac; if non null we are computing an address - GenTree* result = fgMorphSmpOp(tree, mac); + if (tree->OperIs(GT_FIELD)) + { + tree->SetOper(GT_IND); + tree->AsIndir()->SetAddr(addr); + } + else // Otherwise, we have a FIELD_ADDR. + { + tree = addr; + } - JITDUMP("\nFinal value of Compiler::fgMorphField after calling fgMorphSmpOp:\n"); - DISPTREE(result); + if (addExplicitNullCheck) + { + JITDUMP("After adding explicit null check:\n"); + DISPTREE(tree); + } - return result; + return tree; } //------------------------------------------------------------------------ // fgMorphExpandTlsFieldAddr: Expand a TLS field address. // +// Expands ".tls"-style statics, produced by the C++/CLI compiler for +// "__declspec(thread)" variables. An overview of the underlying native +// mechanism can be found here: http://www.nynaeve.net/?p=180. +// // Arguments: // tree - The GT_FIELD_ADDR tree // // Return Value: // The expanded tree - a GT_ADD. // -// Notes: -// This expands ".tls"-style statics, produced by the C++/CLI compiler -// for "__declspec(thread)" variables. An overview of the underlying -// (native) mechanism can be found here: http://www.nynaeve.net/?p=180. -// GenTree* Compiler::fgMorphExpandTlsFieldAddr(GenTree* tree) { // Note we do not support "FIELD"s for TLS statics, for simplicity. @@ -5514,6 +5453,102 @@ GenTree* Compiler::fgMorphExpandTlsFieldAddr(GenTree* tree) return tree; } +//------------------------------------------------------------------------ +// fgMorphExpandStaticField: Expand a simple static field load. +// +// Transforms the field into an explicit indirection off of a constant +// address. +// +// Arguments: +// tree - The GT_FIELD tree +// +// Return Value: +// The expanded tree - a GT_IND. +// +GenTree* Compiler::fgMorphExpandStaticField(GenTree* tree) +{ + // Note we do not support "FIELD_ADDR"s for simple statics. + assert(tree->OperIs(GT_FIELD) && tree->AsField()->IsStatic()); + + // If we can we access the static's address directly + // then pFldAddr will be NULL and + // fldAddr will be the actual address of the static field + // + CORINFO_FIELD_HANDLE fieldHandle = tree->AsField()->gtFldHnd; + void** pFldAddr = nullptr; + void* fldAddr = info.compCompHnd->getFieldAddress(fieldHandle, (void**)&pFldAddr); + + // We should always be able to access this static field address directly + // + assert(pFldAddr == nullptr); + + // For boxed statics, this direct address will be for the box. We have already added + // the indirection for the field itself and attached the sequence, in importation. + FieldSeq* fieldSeq = nullptr; + bool isBoxedStatic = gtIsStaticFieldPtrToBoxedStruct(tree->TypeGet(), fieldHandle); + if (!isBoxedStatic) + { + // Only simple statics get importred as GT_FIELDs. + fieldSeq = GetFieldSeqStore()->Create(fieldHandle, reinterpret_cast(fldAddr), + FieldSeq::FieldKind::SimpleStatic); + } + + // TODO-CQ: enable this optimization for 32 bit targets. + bool isStaticReadOnlyInited = false; +#ifdef TARGET_64BIT + if (tree->TypeIs(TYP_REF) && !isBoxedStatic) + { + bool pIsSpeculative = true; + if (info.compCompHnd->getStaticFieldCurrentClass(fieldHandle, &pIsSpeculative) != NO_CLASS_HANDLE) + { + isStaticReadOnlyInited = !pIsSpeculative; + } + } +#endif // TARGET_64BIT + + GenTreeFlags handleKind = GTF_EMPTY; + if (isBoxedStatic) + { + handleKind = GTF_ICON_STATIC_BOX_PTR; + } + else if (isStaticReadOnlyInited) + { + handleKind = GTF_ICON_CONST_PTR; + } + else + { + handleKind = GTF_ICON_STATIC_HDL; + } + GenTreeIntCon* addr = gtNewIconHandleNode((size_t)fldAddr, handleKind, fieldSeq); + INDEBUG(addr->gtTargetHandle = reinterpret_cast(fieldHandle)); + + // Translate GTF_FLD_INITCLASS to GTF_ICON_INITCLASS, if we need to. + if (((tree->gtFlags & GTF_FLD_INITCLASS) != 0) && !isStaticReadOnlyInited) + { + tree->gtFlags &= ~GTF_FLD_INITCLASS; + addr->gtFlags |= GTF_ICON_INITCLASS; + } + + tree->SetOper(GT_IND); + tree->AsOp()->gtOp1 = addr; + + if (isBoxedStatic) + { + // The box for the static cannot be null, and is logically invariant, since it + // represents (a base for) the static's address. + tree->gtFlags |= (GTF_IND_INVARIANT | GTF_IND_NONFAULTING | GTF_IND_NONNULL); + } + else if (isStaticReadOnlyInited) + { + JITDUMP("Marking initialized static read-only field '%s' as invariant.\n", eeGetFieldName(fieldHandle)); + + // Static readonly field is not null at this point (see getStaticFieldCurrentClass impl). + tree->gtFlags |= (GTF_IND_INVARIANT | GTF_IND_NONFAULTING | GTF_IND_NONNULL); + } + + return tree; +} + //------------------------------------------------------------------------------ // fgMorphCallInline: attempt to inline a call // @@ -9882,14 +9917,8 @@ GenTree* Compiler::fgMorphSmpOp(GenTree* tree, MorphAddrContext* mac, bool* optA break; case GT_FIELD: - return fgMorphField(tree, mac); - case GT_FIELD_ADDR: - tree = fgMorphExpandTlsFieldAddr(tree); - oper = tree->OperGet(); - op1 = tree->AsOp()->gtOp1; - op2 = tree->AsOp()->gtOp2; - break; + return fgMorphField(tree, mac); case GT_INDEX_ADDR: return fgMorphIndexAddr(tree->AsIndexAddr()); From 16fb23c6f5d7c53774e8c9cb24595193c351709d Mon Sep 17 00:00:00 2001 From: SingleAccretion Date: Sun, 23 Oct 2022 19:14:53 +0300 Subject: [PATCH 08/13] Fix ObjectAllocator --- src/coreclr/jit/objectalloc.cpp | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/coreclr/jit/objectalloc.cpp b/src/coreclr/jit/objectalloc.cpp index 1f251f48d38d17..94072a00eb797a 100644 --- a/src/coreclr/jit/objectalloc.cpp +++ b/src/coreclr/jit/objectalloc.cpp @@ -656,6 +656,7 @@ bool ObjectAllocator::CanLclVarEscapeViaParentStack(ArrayStack* parent case GT_COLON: case GT_QMARK: case GT_ADD: + case GT_FIELD_ADDR: // Check whether the local escapes via its grandparent. ++parentIndex; keepChecking = true; @@ -761,6 +762,7 @@ void ObjectAllocator::UpdateAncestorTypes(GenTree* tree, ArrayStack* p case GT_COLON: case GT_QMARK: case GT_ADD: + case GT_FIELD_ADDR: if (parent->TypeGet() == TYP_REF) { parent->ChangeType(newType); From b621b7bf064190526346e814e4e6bfcb9a24ac93 Mon Sep 17 00:00:00 2001 From: SingleAccretion Date: Sun, 23 Oct 2022 03:48:46 +0300 Subject: [PATCH 09/13] Use FIELD_ADDR in ld[s]flda import --- src/coreclr/jit/importer.cpp | 35 +++++++++++++++++++++-------------- 1 file changed, 21 insertions(+), 14 deletions(-) diff --git a/src/coreclr/jit/importer.cpp b/src/coreclr/jit/importer.cpp index 6584e6126c81e1..b041d89c5d2f89 100644 --- a/src/coreclr/jit/importer.cpp +++ b/src/coreclr/jit/importer.cpp @@ -9512,8 +9512,19 @@ void Compiler::impImportBlockCode(BasicBlock* block) obj = impGetStructAddr(obj, objType, CHECK_SPILL_ALL, true); } - /* Create the data member node */ - op1 = gtNewFieldRef(lclTyp, resolvedToken.hField, obj, fieldInfo.offset); + DWORD typeFlags = info.compCompHnd->getClassAttribs(resolvedToken.hClass); + + // TODO-ADDR: use FIELD_ADDR for all fields, not just those of classes. + // + if (isLoadAddress && ((typeFlags & CORINFO_FLG_VALUECLASS) == 0)) + { + op1 = gtNewFieldAddrNode(varTypeIsGC(obj) ? TYP_BYREF : TYP_I_IMPL, resolvedToken.hField, + obj, fieldInfo.offset); + } + else + { + op1 = gtNewFieldRef(lclTyp, resolvedToken.hField, obj, fieldInfo.offset); + } #ifdef FEATURE_READYTORUN if (fieldInfo.fieldAccessor == CORINFO_FIELD_INSTANCE_WITH_BASE) @@ -9527,26 +9538,22 @@ void Compiler::impImportBlockCode(BasicBlock* block) op1->gtFlags |= GTF_EXCEPT; } - DWORD typeFlags = info.compCompHnd->getClassAttribs(resolvedToken.hClass); if (StructHasOverlappingFields(typeFlags)) { op1->AsField()->gtFldMayOverlap = true; } - // wrap it in a address of operator if necessary - if (isLoadAddress) + // Wrap it in a address of operator if necessary. + if (isLoadAddress && op1->OperIs(GT_FIELD)) { - op1 = gtNewOperNode(GT_ADDR, - (var_types)(varTypeIsGC(obj->TypeGet()) ? TYP_BYREF : TYP_I_IMPL), op1); + op1 = gtNewOperNode(GT_ADDR, varTypeIsGC(obj) ? TYP_BYREF : TYP_I_IMPL, op1); } - else + + if (!isLoadAddress && compIsForInlining() && + impInlineIsGuaranteedThisDerefBeforeAnySideEffects(nullptr, nullptr, obj, + impInlineInfo->inlArgInfo)) { - if (compIsForInlining() && - impInlineIsGuaranteedThisDerefBeforeAnySideEffects(nullptr, nullptr, obj, - impInlineInfo->inlArgInfo)) - { - impInlineInfo->thisDereferencedFirst = true; - } + impInlineInfo->thisDereferencedFirst = true; } } break; From 52ef7a65e3d3f068f6d120e5e851b768e5848864 Mon Sep 17 00:00:00 2001 From: SingleAccretion Date: Mon, 24 Oct 2022 20:29:40 +0300 Subject: [PATCH 10/13] Work around morphing issues With NativeAOT, we can have FIELD_ADDR nodes that are effectively NOPs (those for the method table pointer field). This means not all operators that the expansion produces will be simple (in fact, they can be more or less arbitrary). This means we cannot simply call "fgMorphSmpOp" as we used to. Unfortunately, we cannot just call "fgMorphTree" either, because it propagates assertions on newly created (or, I suppose, bashed) IND nodes. This creates lots of new cases where GTF_ORDER_SIDEEFF is applied to these nodes, affecting CQ. Work around this by calling "fgMorphTree" for non-simple operators only. --- src/coreclr/jit/morph.cpp | 21 +++++++++++++++------ 1 file changed, 15 insertions(+), 6 deletions(-) diff --git a/src/coreclr/jit/morph.cpp b/src/coreclr/jit/morph.cpp index 550869fbd47ba1..a5befd3f77f748 100644 --- a/src/coreclr/jit/morph.cpp +++ b/src/coreclr/jit/morph.cpp @@ -5095,16 +5095,25 @@ GenTree* Compiler::fgMorphField(GenTree* tree, MorphAddrContext* mac) } // Pass down the current mac; if non null we are computing an address - GenTree* result = fgMorphTree(tree, mac); - DBEXEC(result == fieldNode, result->gtDebugFlags &= ~GTF_DEBUG_NODE_MORPHED); + GenTree* result; + if (tree->OperIsSimple()) + { + result = fgMorphSmpOp(tree, mac); + DBEXEC(result != fieldNode, result->gtDebugFlags |= GTF_DEBUG_NODE_MORPHED); - // Quirk: preserve previous behavior with this NO_CSE. - if (isAddr && tree->OperIs(GT_COMMA)) + // Quirk: preserve previous behavior with this NO_CSE. + if (isAddr && result->OperIs(GT_COMMA)) + { + result->SetDoNotCSE(); + } + } + else { - tree->SetDoNotCSE(); + result = fgMorphTree(tree, mac); + DBEXEC(result == fieldNode, result->gtDebugFlags &= ~GTF_DEBUG_NODE_MORPHED); } - JITDUMP("\nFinal value of Compiler::fgMorphField after calling fgMorphSmpOp:\n"); + JITDUMP("\nFinal value of Compiler::fgMorphField after morphing:\n"); DISPTREE(result); return result; From f5396d3740372c9502e7e5840678c6d505742cb2 Mon Sep 17 00:00:00 2001 From: SingleAccretion Date: Mon, 31 Oct 2022 23:50:39 +0300 Subject: [PATCH 11/13] Actually fix merge conflicts --- src/coreclr/jit/morph.cpp | 26 +++++--------------------- 1 file changed, 5 insertions(+), 21 deletions(-) diff --git a/src/coreclr/jit/morph.cpp b/src/coreclr/jit/morph.cpp index bc1b1ef45cb59c..2ba9e5f396f83a 100644 --- a/src/coreclr/jit/morph.cpp +++ b/src/coreclr/jit/morph.cpp @@ -4124,7 +4124,7 @@ void Compiler::fgMakeOutgoingStructArgCopy(GenTreeCall* call, CallArg* arg) GenTree* argNode = call->gtArgs.MakeTmpArgNode(this, arg); // Change the expression to "(tmp=val),tmp" - argNode = gtNewOperNode(GT_OCMMA, argNode->TypeGet(), copyBlk, argNode); + argNode = gtNewOperNode(GT_COMMA, argNode->TypeGet(), copyBlk, argNode); #endif // !FEATURE_FIXED_OUT_ARGS @@ -5127,26 +5127,10 @@ GenTree* Compiler::fgMorphExpandInstanceField(GenTree* tree, MorphAddrContext* m */ - var_types objRefType = objRef->TypeGet(); - GenTree* addr = nullptr; - GenTree* comma = nullptr; - - // This flag is set to enable the "conservative" style of explicit null-check insertion. - // This means that we insert an explicit null check whenever we create byref by adding a - // constant offset to a ref, in a MACK_Addr context (meaning that the byref is not immediately - // dereferenced). The alternative is "aggressive", which would not insert such checks (for - // small offsets); in this plan, we would transfer some null-checking responsibility to - // callee's of methods taking byref parameters. They would have to add explicit null checks - // when creating derived byrefs from argument byrefs by adding constants to argument byrefs, in - // contexts where the resulting derived byref is not immediately dereferenced (or if the offset is too - // large). To make the "aggressive" scheme work, however, we'd also have to add explicit derived-from-null - // checks for byref parameters to "external" methods implemented in C++, and in P/Invoke stubs. - // This is left here to point out how to implement it. - CLANG_FORMAT_COMMENT_ANCHOR; - -#define CONSERVATIVE_NULL_CHECK_BYREF_CREATION 1 - - bool addExplicitNullCheck = false; + var_types objRefType = objRef->TypeGet(); + GenTree* addr = nullptr; + GenTree* comma = nullptr; + bool addExplicitNullCheck = false; if (fgAddrCouldBeNull(objRef)) { From 778cf2b51b9068823cb8ee389e6ed83518275552 Mon Sep 17 00:00:00 2001 From: SingleAccretion Date: Tue, 1 Nov 2022 00:19:18 +0300 Subject: [PATCH 12/13] Clang format knows better --- src/coreclr/jit/morph.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/coreclr/jit/morph.cpp b/src/coreclr/jit/morph.cpp index 2ba9e5f396f83a..59384d2db7f8b2 100644 --- a/src/coreclr/jit/morph.cpp +++ b/src/coreclr/jit/morph.cpp @@ -4124,7 +4124,7 @@ void Compiler::fgMakeOutgoingStructArgCopy(GenTreeCall* call, CallArg* arg) GenTree* argNode = call->gtArgs.MakeTmpArgNode(this, arg); // Change the expression to "(tmp=val),tmp" - argNode = gtNewOperNode(GT_COMMA, argNode->TypeGet(), copyBlk, argNode); + argNode = gtNewOperNode(GT_COMMA, argNode->TypeGet(), copyBlk, argNode); #endif // !FEATURE_FIXED_OUT_ARGS From 37d611c07f30977368efbe4c38a273dfb5c44a6e Mon Sep 17 00:00:00 2001 From: SingleAccretion <62474226+SingleAccretion@users.noreply.github.com> Date: Mon, 7 Nov 2022 01:34:00 +0300 Subject: [PATCH 13/13] Fix merge error --- src/coreclr/jit/morph.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/src/coreclr/jit/morph.cpp b/src/coreclr/jit/morph.cpp index e9b77267c0a88a..698ee12196597e 100644 --- a/src/coreclr/jit/morph.cpp +++ b/src/coreclr/jit/morph.cpp @@ -5173,6 +5173,7 @@ GenTree* Compiler::fgMorphExpandInstanceField(GenTree* tree, MorphAddrContext* m // Create the "comma" subtree. GenTree* asg = nullptr; + unsigned lclNum; if (!objRef->OperIs(GT_LCL_VAR) || lvaIsLocalImplicitlyAccessedByRef(objRef->AsLclVar()->GetLclNum())) {