From ee2d4b442a790ac165da5bb1e43a7d1ebfc7ccc9 Mon Sep 17 00:00:00 2001 From: Ben Adams Date: Mon, 14 Sep 2026 13:37:47 +0100 Subject: [PATCH 1/2] JIT: construct promoted arguments directly Passing a promoted struct by value can first synchronize a narrow field to storage and then immediately load a wider overlapping chunk for the outgoing copy. Construct eligible outgoing arguments from replacement registers and the untouched remainder, preserving by-value isolation. Reuse existing remainder bounds and last-use analysis. Limit this to whole-local, non-GC arguments to standalone direct calls with one argument, a primitive-copyable remainder, and no potential EH successors or tail calls. Validation against this change's base on updated main a652cdfed564: - Windows x64 Checked and Release and Linux x64 Release JIT builds. - 18 existing physical-promotion regression runs pass with normal, forced promotion and JitStress=2 settings. - Focused optimized, promotion-stress, JitStress=2 and warmed-up tiering/PGO executions pass on base and candidate; Tier1 is captured. - Windows codegen checks fail on base and pass on candidate. Linux behavior passes, with codegen checks where applicable. - Eleven SuperPMI collections: 983,234 comparable contexts; 238,457,491 -> 238,457,491 bytes (0 saved). 0 shrink; 0 grow by 0 bytes total. Zero compilation failures; 190 missing-recording contexts excluded. The 190 recording gaps affect both sides. - Release PIN JIT instructions, benchmarks: -0.03660%. - Release PIN JIT instructions, HashSet PGO: -0.02404%. The corpus has no assembly differences; the focused FullOpts and Tier1 examples demonstrate the generated-code improvements. Code size, static PerfScore and JIT instruction counts do not establish application execution speed. No ARM64 or x86 execution is claimed. Related to #133833. --- src/coreclr/jit/promotion.cpp | 72 +++++++++- src/coreclr/jit/promotion.h | 1 + src/tests/JIT/Directed/Directed_do.csproj | 1 + .../CopyPromotedArguments.cs | 124 ++++++++++++++++++ .../CopyPromotedArguments.csproj | 14 ++ 5 files changed, 205 insertions(+), 7 deletions(-) create mode 100644 src/tests/JIT/Directed/physicalpromotion/CopyPromotedArguments.cs create mode 100644 src/tests/JIT/Directed/physicalpromotion/CopyPromotedArguments.csproj diff --git a/src/coreclr/jit/promotion.cpp b/src/coreclr/jit/promotion.cpp index 2ecad18d74760f..5620daf3816ab4 100644 --- a/src/coreclr/jit/promotion.cpp +++ b/src/coreclr/jit/promotion.cpp @@ -297,7 +297,11 @@ void AggregateInfoMap::Add(AggregateInfo* agg) // AggregateInfo* AggregateInfoMap::Lookup(unsigned lclNum) { - assert(lclNum < m_numLocals); + // Temporaries introduced while replacing uses were not promotion candidates. + if (lclNum >= m_numLocals) + { + return nullptr; + } unsigned index = m_lclNumToAggregateIndex[lclNum]; if (index == UINT_MAX) @@ -2141,9 +2145,10 @@ void ReplaceVisitor::InsertPreStatementWriteBacks() continue; } - if (m_replacer->CanReplaceCallArgWithFieldListOfReplacements(call, &arg, node->AsLclVarCommon())) + if (m_replacer->CanReplaceCallArgWithFieldListOfReplacements(call, &arg, node->AsLclVarCommon()) || + m_replacer->CanCopyCallArgFromReplacements(call, &arg, node->AsLclVarCommon())) { - // Register arg that can be decomposed into FIELD_LIST. + // The argument can be sourced directly from replacements. continue; } @@ -2399,8 +2404,8 @@ GenTreeFieldList* ReplaceVisitor::CreateFieldListForStructLocal(GenTreeLclVarCom //------------------------------------------------------------------------ // ReplaceCallArgWithFieldList: -// Handle a call that may pass a struct local with replacements as the -// retbuf. +// Source a struct argument from its replacements, either as a FIELD_LIST +// or by materializing the outgoing copy before global morph. // // Parameters: // call - The call @@ -2408,8 +2413,7 @@ GenTreeFieldList* ReplaceVisitor::CreateFieldListForStructLocal(GenTreeLclVarCom // argNode - The argument node // // Returns: -// True if the call argument was replaced with a FIELD_LIST; false if the -// argument could not be represented as a FIELD_LIST. +// True if the argument was replaced; false if write-backs are required. // bool ReplaceVisitor::ReplaceCallArgWithFieldList(GenTreeCall* call, GenTree** use, GenTreeLclVarCommon* argNode) { @@ -2422,6 +2426,23 @@ bool ReplaceVisitor::ReplaceCallArgWithFieldList(GenTreeCall* call, GenTree** us if (!CanReplaceCallArgWithFieldListOfReplacements(call, callArg, argNode)) { + if (CanCopyCallArgFromReplacements(call, callArg, argNode)) + { + // Materialize the outgoing copy while the replacement values are + // available. Global morph can pass this temporary at its last use + // instead of making another copy from synchronized source storage. + unsigned temp = m_compiler->lvaGrabTemp(true DEBUGARG("Decomposed struct argument")); + m_compiler->lvaSetStruct(temp, argNode->GetLayout(m_compiler), false); + GenTree* copy = m_compiler->gtNewStoreLclVarNode(temp, argNode); + HandleStructStore(©, nullptr); + GenTree* value = m_compiler->gtNewLclvNode(temp, TYP_STRUCT); + value->gtFlags |= GTF_VAR_DEATH; + Statement* copyStmt = m_compiler->fgNewStmtFromTree(copy); + m_compiler->fgInsertStmtBefore(m_currentBlock, m_currentStmt, copyStmt); + *use = value; + m_madeChanges = true; + return true; + } return false; } @@ -2436,6 +2457,43 @@ bool ReplaceVisitor::ReplaceCallArgWithFieldList(GenTreeCall* call, GenTree** us return true; } +//------------------------------------------------------------------------ +// CanCopyCallArgFromReplacements: +// Check whether to materialize an outgoing by-value argument before morph. +// Limit this to a standalone direct call with one argument, a dirty non-GC +// whole-local source, and a remainder needing at most one primitive copy. +// A dying source can already be passed without copying. +// +bool ReplaceVisitor::CanCopyCallArgFromReplacements(GenTreeCall* call, CallArg* callArg, GenTreeLclVarCommon* lcl) +{ + if (!lcl->OperIs(GT_LCL_VAR) || !callArg->AbiInfo.IsPassedByReference() || (call->gtArgs.CountArgs() != 1) || + (call->gtCallType != CT_USER_FUNC) || (call != m_currentStmt->GetRootNode()) || (callArg->GetNode() != lcl) || + call->IsTailCall() || lcl->GetLayout(m_compiler)->HasGCPtr() || + m_currentBlock->HasPotentialEHSuccs(m_compiler) || IsPromotedStructLocalDying(lcl)) + { + return false; + } + + AggregateInfo* agg = m_aggregates.Lookup(lcl->GetLclNum()); + // Whole-local arguments can reuse the remainder computed during promotion. + unsigned remainderSize = agg->UnpromotedMax - agg->UnpromotedMin; + bool canCopyRemainder = (remainderSize == 0) || (isPow2(remainderSize) && (remainderSize <= TARGET_POINTER_SIZE)); +#ifdef FEATURE_SIMD + canCopyRemainder |= (remainderSize == 16) && (m_compiler->getPreferredVectorByteLength() >= 16); +#endif + if (canCopyRemainder) + { + for (const Replacement& rep : agg->Replacements) + { + if (rep.NeedsWriteBack) + { + return true; + } + } + } + return false; +} + //------------------------------------------------------------------------ // CanReplaceCallArgWithFieldListOfReplacements: // Returns true if a struct arg is replaceable by a FIELD_LIST containing diff --git a/src/coreclr/jit/promotion.h b/src/coreclr/jit/promotion.h index db24874ad69992..c620ade7c852a1 100644 --- a/src/coreclr/jit/promotion.h +++ b/src/coreclr/jit/promotion.h @@ -298,6 +298,7 @@ class ReplaceVisitor : public GenTreeVisitor bool ReplaceReturnedStructLocal(GenTreeOp* ret, GenTree** use, GenTreeLclVarCommon* value); bool ReplaceCallArgWithFieldList(GenTreeCall* call, GenTree** use, GenTreeLclVarCommon* callArg); bool CanReplaceCallArgWithFieldListOfReplacements(GenTreeCall* call, CallArg* callArg, GenTreeLclVarCommon* lcl); + bool CanCopyCallArgFromReplacements(GenTreeCall* call, CallArg* callArg, GenTreeLclVarCommon* lcl); GenTreeFieldList* CreateFieldListForStructLocal(GenTreeLclVarCommon* value); void ReadBackAfterCall(GenTreeCall* call, GenTree* user); bool IsPromotedStructLocalDying(GenTreeLclVarCommon* structLcl); diff --git a/src/tests/JIT/Directed/Directed_do.csproj b/src/tests/JIT/Directed/Directed_do.csproj index 9ef81e4ada860d..4e1b6673c9ec20 100644 --- a/src/tests/JIT/Directed/Directed_do.csproj +++ b/src/tests/JIT/Directed/Directed_do.csproj @@ -9,6 +9,7 @@ + diff --git a/src/tests/JIT/Directed/physicalpromotion/CopyPromotedArguments.cs b/src/tests/JIT/Directed/physicalpromotion/CopyPromotedArguments.cs new file mode 100644 index 00000000000000..da2012abd64507 --- /dev/null +++ b/src/tests/JIT/Directed/physicalpromotion/CopyPromotedArguments.cs @@ -0,0 +1,124 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using System.Runtime.CompilerServices; +using System.Runtime.InteropServices; +using Xunit; + +public class CopyPromotedArguments +{ + private static long s_consumed; + + [Fact] + public static void TestEntryPoint() + { + foreach (long value in new[] { 0L, 1L, -1L, long.MinValue, long.MaxValue }) + { + CopyArgumentIsolation(value); + CopyArgumentAcrossException(value); + Assert.Equal(value + 1 + (int)(value + 1), CleanSource(value)); + } + } + + [StructLayout(LayoutKind.Explicit, Size = 24)] + private struct S + { + [FieldOffset(0)] public long Wide; + [FieldOffset(0)] public int Narrow; + [FieldOffset(8)] public long Other; + [FieldOffset(16)] public long Last; + } + + [MethodImpl(MethodImplOptions.NoInlining)] + private static S Create(long value) => new S { Wide = value, Other = value + 1, Last = value + 2 }; + + [MethodImpl(MethodImplOptions.NoInlining)] + private static int Observe(int value) => value; + + [MethodImpl(MethodImplOptions.NoInlining)] + private static void Observe(long value) { } + + [MethodImpl(MethodImplOptions.NoInlining)] + private static void Consume(S value) + { + Assert.Equal(value.Other + 1, value.Last); + } + + [MethodImpl(MethodImplOptions.NoInlining)] + private static void CheckFields(S value, long wide, long other, long last) + { + Assert.Equal(wide, value.Wide); + Assert.Equal(other, value.Other); + Assert.Equal(last, value.Last); + } + + [MethodImpl(MethodImplOptions.NoInlining)] + private static void ConsumeAndModify(S value) + { + s_consumed = value.Wide; + value.Wide = ~value.Wide; + value.Other = -7; + CheckFields(value, ~s_consumed, -7, s_consumed + 1); + } + + [MethodImpl(MethodImplOptions.NoInlining)] + private static void CopyArgumentIsolation(long value) + { + S src = Create(value); + src.Wide++; + Observe(src.Wide); + Observe(src.Wide); + Observe(src.Wide); + ConsumeAndModify(src); + Assert.Equal(value + 1, s_consumed); + CheckFields(src, value + 1, value + 1, value + 2); + } + + [MethodImpl(MethodImplOptions.NoInlining)] + private static void ConsumeThenThrow(S value) + { + ConsumeAndModify(value); + throw new System.InvalidOperationException(); + } + + [MethodImpl(MethodImplOptions.NoInlining)] + private static void CopyArgumentAcrossException(long value) + { + S src = Create(value); + try + { + src.Wide++; + Observe(src.Wide); + Observe(src.Wide); + Observe(src.Wide); + ConsumeThenThrow(src); + } + catch (System.InvalidOperationException) + { + CheckFields(src, value + 1, value + 1, value + 2); + } + } + + [MethodImpl(MethodImplOptions.NoInlining)] + private static long CleanSource(long value) + { + // X64-WINDOWS: call {{.*}}CopyPromotedArguments:Create + // Build the outgoing argument from the replacement, without synchronizing + // source storage and immediately loading across that narrow store. + // X64-WINDOWS: mov [[VALUE:r[a-z0-9]+]], qword ptr [rsp+[[SOURCE:0x[0-9A-Fa-f]+]]] + // X64-WINDOWS-NOT: mov qword ptr [rsp+[[SOURCE]]], [[VALUE]] + // X64-WINDOWS: call {{.*}}CopyPromotedArguments:Consume + S src = Create(value); + src.Wide++; + Observe(src.Wide); + Observe(src.Wide); + Observe(src.Wide); + Consume(src); // The outgoing copy can take Wide directly from its replacement. + S dst = src; + Observe(dst.Narrow); + Observe(dst.Narrow); + Observe(dst.Narrow); + Consume(src); + return src.Wide + dst.Narrow; + } +} diff --git a/src/tests/JIT/Directed/physicalpromotion/CopyPromotedArguments.csproj b/src/tests/JIT/Directed/physicalpromotion/CopyPromotedArguments.csproj new file mode 100644 index 00000000000000..76c8fa127633a1 --- /dev/null +++ b/src/tests/JIT/Directed/physicalpromotion/CopyPromotedArguments.csproj @@ -0,0 +1,14 @@ + + + true + true + True + + + + true + + + + + From d0efa4a8614190626ce39f8b0a273817ca081c75 Mon Sep 17 00:00:00 2001 From: Ben Adams Date: Mon, 14 Sep 2026 15:30:58 +0100 Subject: [PATCH 2/2] JIT: guard promoted argument copies by copy-omission support Match CanCopyCallArgFromReplacements to the platform guard used by fgMarkImplicitByRefCopyOmissionCandidates, so the generated temporary can be passed without another outgoing copy. Ordinary SysV AMD64 arguments already fail IsPassedByReference, but Swift arguments can pass that check. Keep the physical-promotion test project references alphabetized. Validation: Windows x64 Checked and Linux x64 Release JIT builds; Windows focused tests in normal, forced-promotion, JitStress=2 and tiering/PGO modes; Windows assembly checks; Linux focused behavioral tests. All passed. --- src/coreclr/jit/promotion.cpp | 5 +++++ src/tests/JIT/Directed/Directed_do.csproj | 2 +- 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/src/coreclr/jit/promotion.cpp b/src/coreclr/jit/promotion.cpp index 5620daf3816ab4..2bc284ab55be12 100644 --- a/src/coreclr/jit/promotion.cpp +++ b/src/coreclr/jit/promotion.cpp @@ -2466,6 +2466,7 @@ bool ReplaceVisitor::ReplaceCallArgWithFieldList(GenTreeCall* call, GenTree** us // bool ReplaceVisitor::CanCopyCallArgFromReplacements(GenTreeCall* call, CallArg* callArg, GenTreeLclVarCommon* lcl) { +#if FEATURE_IMPLICIT_BYREFS && !defined(UNIX_AMD64_ABI) if (!lcl->OperIs(GT_LCL_VAR) || !callArg->AbiInfo.IsPassedByReference() || (call->gtArgs.CountArgs() != 1) || (call->gtCallType != CT_USER_FUNC) || (call != m_currentStmt->GetRootNode()) || (callArg->GetNode() != lcl) || call->IsTailCall() || lcl->GetLayout(m_compiler)->HasGCPtr() || @@ -2492,6 +2493,10 @@ bool ReplaceVisitor::CanCopyCallArgFromReplacements(GenTreeCall* call, CallArg* } } return false; +#else + // The temporary requires fgMarkImplicitByRefCopyOmissionCandidates to avoid another outgoing copy. + return false; +#endif } //------------------------------------------------------------------------ diff --git a/src/tests/JIT/Directed/Directed_do.csproj b/src/tests/JIT/Directed/Directed_do.csproj index 4e1b6673c9ec20..2842935114737e 100644 --- a/src/tests/JIT/Directed/Directed_do.csproj +++ b/src/tests/JIT/Directed/Directed_do.csproj @@ -9,8 +9,8 @@ - +