From 8aa6afc722d2520f9e5a74a69aa9791bbbc2ee85 Mon Sep 17 00:00:00 2001 From: Egor Bogatov Date: Sun, 4 Feb 2024 21:59:19 +0100 Subject: [PATCH] Don't use VN in late phases (#97943) --- src/coreclr/jit/gcinfo.cpp | 4 +-- src/coreclr/jit/gentree.h | 2 +- src/coreclr/jit/helperexpansion.cpp | 52 +++++------------------------ 3 files changed, 12 insertions(+), 46 deletions(-) diff --git a/src/coreclr/jit/gcinfo.cpp b/src/coreclr/jit/gcinfo.cpp index 2364ce374e494..138d4e4a5169d 100644 --- a/src/coreclr/jit/gcinfo.cpp +++ b/src/coreclr/jit/gcinfo.cpp @@ -240,8 +240,8 @@ GCInfo::WriteBarrierForm GCInfo::gcIsWriteBarrierCandidate(GenTreeStoreInd* stor } // Ignore any assignments of NULL. - GenTree* const data = store->Data()->gtSkipReloadOrCopy(); - if ((data->GetVN(VNK_Liberal) == ValueNumStore::VNForNull()) || data->IsIntegralConst(0)) + GenTree* const data = store->Data()->gtSkipReloadOrCopy()->gtEffectiveVal(); + if (data->IsIntegralConst(0)) { return WBF_NoBarrier; } diff --git a/src/coreclr/jit/gentree.h b/src/coreclr/jit/gentree.h index 6eb9a1858ae78..5a192e68a7e54 100644 --- a/src/coreclr/jit/gentree.h +++ b/src/coreclr/jit/gentree.h @@ -4101,7 +4101,7 @@ enum GenTreeCallFlags : unsigned int GTF_CALL_M_GUARDED_DEVIRT_CHAIN = 0x00080000, // this call is a candidate for chained guarded devirtualization GTF_CALL_M_ALLOC_SIDE_EFFECTS = 0x00100000, // this is a call to an allocator with side effects GTF_CALL_M_SUPPRESS_GC_TRANSITION = 0x00200000, // suppress the GC transition (i.e. during a pinvoke) but a separate GC safe point is required. - GTF_CALL_M_EXP_RUNTIME_LOOKUP = 0x00400000, // this call needs to be transformed into CFG for the dynamic dictionary expansion feature. + GTF_CALL_M_EXP_RUNTIME_LOOKUP = 0x00400000, // [DEBUG only] this call needs to be transformed into CFG for the dynamic dictionary expansion feature. GTF_CALL_M_EXPANDED_EARLY = 0x00800000, // the Virtual Call target address is expanded and placed in gtControlExpr in Morph rather than in Lower GTF_CALL_M_HAS_LATE_DEVIRT_INFO = 0x01000000, // this call has late devirtualzation info GTF_CALL_M_LDVIRTFTN_INTERFACE = 0x02000000, // ldvirtftn on an interface type diff --git a/src/coreclr/jit/helperexpansion.cpp b/src/coreclr/jit/helperexpansion.cpp index d9b2b2aa5c5a8..994e51397ca1b 100644 --- a/src/coreclr/jit/helperexpansion.cpp +++ b/src/coreclr/jit/helperexpansion.cpp @@ -7,21 +7,6 @@ #pragma hdrstop #endif -// Obtain constant pointer from a tree -static void* GetConstantPointer(Compiler* comp, GenTree* tree) -{ - void* cns = nullptr; - if (tree->gtEffectiveVal()->IsCnsIntOrI()) - { - cns = (void*)tree->gtEffectiveVal()->AsIntCon()->IconValue(); - } - else if (comp->vnStore->IsVNConstant(tree->gtVNPair.GetLiberal())) - { - cns = (void*)comp->vnStore->CoercedConstantValue(tree->gtVNPair.GetLiberal()); - } - return cns; -} - // Save expression to a local and append it as the last statement in exprBlock static GenTree* SpillExpression(Compiler* comp, GenTree* expr, BasicBlock* exprBlock, DebugInfo& debugInfo) { @@ -186,14 +171,16 @@ bool Compiler::fgExpandRuntimeLookupsForCall(BasicBlock** pBlock, Statement* stm // // type = call(genericCtx, signatureCns); // - void* signature = GetConstantPointer(this, call->gtArgs.GetArgByIndex(1)->GetNode()); - if (signature == nullptr) + const GenTree* signatureNode = call->gtArgs.GetArgByIndex(1)->GetNode(); + if (!signatureNode->IsCnsIntOrI()) { - // Technically, it is possible (e.g. it was CSE'd and then VN was erased), but for Debug mode we - // want to catch such cases as we really don't want to emit just a fallback call - it's too slow + // We expect the signature to be a constant node here (it's marked as DONT_CSE) + // It's still correct if JIT decides to violate this assumption, but we really + // don't want it to happen, hence, the assert. assert(!"can't restore signature argument value"); return false; } + void* signature = reinterpret_cast(signatureNode->AsIntCon()->IconValue()); JITDUMP("Expanding runtime lookup for [%06d] in " FMT_BB ":\n", dspTreeID(call), block->bbNum) DISPTREE(call) @@ -1635,12 +1622,9 @@ bool Compiler::fgVNBasedIntrinsicExpansionForCall_ReadUtf8(BasicBlock** pBlock, DebugInfo debugInfo = stmt->GetDebugInfo(); - // Split block right before the call tree (this is a standard pattern we use in helperexpansion.cpp) - BasicBlock* prevBb = block; - GenTree** callUse = nullptr; - Statement* newFirstStmt = nullptr; - block = fgSplitBlockBeforeTree(block, stmt, call, &newFirstStmt, &callUse); - assert(prevBb != nullptr && block != nullptr); + BasicBlock* prevBb; + const unsigned resultLclNum = SplitAtTreeAndReplaceItWithLocal(this, block, stmt, call, &prevBb, &block); + *pBlock = block; // If we suddenly need to use these arguments, we'll have to reload them from the call @@ -1648,27 +1632,9 @@ bool Compiler::fgVNBasedIntrinsicExpansionForCall_ReadUtf8(BasicBlock** pBlock, srcLen = nullptr; srcPtr = nullptr; - // Block ops inserted by the split need to be morphed here since we are after morph. - // We cannot morph stmt yet as we may modify it further below, and the morphing - // could invalidate callUse - while ((newFirstStmt != nullptr) && (newFirstStmt != stmt)) - { - fgMorphStmtBlockOps(block, newFirstStmt); - newFirstStmt = newFirstStmt->GetNextStmt(); - } - // We don't need this flag anymore. call->gtCallMoreFlags &= ~GTF_CALL_M_SPECIAL_INTRINSIC; - // Grab a temp to store the result. - // The result corresponds the number of bytes written to dstPtr (int32). - assert(call->TypeIs(TYP_INT)); - const unsigned resultLclNum = lvaGrabTemp(true DEBUGARG("local for result")); - lvaTable[resultLclNum].lvType = TYP_INT; - *callUse = gtNewLclvNode(resultLclNum, TYP_INT); - fgMorphStmtBlockOps(block, stmt); - gtUpdateStmtSideEffects(stmt); - // srcLenU8 is the length of the string literal in chars (UTF16) // but we're going to use the same value as the "bytesWritten" result in the fast path and in the length check. GenTree* srcLenU8Node = gtNewIconNode(srcLenU8);