From 10012c478d2eefd9d17aa11afee4f3bea4e41857 Mon Sep 17 00:00:00 2001 From: SingleAccretion <62474226+SingleAccretion@users.noreply.github.com> Date: Sat, 23 Apr 2022 01:35:47 +0300 Subject: [PATCH] Cleaning up write barrier code (#68124) * Write barrier code cleanup 1) Delete leftovers from legacy codegen, including the "tgt" naming. 2) Do not second-guess the write barrier form when selecting a helper, always return "known" WBF from "gcIsWriteBarrierCandidate" instead. * Delete GTF_IND_TGT_ANYWHERE And create GTF_IND_TGT_HEAP instead, its logical opposite. For reference, the situation before this change was as follows: // Type of the ("simplified"/"base") address: // // - TYP_I_IMPL: checked write barrier // - TYP_REF: unchecked write barrier // - TYP_BYREF: // 1) No flags: unchecked write barrier // 2) TGT_ANYWHERE: checked write barrier The TYP_BYREF case will now look like this: // // 1) No flags: checked write barrier // 2) TGT_HEAP: unchecked write barrier The main benefit is that we get the default state (no flags) to be the conservatively correct one. * Add a quirk to avoid diffs on x86 --- src/coreclr/jit/codegen.h | 2 +- src/coreclr/jit/codegenarm.cpp | 2 +- src/coreclr/jit/codegenarm64.cpp | 4 +- src/coreclr/jit/codegencommon.cpp | 148 +++++-------- src/coreclr/jit/codegeninterface.h | 4 +- src/coreclr/jit/codegenloongarch64.cpp | 2 +- src/coreclr/jit/codegenxarch.cpp | 8 +- src/coreclr/jit/compiler.cpp | 16 +- src/coreclr/jit/fgdiagnostic.cpp | 2 + src/coreclr/jit/gcinfo.cpp | 280 ++++++++++++------------- src/coreclr/jit/gentree.cpp | 10 +- src/coreclr/jit/gentree.h | 8 +- src/coreclr/jit/importer.cpp | 52 ++--- src/coreclr/jit/jitgcinfo.h | 11 +- src/coreclr/jit/lower.cpp | 5 - src/coreclr/jit/lsraarm.cpp | 2 +- src/coreclr/jit/lsraarm64.cpp | 2 +- src/coreclr/jit/lsrabuild.cpp | 11 +- src/coreclr/jit/lsraloongarch64.cpp | 2 +- src/coreclr/jit/lsraxarch.cpp | 2 +- src/coreclr/jit/morph.cpp | 19 +- src/coreclr/jit/morphblock.cpp | 4 - src/coreclr/jit/objectalloc.cpp | 19 +- src/coreclr/jit/rationalize.cpp | 11 +- src/coreclr/jit/simdcodegenxarch.cpp | 2 +- 25 files changed, 272 insertions(+), 356 deletions(-) diff --git a/src/coreclr/jit/codegen.h b/src/coreclr/jit/codegen.h index 3d77e7b05fb70..722503cd5260c 100644 --- a/src/coreclr/jit/codegen.h +++ b/src/coreclr/jit/codegen.h @@ -218,7 +218,7 @@ class CodeGen final : public CodeGenInterface protected: void genEmitHelperCall(unsigned helper, int argSize, emitAttr retSize, regNumber callTarget = REG_NA); - void genGCWriteBarrier(GenTree* tgt, GCInfo::WriteBarrierForm wbf); + void genGCWriteBarrier(GenTreeStoreInd* store, GCInfo::WriteBarrierForm wbf); BasicBlock* genCreateTempLabel(); diff --git a/src/coreclr/jit/codegenarm.cpp b/src/coreclr/jit/codegenarm.cpp index 75c4dd62eadd5..d43958a1dbfa4 100644 --- a/src/coreclr/jit/codegenarm.cpp +++ b/src/coreclr/jit/codegenarm.cpp @@ -1329,7 +1329,7 @@ void CodeGen::genCodeForStoreInd(GenTreeStoreInd* tree) assert(!varTypeIsFloating(type) || (type == data->TypeGet())); - GCInfo::WriteBarrierForm writeBarrierForm = gcInfo.gcIsWriteBarrierCandidate(tree, data); + GCInfo::WriteBarrierForm writeBarrierForm = gcInfo.gcIsWriteBarrierCandidate(tree); if (writeBarrierForm != GCInfo::WBF_NoBarrier) { // data and addr must be in registers. diff --git a/src/coreclr/jit/codegenarm64.cpp b/src/coreclr/jit/codegenarm64.cpp index d2f8daf50d44e..5fbe37a489fa2 100644 --- a/src/coreclr/jit/codegenarm64.cpp +++ b/src/coreclr/jit/codegenarm64.cpp @@ -3830,7 +3830,7 @@ void CodeGen::genCodeForStoreInd(GenTreeStoreInd* tree) GenTree* data = tree->Data(); GenTree* addr = tree->Addr(); - GCInfo::WriteBarrierForm writeBarrierForm = gcInfo.gcIsWriteBarrierCandidate(tree, data); + GCInfo::WriteBarrierForm writeBarrierForm = gcInfo.gcIsWriteBarrierCandidate(tree); if (writeBarrierForm != GCInfo::WBF_NoBarrier) { // data and addr must be in registers. @@ -4956,7 +4956,7 @@ void CodeGen::genStoreIndTypeSIMD12(GenTree* treeNode) #ifdef DEBUG // Should not require a write barrier - GCInfo::WriteBarrierForm writeBarrierForm = gcInfo.gcIsWriteBarrierCandidate(treeNode, data); + GCInfo::WriteBarrierForm writeBarrierForm = gcInfo.gcIsWriteBarrierCandidate(treeNode->AsStoreInd()); assert(writeBarrierForm == GCInfo::WBF_NoBarrier); #endif diff --git a/src/coreclr/jit/codegencommon.cpp b/src/coreclr/jit/codegencommon.cpp index acf280007f5fd..3eeda3f1a2c5c 100644 --- a/src/coreclr/jit/codegencommon.cpp +++ b/src/coreclr/jit/codegencommon.cpp @@ -2698,19 +2698,18 @@ bool CodeGenInterface::genUseOptimizedWriteBarriers(GCInfo::WriteBarrierForm wbf // determining what the required write barrier form is, if possible. // // Arguments: -// tgt - target tree of write (e.g., GT_STOREIND) -// assignVal - tree with value to write +// store - the GT_STOREIND node // // Return Value: // true if an optimized write barrier helper should be used, false otherwise. // Note: only x86 implements register-specific source optimized write // barriers currently. // -bool CodeGenInterface::genUseOptimizedWriteBarriers(GenTree* tgt, GenTree* assignVal) +bool CodeGenInterface::genUseOptimizedWriteBarriers(GenTreeStoreInd* store) { #if defined(TARGET_X86) && NOGC_WRITE_BARRIERS #ifdef DEBUG - GCInfo::WriteBarrierForm wbf = compiler->codeGen->gcInfo.gcIsWriteBarrierCandidate(tgt, assignVal); + GCInfo::WriteBarrierForm wbf = compiler->codeGen->gcInfo.gcIsWriteBarrierCandidate(store); return (wbf != GCInfo::WBF_NoBarrier_CheckNotHeapInDebug); // This one is always a call to a C++ method. #else return true; @@ -2721,12 +2720,11 @@ bool CodeGenInterface::genUseOptimizedWriteBarriers(GenTree* tgt, GenTree* assig } //---------------------------------------------------------------------- -// genWriteBarrierHelperForWriteBarrierForm: Given a write node requiring a write -// barrier, and the write barrier form required, determine the helper to call. +// genWriteBarrierHelperForWriteBarrierForm: Given a write barrier form +// return the corresponding helper. // // Arguments: -// tgt - target tree of write (e.g., GT_STOREIND) -// wbf - already computed write barrier form to use +// wbf - the write barrier form // // Return Value: // Write barrier helper to use. @@ -2734,115 +2732,81 @@ bool CodeGenInterface::genUseOptimizedWriteBarriers(GenTree* tgt, GenTree* assig // Note: do not call this function to get an optimized write barrier helper (e.g., // for x86). // -CorInfoHelpFunc CodeGenInterface::genWriteBarrierHelperForWriteBarrierForm(GenTree* tgt, GCInfo::WriteBarrierForm wbf) +CorInfoHelpFunc CodeGenInterface::genWriteBarrierHelperForWriteBarrierForm(GCInfo::WriteBarrierForm wbf) { - noway_assert(tgt->gtOper == GT_STOREIND); + switch (wbf) + { + case GCInfo::WBF_BarrierChecked: + return CORINFO_HELP_CHECKED_ASSIGN_REF; - CorInfoHelpFunc helper = CORINFO_HELP_ASSIGN_REF; + case GCInfo::WBF_BarrierUnchecked: + return CORINFO_HELP_ASSIGN_REF; #ifdef DEBUG - if (wbf == GCInfo::WBF_NoBarrier_CheckNotHeapInDebug) - { - helper = CORINFO_HELP_ASSIGN_REF_ENSURE_NONHEAP; - } - else -#endif - if (tgt->gtOper != GT_CLS_VAR) - { - if (wbf != GCInfo::WBF_BarrierUnchecked) // This overrides the tests below. - { - if (tgt->gtFlags & GTF_IND_TGTANYWHERE) - { - helper = CORINFO_HELP_CHECKED_ASSIGN_REF; - } - else if (tgt->AsOp()->gtOp1->TypeGet() == TYP_I_IMPL) - { - helper = CORINFO_HELP_CHECKED_ASSIGN_REF; - } - } - } - assert(((helper == CORINFO_HELP_ASSIGN_REF_ENSURE_NONHEAP) && (wbf == GCInfo::WBF_NoBarrier_CheckNotHeapInDebug)) || - ((helper == CORINFO_HELP_CHECKED_ASSIGN_REF) && - (wbf == GCInfo::WBF_BarrierChecked || wbf == GCInfo::WBF_BarrierUnknown)) || - ((helper == CORINFO_HELP_ASSIGN_REF) && - (wbf == GCInfo::WBF_BarrierUnchecked || wbf == GCInfo::WBF_BarrierUnknown))); + case GCInfo::WBF_NoBarrier_CheckNotHeapInDebug: + return CORINFO_HELP_ASSIGN_REF_ENSURE_NONHEAP; +#endif // DEBUG - return helper; + default: + unreached(); + } } //---------------------------------------------------------------------- // genGCWriteBarrier: Generate a write barrier for a node. // // Arguments: -// tgt - target tree of write (e.g., GT_STOREIND) -// wbf - already computed write barrier form to use +// store - the GT_STOREIND node +// wbf - already computed write barrier form to use // -void CodeGen::genGCWriteBarrier(GenTree* tgt, GCInfo::WriteBarrierForm wbf) +void CodeGen::genGCWriteBarrier(GenTreeStoreInd* store, GCInfo::WriteBarrierForm wbf) { - CorInfoHelpFunc helper = genWriteBarrierHelperForWriteBarrierForm(tgt, wbf); + CorInfoHelpFunc helper = genWriteBarrierHelperForWriteBarrierForm(wbf); #ifdef FEATURE_COUNT_GC_WRITE_BARRIERS - // We classify the "tgt" trees as follows: - // If "tgt" is of the form (where [ x ] indicates an optional x, and { x1, ..., xn } means "one of the x_i forms"): - // IND [-> ADDR -> IND] -> { GT_LCL_VAR, ADD({GT_LCL_VAR}, X), ADD(X, (GT_LCL_VAR)) } - // then let "v" be the GT_LCL_VAR. - // * If "v" is the return buffer argument, classify as CWBKind_RetBuf. - // * If "v" is another by-ref argument, classify as CWBKind_ByRefArg. - // * Otherwise, classify as CWBKind_OtherByRefLocal. - // If "tgt" is of the form IND -> ADDR -> GT_LCL_VAR, clasify as CWBKind_AddrOfLocal. - // Otherwise, classify as CWBKind_Unclassified. - - CheckedWriteBarrierKinds wbKind = CWBKind_Unclassified; - if (tgt->gtOper == GT_IND) + // Under FEATURE_COUNT_GC_WRITE_BARRIERS, we will add an extra argument to the + // checked write barrier call denoting the kind of address being written to. + // + if (helper == CORINFO_HELP_CHECKED_ASSIGN_REF) { - GenTree* lcl = NULL; + CheckedWriteBarrierKinds wbKind = CWBKind_Unclassified; + GenTree* tgtAddr = store->Addr(); - GenTree* indArg = tgt->AsOp()->gtOp1; - if (indArg->gtOper == GT_ADDR && indArg->AsOp()->gtOp1->gtOper == GT_IND) - { - indArg = indArg->AsOp()->gtOp1->AsOp()->gtOp1; - } - if (indArg->gtOper == GT_LCL_VAR) + while (tgtAddr->OperIs(GT_ADD, GT_LEA)) { - lcl = indArg; - } - else if (indArg->gtOper == GT_ADD) - { - if (indArg->AsOp()->gtOp1->gtOper == GT_LCL_VAR) + if (tgtAddr->OperIs(GT_LEA) && tgtAddr->AsAddrMode()->HasBase()) + { + tgtAddr = tgtAddr->AsAddrMode()->Base(); + } + else if (tgtAddr->OperIs(GT_ADD) && tgtAddr->AsOp()->gtGetOp2()->IsCnsIntOrI()) { - lcl = indArg->AsOp()->gtOp1; + tgtAddr = tgtAddr->AsOp()->gtGetOp1(); } - else if (indArg->AsOp()->gtOp2->gtOper == GT_LCL_VAR) + else { - lcl = indArg->AsOp()->gtOp2; + break; } } - if (lcl != NULL) + + if (tgtAddr->OperIs(GT_LCL_VAR)) { - wbKind = CWBKind_OtherByRefLocal; // Unclassified local variable. - unsigned lclNum = lcl->AsLclVar()->GetLclNum(); + unsigned lclNum = tgtAddr->AsLclVar()->GetLclNum(); + LclVarDsc* varDsc = compiler->lvaGetDesc(lclNum); if (lclNum == compiler->info.compRetBuffArg) { - wbKind = CWBKind_RetBuf; // Ret buff. Can happen if the struct exceeds the size limit. + wbKind = CWBKind_RetBuf } - else + else if (varDsc->TypeGet() == TYP_BYREF) { - const LclVarDsc* varDsc = compiler->lvaGetDesc(lclNum); - if (varDsc->lvIsParam && varDsc->lvType == TYP_BYREF) - { - wbKind = CWBKind_ByRefArg; // Out (or in/out) arg - } + wbKind = varDsc->lvIsParam ? CWBKind_ByRefArg : CWBKind_OtherByRefLocal; } } - else + else if (tgtAddr->OperIsLocalAddr()) { - // We should have eliminated the barrier for this case. - assert(!(indArg->gtOper == GT_ADDR && indArg->AsOp()->gtOp1->gtOper == GT_LCL_VAR)); + // Ideally, we should have eliminated the barrier for this case. + wbKind = CWBKind_AddrOfLocal; } - } - if (helper == CORINFO_HELP_CHECKED_ASSIGN_REF) - { #if 0 #ifdef DEBUG // Enable this to sample the unclassified trees. @@ -2850,29 +2814,27 @@ void CodeGen::genGCWriteBarrier(GenTree* tgt, GCInfo::WriteBarrierForm wbf) if (wbKind == CWBKind_Unclassified) { unclassifiedBarrierSite++; - printf("unclassifiedBarrierSite = %d:\n", unclassifiedBarrierSite); compiler->gtDispTree(tgt); printf(""); printf("\n"); + printf("unclassifiedBarrierSite = %d:\n", unclassifiedBarrierSite); + compiler->gtDispTree(store); + printf(""); // Flush. + printf("\n"); } #endif // DEBUG #endif // 0 + AddStackLevel(4); inst_IV(INS_push, wbKind); genEmitHelperCall(helper, 4, // argSize EA_PTRSIZE); // retSize SubtractStackLevel(4); + return; } - else - { - genEmitHelperCall(helper, - 0, // argSize - EA_PTRSIZE); // retSize - } +#endif // FEATURE_COUNT_GC_WRITE_BARRIERS -#else // !FEATURE_COUNT_GC_WRITE_BARRIERS genEmitHelperCall(helper, 0, // argSize EA_PTRSIZE); // retSize -#endif // !FEATURE_COUNT_GC_WRITE_BARRIERS } /* diff --git a/src/coreclr/jit/codegeninterface.h b/src/coreclr/jit/codegeninterface.h index dbd53ffbad46f..17bdcb1952f67 100644 --- a/src/coreclr/jit/codegeninterface.h +++ b/src/coreclr/jit/codegeninterface.h @@ -148,8 +148,8 @@ class CodeGenInterface public: bool genUseOptimizedWriteBarriers(GCInfo::WriteBarrierForm wbf); - bool genUseOptimizedWriteBarriers(GenTree* tgt, GenTree* assignVal); - CorInfoHelpFunc genWriteBarrierHelperForWriteBarrierForm(GenTree* tgt, GCInfo::WriteBarrierForm wbf); + bool genUseOptimizedWriteBarriers(GenTreeStoreInd* store); + CorInfoHelpFunc genWriteBarrierHelperForWriteBarrierForm(GCInfo::WriteBarrierForm wbf); // The following property indicates whether the current method sets up // an explicit stack frame or not. diff --git a/src/coreclr/jit/codegenloongarch64.cpp b/src/coreclr/jit/codegenloongarch64.cpp index 02d78a0b52d2d..2fb20f3e2835b 100644 --- a/src/coreclr/jit/codegenloongarch64.cpp +++ b/src/coreclr/jit/codegenloongarch64.cpp @@ -3579,7 +3579,7 @@ void CodeGen::genCodeForStoreInd(GenTreeStoreInd* tree) GenTree* data = tree->Data(); GenTree* addr = tree->Addr(); - GCInfo::WriteBarrierForm writeBarrierForm = gcInfo.gcIsWriteBarrierCandidate(tree, data); + GCInfo::WriteBarrierForm writeBarrierForm = gcInfo.gcIsWriteBarrierCandidate(tree); if (writeBarrierForm != GCInfo::WBF_NoBarrier) { // data and addr must be in registers. diff --git a/src/coreclr/jit/codegenxarch.cpp b/src/coreclr/jit/codegenxarch.cpp index a2759bd7e8e66..1c5cda19da022 100644 --- a/src/coreclr/jit/codegenxarch.cpp +++ b/src/coreclr/jit/codegenxarch.cpp @@ -5016,7 +5016,7 @@ void CodeGen::genCodeForStoreInd(GenTreeStoreInd* tree) assert(!varTypeIsFloating(targetType) || (genTypeSize(targetType) == genTypeSize(data->TypeGet()))); - GCInfo::WriteBarrierForm writeBarrierForm = gcInfo.gcIsWriteBarrierCandidate(tree, data); + GCInfo::WriteBarrierForm writeBarrierForm = gcInfo.gcIsWriteBarrierCandidate(tree); if (writeBarrierForm != GCInfo::WBF_NoBarrier) { // data and addr must be in registers. @@ -5284,10 +5284,8 @@ bool CodeGen::genEmitOptimizedGCWriteBarrier(GCInfo::WriteBarrierForm writeBarri tgtAnywhere = 1; } - // We might want to call a modified version of genGCWriteBarrier() to get the benefit of - // the FEATURE_COUNT_GC_WRITE_BARRIERS code there, but that code doesn't look like it works - // with rationalized RyuJIT IR. So, for now, just emit the helper call directly here. - + // Here we might want to call a modified version of genGCWriteBarrier() to get the benefit + // of the FEATURE_COUNT_GC_WRITE_BARRIERS code. For now, just emit the helper call directly. genEmitHelperCall(regToHelper[tgtAnywhere][reg], 0, // argSize EA_PTRSIZE); // retSize diff --git a/src/coreclr/jit/compiler.cpp b/src/coreclr/jit/compiler.cpp index 339fbfe9fc38a..57755adfe54bb 100644 --- a/src/coreclr/jit/compiler.cpp +++ b/src/coreclr/jit/compiler.cpp @@ -9215,6 +9215,10 @@ void cTreeFlags(Compiler* comp, GenTree* tree) { chars += printf("[FLD_VOLATILE]"); } + if (tree->gtFlags & GTF_FLD_TGT_HEAP) + { + chars += printf("[FLD_TGT_HEAP]"); + } break; case GT_INDEX: @@ -9238,14 +9242,14 @@ void cTreeFlags(Compiler* comp, GenTree* tree) { chars += printf("[IND_VOLATILE]"); } - if (tree->gtFlags & GTF_IND_TGTANYWHERE) - { - chars += printf("[IND_TGTANYWHERE]"); - } if (tree->gtFlags & GTF_IND_TGT_NOT_HEAP) { chars += printf("[IND_TGT_NOT_HEAP]"); } + if (tree->gtFlags & GTF_IND_TGT_HEAP) + { + chars += printf("[IND_TGT_HEAP]"); + } if (tree->gtFlags & GTF_IND_TLS_REF) { chars += printf("[IND_TLS_REF]"); @@ -9274,6 +9278,10 @@ void cTreeFlags(Compiler* comp, GenTree* tree) { chars += printf("[CLS_VAR_ASG_LHS]"); } + if (tree->gtFlags & GTF_CLS_VAR_TGT_HEAP) + { + chars += printf("[CLS_VAR_TGT_HEAP]"); + } break; case GT_MUL: diff --git a/src/coreclr/jit/fgdiagnostic.cpp b/src/coreclr/jit/fgdiagnostic.cpp index 99a34f8f0c96d..6328919219e7e 100644 --- a/src/coreclr/jit/fgdiagnostic.cpp +++ b/src/coreclr/jit/fgdiagnostic.cpp @@ -3033,6 +3033,8 @@ void Compiler::fgDebugCheckFlags(GenTree* tree) // Currently we expect all indirections with constant addresses to be nonfaulting. expectedFlags |= GTF_IND_NONFAULTING; } + + assert(((tree->gtFlags & GTF_IND_TGT_NOT_HEAP) == 0) || ((tree->gtFlags & GTF_IND_TGT_HEAP) == 0)); break; case GT_CALL: diff --git a/src/coreclr/jit/gcinfo.cpp b/src/coreclr/jit/gcinfo.cpp index 85e7f688c2f2e..d48f1962b5758 100644 --- a/src/coreclr/jit/gcinfo.cpp +++ b/src/coreclr/jit/gcinfo.cpp @@ -221,83 +221,167 @@ void GCInfo::gcMarkRegPtrVal(regNumber reg, var_types type) } } -/*****************************************************************************/ - -GCInfo::WriteBarrierForm GCInfo::gcIsWriteBarrierCandidate(GenTree* tgt, GenTree* assignVal) +//------------------------------------------------------------------------ +// gcIsWriteBarrierCandidate: Get the write barrier kind for the given store. +// +// Arguments: +// store - The STOREIND node in question +// +// Return Value: +// Form of the write barrier that will need to be used when generating +// code for "store". +// +GCInfo::WriteBarrierForm GCInfo::gcIsWriteBarrierCandidate(GenTreeStoreInd* store) { - /* Are we storing a GC ptr? */ - - if (!varTypeIsGC(tgt->TypeGet())) + if (!store->TypeIs(TYP_REF)) { + // Note that byref values cannot be in the managed heap. return WBF_NoBarrier; } - /* Ignore any assignments of NULL */ - - // 'assignVal' can be the constant Null or something else (LclVar, etc..) - // that is known to be null via Value Numbering. - if (assignVal->GetVN(VNK_Liberal) == ValueNumStore::VNForNull()) + // Ignore any assignments of NULL. + if ((store->Data()->GetVN(VNK_Liberal) == ValueNumStore::VNForNull()) || store->Data()->IsIntegralConst(0)) { return WBF_NoBarrier; } - if (assignVal->gtOper == GT_CNS_INT && assignVal->AsIntCon()->gtIconVal == 0) + if ((store->gtFlags & GTF_IND_TGT_NOT_HEAP) != 0) { + // This indirection is not from to the heap. + // This case occurs for stack-allocated objects. return WBF_NoBarrier; } - /* Where are we storing into? */ + WriteBarrierForm wbf = gcWriteBarrierFormFromTargetAddress(store->Addr()); - tgt = tgt->gtEffectiveVal(); + if (wbf == WBF_BarrierUnknown) + { + if (compiler->codeGen->genUseOptimizedWriteBarriers(wbf)) + { + // TODO-CQ: remove this pessimization, it was added to avoid diffs. + wbf = WBF_BarrierChecked; + } + else + { + assert(store->Addr()->TypeIs(TYP_BYREF)); + wbf = ((store->gtFlags & GTF_IND_TGT_HEAP) != 0) ? WBF_BarrierUnchecked : WBF_BarrierChecked; + } + } + + return wbf; +} - switch (tgt->gtOper) +//------------------------------------------------------------------------ +// gcWriteBarrierFormFromTargetAddress: Get the write barrier form from address. +// +// Arguments: +// tgtAddr - The target address of the store +// +// Return Value: +// The write barrier form to use for "STOREIND(tgtAddr, ...)". +// +GCInfo::WriteBarrierForm GCInfo::gcWriteBarrierFormFromTargetAddress(GenTree* tgtAddr) +{ + // If we store through an int to a GC_REF field, we'll assume that needs to use a checked barriers. + if (tgtAddr->TypeGet() == TYP_I_IMPL) { + // TODO-CQ: return "WBF_BarrierUnknown" here and let the caller check "GTF_IND_TGT_HEAP". + // This will result in using an unchecked barrier for "STOREIND(static-field-addr, ...)". + return GCInfo::WBF_BarrierChecked; + } - case GT_STOREIND: - case GT_IND: /* Could be the managed heap */ - if (tgt->TypeGet() == TYP_BYREF) + // Otherwise... + assert(tgtAddr->TypeGet() == TYP_BYREF); + bool simplifiedExpr = true; + while (simplifiedExpr) + { + simplifiedExpr = false; + + tgtAddr = tgtAddr->gtSkipReloadOrCopy(); + + // For additions, one of the operands is a byref or a ref (and the other is not). Follow this down to its + // source. + while (tgtAddr->OperIs(GT_ADD, GT_LEA)) + { + if (tgtAddr->OperGet() == GT_ADD) { - // Byref values cannot be in managed heap. - // This case occurs for Span. - return WBF_NoBarrier; + GenTree* addOp1 = tgtAddr->AsOp()->gtGetOp1(); + GenTree* addOp2 = tgtAddr->AsOp()->gtGetOp2(); + var_types addOp1Type = addOp1->TypeGet(); + var_types addOp2Type = addOp2->TypeGet(); + if (addOp1Type == TYP_BYREF || addOp1Type == TYP_REF) + { + assert(addOp2Type != TYP_BYREF && addOp2Type != TYP_REF); + tgtAddr = addOp1; + simplifiedExpr = true; + } + else if (addOp2Type == TYP_BYREF || addOp2Type == TYP_REF) + { + tgtAddr = addOp2; + simplifiedExpr = true; + } + else + { + // We might have a native int. For example: + // const int 0 + // + byref + // lclVar int V06 loc5 // this is a local declared "valuetype VType*" + return GCInfo::WBF_BarrierUnknown; + } } - if (tgt->gtFlags & GTF_IND_TGT_NOT_HEAP) + else { - // This indirection is not from to the heap. - // This case occurs for stack-allocated objects. - return WBF_NoBarrier; + // Must be an LEA (i.e., an AddrMode) + assert(tgtAddr->OperGet() == GT_LEA); + tgtAddr = tgtAddr->AsAddrMode()->Base(); + if (tgtAddr->TypeGet() == TYP_BYREF || tgtAddr->TypeGet() == TYP_REF) + { + simplifiedExpr = true; + } + else + { + // We might have a native int. + return GCInfo::WBF_BarrierUnknown; + } } - return gcWriteBarrierFormFromTargetAddress(tgt->AsOp()->gtOp1); - - case GT_LEA: - return gcWriteBarrierFormFromTargetAddress(tgt->AsAddrMode()->Base()); + } + } - case GT_CLS_VAR: - return WBF_BarrierUnchecked; + if (tgtAddr->IsLocalAddrExpr() != nullptr) + { + // No need for a GC barrier when writing to a local variable. + return GCInfo::WBF_NoBarrier; + } - case GT_LCL_VAR: /* Definitely not in the managed heap */ - case GT_LCL_FLD: - case GT_STORE_LCL_VAR: - case GT_STORE_LCL_FLD: - return WBF_NoBarrier; + if (tgtAddr->OperGet() == GT_LCL_VAR) + { + unsigned lclNum = tgtAddr->AsLclVar()->GetLclNum(); - default: - break; - } + LclVarDsc* varDsc = compiler->lvaGetDesc(lclNum); - assert(!"Missing case in gcIsWriteBarrierCandidate"); + // Instead of marking LclVar with 'lvStackByref', + // Consider decomposing the Value Number given to this LclVar to see if it was + // created using a GT_ADDR(GT_LCLVAR) or a GT_ADD( GT_ADDR(GT_LCLVAR), Constant) - return WBF_NoBarrier; -} + // We may have an internal compiler temp created in fgMorphCopyBlock() that we know + // points at one of our stack local variables, it will have lvStackByref set to true. + // + if (varDsc->lvStackByref) + { + assert(varDsc->TypeGet() == TYP_BYREF); + return GCInfo::WBF_NoBarrier; + } + } -bool GCInfo::gcIsWriteBarrierStoreIndNode(GenTree* op) -{ - assert(op->OperIs(GT_STOREIND)); + if (tgtAddr->TypeGet() == TYP_REF) + { + return GCInfo::WBF_BarrierUnchecked; + } - return gcIsWriteBarrierCandidate(op, op->AsOp()->gtOp2) != WBF_NoBarrier; + // Otherwise, we have no information. + return GCInfo::WBF_BarrierUnknown; } -/*****************************************************************************/ /***************************************************************************** * * Initialize the non-register pointer variable tracking logic. @@ -635,108 +719,6 @@ void GCInfo::gcRegPtrSetInit() #endif // JIT32_GCENCODER -GCInfo::WriteBarrierForm GCInfo::gcWriteBarrierFormFromTargetAddress(GenTree* tgtAddr) -{ - // If we store through an int to a GC_REF field, we'll assume that needs to use a checked barriers. - if (tgtAddr->TypeGet() == TYP_I_IMPL) - { - return GCInfo::WBF_BarrierChecked; // Why isn't this GCInfo::WBF_BarrierUnknown? - } - - // Otherwise... - assert(tgtAddr->TypeGet() == TYP_BYREF); - bool simplifiedExpr = true; - while (simplifiedExpr) - { - simplifiedExpr = false; - - tgtAddr = tgtAddr->gtSkipReloadOrCopy(); - - while (tgtAddr->OperGet() == GT_ADDR && tgtAddr->AsOp()->gtOp1->OperGet() == GT_IND) - { - tgtAddr = tgtAddr->AsOp()->gtOp1->AsOp()->gtOp1; - simplifiedExpr = true; - assert(tgtAddr->TypeGet() == TYP_BYREF); - } - // For additions, one of the operands is a byref or a ref (and the other is not). Follow this down to its - // source. - while (tgtAddr->OperIs(GT_ADD, GT_LEA)) - { - if (tgtAddr->OperGet() == GT_ADD) - { - GenTree* addOp1 = tgtAddr->AsOp()->gtGetOp1(); - GenTree* addOp2 = tgtAddr->AsOp()->gtGetOp2(); - var_types addOp1Type = addOp1->TypeGet(); - var_types addOp2Type = addOp2->TypeGet(); - if (addOp1Type == TYP_BYREF || addOp1Type == TYP_REF) - { - assert(addOp2Type != TYP_BYREF && addOp2Type != TYP_REF); - tgtAddr = addOp1; - simplifiedExpr = true; - } - else if (addOp2Type == TYP_BYREF || addOp2Type == TYP_REF) - { - tgtAddr = addOp2; - simplifiedExpr = true; - } - else - { - // We might have a native int. For example: - // const int 0 - // + byref - // lclVar int V06 loc5 // this is a local declared "valuetype VType*" - return GCInfo::WBF_BarrierUnknown; - } - } - else - { - // Must be an LEA (i.e., an AddrMode) - assert(tgtAddr->OperGet() == GT_LEA); - tgtAddr = tgtAddr->AsAddrMode()->Base(); - if (tgtAddr->TypeGet() == TYP_BYREF || tgtAddr->TypeGet() == TYP_REF) - { - simplifiedExpr = true; - } - else - { - // We might have a native int. - return GCInfo::WBF_BarrierUnknown; - } - } - } - } - if (tgtAddr->IsLocalAddrExpr() != nullptr) - { - // No need for a GC barrier when writing to a local variable. - return GCInfo::WBF_NoBarrier; - } - if (tgtAddr->OperGet() == GT_LCL_VAR) - { - unsigned lclNum = tgtAddr->AsLclVar()->GetLclNum(); - - LclVarDsc* varDsc = compiler->lvaGetDesc(lclNum); - - // Instead of marking LclVar with 'lvStackByref', - // Consider decomposing the Value Number given to this LclVar to see if it was - // created using a GT_ADDR(GT_LCLVAR) or a GT_ADD( GT_ADDR(GT_LCLVAR), Constant) - - // We may have an internal compiler temp created in fgMorphCopyBlock() that we know - // points at one of our stack local variables, it will have lvStackByref set to true. - // - if (varDsc->lvStackByref) - { - assert(varDsc->TypeGet() == TYP_BYREF); - return GCInfo::WBF_NoBarrier; - } - } - if (tgtAddr->TypeGet() == TYP_REF) - { - return GCInfo::WBF_BarrierUnchecked; - } - // Otherwise, we have no information. - return GCInfo::WBF_BarrierUnknown; -} - //------------------------------------------------------------------------ // gcUpdateForRegVarMove: Update the masks when a variable is moved // diff --git a/src/coreclr/jit/gentree.cpp b/src/coreclr/jit/gentree.cpp index da02f077c1775..e420c710fcdfe 100644 --- a/src/coreclr/jit/gentree.cpp +++ b/src/coreclr/jit/gentree.cpp @@ -10156,15 +10156,15 @@ void Compiler::gtDispNode(GenTree* tree, IndentStack* indentStack, _In_ _In_opt_ // We prefer printing V or U if ((tree->gtFlags & (GTF_IND_VOLATILE | GTF_IND_UNALIGNED)) == 0) { - if (tree->gtFlags & GTF_IND_TGTANYWHERE) + if (tree->gtFlags & GTF_IND_TGT_NOT_HEAP) { - printf("*"); + printf("s"); --msgLength; break; } - if (tree->gtFlags & GTF_IND_TGT_NOT_HEAP) + if (tree->gtFlags & GTF_IND_TGT_HEAP) { - printf("s"); + printf("h"); --msgLength; break; } @@ -15408,7 +15408,7 @@ GenTree* Compiler::gtNewRefCOMfield(GenTree* objPtr, else { result = gtNewOperNode(GT_IND, lclTyp, result); - result->gtFlags |= (GTF_EXCEPT | GTF_GLOB_REF | GTF_IND_TGTANYWHERE); + result->gtFlags |= (GTF_EXCEPT | GTF_GLOB_REF); result = gtNewAssignNode(result, assg); } } diff --git a/src/coreclr/jit/gentree.h b/src/coreclr/jit/gentree.h index 368f8766442d6..0fa58b87233e4 100644 --- a/src/coreclr/jit/gentree.h +++ b/src/coreclr/jit/gentree.h @@ -489,6 +489,7 @@ enum GenTreeFlags : unsigned int GTF_FLD_VOLATILE = 0x40000000, // GT_FIELD/GT_CLS_VAR -- same as GTF_IND_VOLATILE GTF_FLD_INITCLASS = 0x20000000, // GT_FIELD/GT_CLS_VAR -- field access requires preceding class/static init helper + GTF_FLD_TGT_HEAP = 0x10000000, // GT_FIELD/GT_CLS_VAR -- same as GTF_IND_TGT_HEAP GTF_INX_RNGCHK = 0x80000000, // GT_INDEX/GT_INDEX_ADDR -- the array reference should be range-checked. GTF_INX_STRING_LAYOUT = 0x40000000, // GT_INDEX -- this uses the special string array layout @@ -498,7 +499,7 @@ enum GenTreeFlags : unsigned int GTF_IND_VOLATILE = 0x40000000, // GT_IND -- the load or store must use volatile sematics (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_TGTANYWHERE = 0x10000000, // GT_IND -- the target could be anywhere + 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. @@ -513,11 +514,12 @@ enum GenTreeFlags : unsigned int 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_TGTANYWHERE | GTF_IND_NONFAULTING | GTF_IND_TLS_REF | - GTF_IND_UNALIGNED | GTF_IND_INVARIANT | GTF_IND_NONNULL | GTF_IND_TGT_NOT_HEAP, + GTF_IND_FLAGS = GTF_IND_VOLATILE | GTF_IND_NONFAULTING | GTF_IND_TLS_REF | GTF_IND_UNALIGNED | GTF_IND_INVARIANT | + GTF_IND_NONNULL | GTF_IND_TGT_NOT_HEAP | GTF_IND_TGT_HEAP, GTF_CLS_VAR_VOLATILE = 0x40000000, // GT_FIELD/GT_CLS_VAR -- same as GTF_IND_VOLATILE GTF_CLS_VAR_INITCLASS = 0x20000000, // GT_FIELD/GT_CLS_VAR -- same as GTF_FLD_INITCLASS + GTF_CLS_VAR_TGT_HEAP = 0x10000000, // GT_FIELD/GT_CLS_VAR -- same as GTF_IND_TGT_HEAP GTF_CLS_VAR_ASG_LHS = 0x04000000, // GT_CLS_VAR -- this GT_CLS_VAR node is (the effective val) of the LHS // of an assignment; don't evaluate it independently. diff --git a/src/coreclr/jit/importer.cpp b/src/coreclr/jit/importer.cpp index 1be934ae2ac64..d5b9fcb728694 100644 --- a/src/coreclr/jit/importer.cpp +++ b/src/coreclr/jit/importer.cpp @@ -1357,10 +1357,7 @@ GenTree* Compiler::impAssignStructPtr(GenTree* destAddr, } else // we don't have a GT_ADDR of a GT_LCL_VAR { - // !!! The destination could be on stack. !!! - // This flag will let us choose the correct write barrier. - asgType = returnType; - destFlags = GTF_IND_TGTANYWHERE; + asgType = returnType; } } } @@ -1387,13 +1384,6 @@ GenTree* Compiler::impAssignStructPtr(GenTree* destAddr, // Case of inline method returning a struct in one or more registers. // We won't need a return buffer asgType = src->gtType; - - if ((destAddr->gtOper != GT_ADDR) || (destAddr->AsOp()->gtOp1->gtOper != GT_LCL_VAR)) - { - // !!! The destination could be on stack. !!! - // This flag will let us choose the correct write barrier. - destFlags = GTF_IND_TGTANYWHERE; - } } } else if (src->OperIsBlk()) @@ -7547,7 +7537,7 @@ GenTree* Compiler::impTransformThis(GenTree* thisPtr, obj = gtNewOperNode(GT_IND, JITtype2varType(constraintTyp), obj); // ldind could point anywhere, example a boxed class static int - obj->gtFlags |= (GTF_EXCEPT | GTF_GLOB_REF | GTF_IND_TGTANYWHERE); + obj->gtFlags |= (GTF_EXCEPT | GTF_GLOB_REF); return obj; } @@ -7573,9 +7563,6 @@ GenTree* Compiler::impTransformThis(GenTree* thisPtr, if (obj->OperIsBlk()) { obj->ChangeOperUnchecked(GT_IND); - - // Obj could point anywhere, example a boxed class static int - obj->gtFlags |= GTF_IND_TGTANYWHERE; obj->AsOp()->gtOp2 = nullptr; // must be zero for tree walkers } @@ -14424,9 +14411,6 @@ void Compiler::impImportBlockCode(BasicBlock* block) op1 = gtNewOperNode(GT_IND, lclTyp, op1); - // stind could point anywhere, example a boxed class static int - op1->gtFlags |= GTF_IND_TGTANYWHERE; - if (prefixFlags & PREFIX_VOLATILE) { assert(op1->OperGet() == GT_IND); @@ -14501,7 +14485,7 @@ void Compiler::impImportBlockCode(BasicBlock* block) op1 = gtNewOperNode(GT_IND, lclTyp, op1); // ldind could point anywhere, example a boxed class static int - op1->gtFlags |= (GTF_EXCEPT | GTF_GLOB_REF | GTF_IND_TGTANYWHERE); + op1->gtFlags |= (GTF_EXCEPT | GTF_GLOB_REF); if (prefixFlags & PREFIX_VOLATILE) { @@ -15261,13 +15245,6 @@ void Compiler::impImportBlockCode(BasicBlock* block) op1->gtFlags |= GTF_EXCEPT; } - // If the object is a BYREF then our target is a value class and - // it could point anywhere, example a boxed class static int - if (obj->gtType == TYP_BYREF) - { - op1->gtFlags |= GTF_IND_TGTANYWHERE; - } - DWORD typeFlags = info.compCompHnd->getClassAttribs(resolvedToken.hClass); if (StructHasOverlappingFields(typeFlags)) { @@ -15573,13 +15550,6 @@ void Compiler::impImportBlockCode(BasicBlock* block) op1->gtFlags |= GTF_EXCEPT; } - // If object is a BYREF then our target is a value class and - // it could point anywhere, example a boxed class static int - if (obj->gtType == TYP_BYREF) - { - op1->gtFlags |= GTF_IND_TGTANYWHERE; - } - if (compIsForInlining() && impInlineIsGuaranteedThisDerefBeforeAnySideEffects(op2, nullptr, obj, impInlineInfo->inlArgInfo)) @@ -15629,18 +15599,28 @@ void Compiler::impImportBlockCode(BasicBlock* block) if (!deferStructAssign) { + assert(op1->OperIs(GT_FIELD, GT_IND)); + if (prefixFlags & PREFIX_VOLATILE) { - assert((op1->OperGet() == GT_FIELD) || (op1->OperGet() == GT_IND)); op1->gtFlags |= GTF_ORDER_SIDEEFF; // Prevent this from being reordered op1->gtFlags |= GTF_IND_VOLATILE; } if ((prefixFlags & PREFIX_UNALIGNED) && !varTypeIsByte(lclTyp)) { - assert((op1->OperGet() == GT_FIELD) || (op1->OperGet() == GT_IND)); op1->gtFlags |= GTF_IND_UNALIGNED; } + // Currently, *all* TYP_REF statics are stored inside an "object[]" array that itself + // resides on the managed heap, and so we can use an unchecked write barrier for this + // store. Likewise if we're storing to a field of an on-heap object. + if ((lclTyp == TYP_REF) && + (((fieldInfo.fieldFlags & CORINFO_FLG_FIELD_STATIC) != 0) || obj->TypeIs(TYP_REF))) + { + static_assert_no_msg(GTF_FLD_TGT_HEAP == GTF_IND_TGT_HEAP); + op1->gtFlags |= GTF_FLD_TGT_HEAP; + } + /* V4.0 allows assignment of i4 constant values to i8 type vars when IL verifier is bypassed (full trust apps). The reason this works is that JIT stores an i4 constant in Gentree union during importation and reads from the union as if it were a long during code generation. Though this @@ -16866,7 +16846,7 @@ void Compiler::impImportBlockCode(BasicBlock* block) op1 = gtNewOperNode(GT_IND, JITtype2varType(jitTyp), op1); // Could point anywhere, example a boxed class static int - op1->gtFlags |= GTF_IND_TGTANYWHERE | GTF_GLOB_REF; + op1->gtFlags |= GTF_GLOB_REF; assertImp(varTypeIsArithmetic(op1->gtType)); } else diff --git a/src/coreclr/jit/jitgcinfo.h b/src/coreclr/jit/jitgcinfo.h index 400437ec2d1c9..b73e8fbc68773 100644 --- a/src/coreclr/jit/jitgcinfo.h +++ b/src/coreclr/jit/jitgcinfo.h @@ -323,13 +323,14 @@ class GCInfo // might accidentally be violated in the future.) }; - WriteBarrierForm gcIsWriteBarrierCandidate(GenTree* tgt, GenTree* assignVal); - bool gcIsWriteBarrierStoreIndNode(GenTree* op); - - // Returns a WriteBarrierForm decision based on the form of "tgtAddr", which is assumed to be the - // argument of a GT_IND LHS. + WriteBarrierForm gcIsWriteBarrierCandidate(GenTreeStoreInd* store); WriteBarrierForm gcWriteBarrierFormFromTargetAddress(GenTree* tgtAddr); + bool gcIsWriteBarrierStoreIndNode(GenTreeStoreInd* store) + { + return gcIsWriteBarrierCandidate(store) != WBF_NoBarrier; + } + //------------------------------------------------------------------------- // // These record the info about the procedure in the info-block diff --git a/src/coreclr/jit/lower.cpp b/src/coreclr/jit/lower.cpp index 16d6111ad812d..01296b929fd27 100644 --- a/src/coreclr/jit/lower.cpp +++ b/src/coreclr/jit/lower.cpp @@ -7317,11 +7317,6 @@ bool Lowering::TryTransformStoreObjAsStoreInd(GenTreeBlk* blkNode) blkNode->ChangeOper(GT_STOREIND); blkNode->ChangeType(regType); - if ((blkNode->gtFlags & GTF_IND_TGT_NOT_HEAP) == 0) - { - blkNode->gtFlags |= GTF_IND_TGTANYWHERE; - } - if (varTypeIsStruct(src)) { src->ChangeType(regType); diff --git a/src/coreclr/jit/lsraarm.cpp b/src/coreclr/jit/lsraarm.cpp index 9050f1f5ac7d2..090780f49f9c1 100644 --- a/src/coreclr/jit/lsraarm.cpp +++ b/src/coreclr/jit/lsraarm.cpp @@ -675,7 +675,7 @@ int LinearScan::BuildNode(GenTree* tree) assert(dstCount == 0); GenTree* src = tree->gtGetOp2(); - if (compiler->codeGen->gcInfo.gcIsWriteBarrierStoreIndNode(tree)) + if (compiler->codeGen->gcInfo.gcIsWriteBarrierStoreIndNode(tree->AsStoreInd())) { srcCount = BuildGCWriteBarrier(tree); break; diff --git a/src/coreclr/jit/lsraarm64.cpp b/src/coreclr/jit/lsraarm64.cpp index 3a3d0ee23241f..9ab93ab7b6eb1 100644 --- a/src/coreclr/jit/lsraarm64.cpp +++ b/src/coreclr/jit/lsraarm64.cpp @@ -735,7 +735,7 @@ int LinearScan::BuildNode(GenTree* tree) { assert(dstCount == 0); - if (compiler->codeGen->gcInfo.gcIsWriteBarrierStoreIndNode(tree)) + if (compiler->codeGen->gcInfo.gcIsWriteBarrierStoreIndNode(tree->AsStoreInd())) { srcCount = BuildGCWriteBarrier(tree); break; diff --git a/src/coreclr/jit/lsrabuild.cpp b/src/coreclr/jit/lsrabuild.cpp index 65d06d30ffd33..19195e136ed63 100644 --- a/src/coreclr/jit/lsrabuild.cpp +++ b/src/coreclr/jit/lsrabuild.cpp @@ -771,9 +771,7 @@ regMaskTP LinearScan::getKillSetForStoreInd(GenTreeStoreInd* tree) regMaskTP killMask = RBM_NONE; - GenTree* data = tree->Data(); - - GCInfo::WriteBarrierForm writeBarrierForm = compiler->codeGen->gcInfo.gcIsWriteBarrierCandidate(tree, data); + GCInfo::WriteBarrierForm writeBarrierForm = compiler->codeGen->gcInfo.gcIsWriteBarrierCandidate(tree); if (writeBarrierForm != GCInfo::WBF_NoBarrier) { if (compiler->codeGen->genUseOptimizedWriteBarriers(writeBarrierForm)) @@ -787,9 +785,8 @@ regMaskTP LinearScan::getKillSetForStoreInd(GenTreeStoreInd* tree) else { // Figure out which helper we're going to use, and then get the kill set for that helper. - CorInfoHelpFunc helper = - compiler->codeGen->genWriteBarrierHelperForWriteBarrierForm(tree, writeBarrierForm); - killMask = compiler->compHelperCallKillSet(helper); + CorInfoHelpFunc helper = compiler->codeGen->genWriteBarrierHelperForWriteBarrierForm(writeBarrierForm); + killMask = compiler->compHelperCallKillSet(helper); } } return killMask; @@ -3979,7 +3976,7 @@ int LinearScan::BuildGCWriteBarrier(GenTree* tree) #if defined(TARGET_X86) && NOGC_WRITE_BARRIERS - bool useOptimizedWriteBarrierHelper = compiler->codeGen->genUseOptimizedWriteBarriers(tree, src); + bool useOptimizedWriteBarrierHelper = compiler->codeGen->genUseOptimizedWriteBarriers(tree->AsStoreInd()); if (useOptimizedWriteBarrierHelper) { // Special write barrier: diff --git a/src/coreclr/jit/lsraloongarch64.cpp b/src/coreclr/jit/lsraloongarch64.cpp index 59fe99ddb678f..577b6dc03fc68 100644 --- a/src/coreclr/jit/lsraloongarch64.cpp +++ b/src/coreclr/jit/lsraloongarch64.cpp @@ -597,7 +597,7 @@ int LinearScan::BuildNode(GenTree* tree) { assert(dstCount == 0); - if (compiler->codeGen->gcInfo.gcIsWriteBarrierStoreIndNode(tree)) + if (compiler->codeGen->gcInfo.gcIsWriteBarrierStoreIndNode(tree->AsStoreInd())) { srcCount = BuildGCWriteBarrier(tree); break; diff --git a/src/coreclr/jit/lsraxarch.cpp b/src/coreclr/jit/lsraxarch.cpp index 09a05a6bc34a9..08729f53e67fa 100644 --- a/src/coreclr/jit/lsraxarch.cpp +++ b/src/coreclr/jit/lsraxarch.cpp @@ -599,7 +599,7 @@ int LinearScan::BuildNode(GenTree* tree) break; case GT_STOREIND: - if (compiler->codeGen->gcInfo.gcIsWriteBarrierStoreIndNode(tree)) + if (compiler->codeGen->gcInfo.gcIsWriteBarrierStoreIndNode(tree->AsStoreInd())) { srcCount = BuildGCWriteBarrier(tree); break; diff --git a/src/coreclr/jit/morph.cpp b/src/coreclr/jit/morph.cpp index b3d6cf6b57439..facba7b6a4e76 100644 --- a/src/coreclr/jit/morph.cpp +++ b/src/coreclr/jit/morph.cpp @@ -5174,7 +5174,6 @@ GenTree* Compiler::fgMorphStackArgForVarArgs(unsigned lclNum, var_types varType, { tree = gtNewOperNode(GT_IND, varType, ptrArg); } - tree->gtFlags |= GTF_IND_TGTANYWHERE; if (varDsc->IsAddressExposed()) { @@ -5818,10 +5817,11 @@ GenTree* Compiler::fgMorphField(GenTree* tree, MorphAddrContext* mac) } else { - // Only volatile or classinit could be set, and they map over - noway_assert((tree->gtFlags & ~(GTF_FLD_VOLATILE | GTF_FLD_INITCLASS | GTF_COMMON_MASK)) == 0); + assert((tree->gtFlags & ~(GTF_FLD_VOLATILE | GTF_FLD_INITCLASS | GTF_FLD_TGT_HEAP | GTF_COMMON_MASK)) == + 0); static_assert_no_msg(GTF_FLD_VOLATILE == GTF_CLS_VAR_VOLATILE); static_assert_no_msg(GTF_FLD_INITCLASS == GTF_CLS_VAR_INITCLASS); + static_assert_no_msg(GTF_FLD_TGT_HEAP == GTF_CLS_VAR_TGT_HEAP); tree->SetOper(GT_CLS_VAR); tree->AsClsVar()->gtClsVarHnd = symHnd; tree->AsClsVar()->gtFieldSeq = fieldSeq; @@ -9453,7 +9453,7 @@ GenTree* Compiler::fgMorphOneAsgBlockOp(GenTree* tree) if (!fgIsIndirOfAddrOfLocal(dest)) { - dest->gtFlags |= (GTF_GLOB_REF | GTF_IND_TGTANYWHERE); + dest->gtFlags |= GTF_GLOB_REF; tree->gtFlags |= GTF_GLOB_REF; } @@ -9496,11 +9496,8 @@ GenTree* Compiler::fgMorphOneAsgBlockOp(GenTree* tree) { if (!fgIsIndirOfAddrOfLocal(src)) { - // If we have no information about the src, we have to assume it could - // live anywhere (not just in the GC heap). - // Mark the GT_IND node so that we use the correct write barrier helper in case - // the field is a GC ref. - src->gtFlags |= (GTF_GLOB_REF | GTF_IND_TGTANYWHERE); + // TODO-Bug: the GLOB_REF also needs to be set in case "src" is address-exposed. + src->gtFlags |= GTF_GLOB_REF; } src->SetIndirExceptionFlags(this); @@ -17494,8 +17491,8 @@ GenTree* Compiler::fgMorphImplicitByRefArgs(GenTree* tree, bool isAddr) } // TODO-CQ: If the VM ever stops violating the ABI and passing heap references - // we could remove TGTANYWHERE - tree->gtFlags = ((tree->gtFlags & GTF_COMMON_MASK) | GTF_IND_TGTANYWHERE); + // we could apply TGT_NOT_HEAP here. + tree->gtFlags = (tree->gtFlags & GTF_COMMON_MASK); #ifdef DEBUG if (verbose) diff --git a/src/coreclr/jit/morphblock.cpp b/src/coreclr/jit/morphblock.cpp index 3ee695e8658da..4e1f13a263366 100644 --- a/src/coreclr/jit/morphblock.cpp +++ b/src/coreclr/jit/morphblock.cpp @@ -1269,10 +1269,6 @@ GenTree* MorphCopyBlockHelper::CopyFieldByField() // TODO-1stClassStructs: remove this and implement storing to a field in a struct in a reg. m_comp->lvaSetVarDoNotEnregister(m_dstLclNum DEBUGARG(DoNotEnregisterReason::LocalField)); } - - // !!! The destination could be on stack. !!! - // This flag will let us choose the correct write barrier. - dstFld->gtFlags |= GTF_IND_TGTANYWHERE; } } diff --git a/src/coreclr/jit/objectalloc.cpp b/src/coreclr/jit/objectalloc.cpp index c0300dcc9639c..1f251f48d38d1 100644 --- a/src/coreclr/jit/objectalloc.cpp +++ b/src/coreclr/jit/objectalloc.cpp @@ -716,10 +716,9 @@ bool ObjectAllocator::CanLclVarEscapeViaParentStack(ArrayStack* parent // Notes: // If newType is TYP_I_IMPL, the tree is definitely pointing to the stack (or is null); // if newType is TYP_BYREF, the tree may point to the stack. -// In addition to updating types this method may set GTF_IND_TGTANYWHERE -// or GTF_IND_TGT_NOT_HEAP on ancestor indirections to help codegen -// with write barrier selection. - +// In addition to updating types this method may set GTF_IND_TGT_NOT_HEAP on ancestor +// indirections to help codegen with write barrier selection. +// void ObjectAllocator::UpdateAncestorTypes(GenTree* tree, ArrayStack* parentStack, var_types newType) { assert(newType == TYP_BYREF || newType == TYP_I_IMPL); @@ -773,19 +772,17 @@ void ObjectAllocator::UpdateAncestorTypes(GenTree* tree, ArrayStack* p case GT_FIELD: case GT_IND: { - if (newType == TYP_BYREF) - { - // This ensures that a checked write barrier is used when writing - // to this field/indirection (it can be inside a stack-allocated object). - parent->gtFlags |= GTF_IND_TGTANYWHERE; - } - else + // The new target could be *not* on the heap. + parent->gtFlags &= ~GTF_IND_TGT_HEAP; + + if (newType != TYP_BYREF) { // This indicates that a write barrier is not needed when writing // to this field/indirection since the address is not pointing to the heap. // It's either null or points to inside a stack-allocated object. parent->gtFlags |= GTF_IND_TGT_NOT_HEAP; } + int grandParentIndex = parentIndex + 1; if (parentStack->Height() > grandParentIndex) diff --git a/src/coreclr/jit/rationalize.cpp b/src/coreclr/jit/rationalize.cpp index 6eac05e951e05..c431596bcd4f0 100644 --- a/src/coreclr/jit/rationalize.cpp +++ b/src/coreclr/jit/rationalize.cpp @@ -452,18 +452,17 @@ void Rationalizer::RewriteAssignment(LIR::Use& use) case GT_CLS_VAR: { - bool isVolatile = (location->gtFlags & GTF_CLS_VAR_VOLATILE) != 0; + GenTreeFlags indFlags = location->gtFlags & (GTF_CLS_VAR_VOLATILE | GTF_CLS_VAR_TGT_HEAP); + static_assert_no_msg(GTF_CLS_VAR_VOLATILE == GTF_IND_VOLATILE); + static_assert_no_msg(GTF_CLS_VAR_TGT_HEAP == GTF_IND_TGT_HEAP); - location->gtFlags &= ~GTF_CLS_VAR_VOLATILE; + location->gtFlags &= ~indFlags; location->SetOper(GT_CLS_VAR_ADDR); location->gtType = TYP_BYREF; assignment->SetOper(GT_STOREIND); assignment->AsStoreInd()->SetRMWStatusDefault(); - if (isVolatile) - { - assignment->gtFlags |= GTF_IND_VOLATILE; - } + assignment->gtFlags |= indFlags; // TODO: JIT dump } diff --git a/src/coreclr/jit/simdcodegenxarch.cpp b/src/coreclr/jit/simdcodegenxarch.cpp index 5d364504cc060..5e0942d171133 100644 --- a/src/coreclr/jit/simdcodegenxarch.cpp +++ b/src/coreclr/jit/simdcodegenxarch.cpp @@ -866,7 +866,7 @@ void CodeGen::genStoreIndTypeSIMD12(GenTree* treeNode) #ifdef DEBUG // Should not require a write barrier - GCInfo::WriteBarrierForm writeBarrierForm = gcInfo.gcIsWriteBarrierCandidate(treeNode, data); + GCInfo::WriteBarrierForm writeBarrierForm = gcInfo.gcIsWriteBarrierCandidate(treeNode->AsStoreInd()); assert(writeBarrierForm == GCInfo::WBF_NoBarrier); #endif