JIT: unpin locals whose value is provably non-movable

src/coreclr/jit/compiler.cpp

@@ -4548,6 +4548,10 @@ void Compiler::compCompile(void** methodCodePtr, uint32_t* methodCodeSize, JitFl
     //
     DoPhase(this, PHASE_LOCAL_MORPH, &Compiler::fgLocalMorph);
 
+    // Unpin pinned locals whose value is provably non-movable.
+    //
+    DoPhase(this, PHASE_UNPIN_LOCALS, &Compiler::fgUnpinNonMovableLocals);
+
     // Optimize away conversions to/from masks in local variables.
     //
     DoPhase(this, PHASE_OPTIMIZE_MASK_CONVERSIONS, &Compiler::fgOptimizeMaskConversions);

src/coreclr/jit/compiler.h

@@ -7375,6 +7375,7 @@ class Compiler
 
     PhaseStatus fgLocalMorph();
     bool fgExposeUnpropagatedLocals(bool propagatedAny, class LocalEqualsLocalAddrAssertions* assertions);
+    PhaseStatus fgUnpinNonMovableLocals();
     void fgExposeLocalsInBitVec(BitVec_ValArg_T bitVec);
 
     PhaseStatus fgOptimizeMaskConversions();

src/coreclr/jit/compphases.h

@@ -50,6 +50,7 @@ CompPhaseNameMacro(PHASE_DFS_BLOCKS1,                "DFS blocks and remove dead
 CompPhaseNameMacro(PHASE_DFS_BLOCKS2,                "DFS blocks and remove dead code 2",false, -1, false)
 CompPhaseNameMacro(PHASE_DFS_BLOCKS3,                "DFS blocks and remove dead code 3",false, -1, false)
 CompPhaseNameMacro(PHASE_LOCAL_MORPH,                "Local morph",        false, -1, false)
+CompPhaseNameMacro(PHASE_UNPIN_LOCALS,               "Unpin non-movable locals",       false, -1, false)
 CompPhaseNameMacro(PHASE_OPTIMIZE_MASK_CONVERSIONS,  "Optimize mask conversions",      false, -1, false)
 CompPhaseNameMacro(PHASE_EARLY_LIVENESS,             "Early liveness",                 false, -1, false)
 CompPhaseNameMacro(PHASE_PHYSICAL_PROMOTION,         "Physical promotion",             false, -1, false)

src/coreclr/jit/lclmorph.cpp

@@ -2401,6 +2401,202 @@ PhaseStatus Compiler::fgLocalMorph()
     return madeChanges ? PhaseStatus::MODIFIED_EVERYTHING : PhaseStatus::MODIFIED_NOTHING;
 }
 
+//------------------------------------------------------------------------
+// fgUnpinNonMovableLocals: unpin pinned locals whose value is provably
+//   non-movable.
+//
+// Notes:
+//   For each STORE V = x, V is a "no-gc value" candidate if x is direct
+//   no-gc (constant, LCL_ADDR, frozen object handle, static handle) or
+//   if x is a LCL_VAR V' that is itself a no-gc value candidate. The
+//   per-local property is the AND over all stores, so the lattice only
+//   ever flips from true to false; iteration terminates in at most one
+//   round per chain depth. Walks the per-statement local thread that
+//   was established by LocalSequencer in local morph rather than the
+//   full IR.
+//
+//   Params, implicitly-referenced locals, and address-exposed locals
+//   have defs we cannot see (call-site initialization, runtime writes,
+//   or aliased writes via the local's address) and so are excluded.
+//
+PhaseStatus Compiler::fgUnpinNonMovableLocals()
+{
+    if (opts.OptimizationDisabled())
+    {
+        return PhaseStatus::MODIFIED_NOTHING;
+    }
+
+    // Single scan to detect pinned locals and initialize the lattice.
+    //
+    BitVecTraits traits(lvaCount, this);
+    BitVec       hasNoGcValue = BitVecOps::MakeEmpty(&traits);
+    bool         anyPinned    = false;
+    for (unsigned lclNum = 0; lclNum < lvaCount; lclNum++)
+    {
+        LclVarDsc* const varDsc = lvaGetDesc(lclNum);
+        anyPinned |= varDsc->lvPinned;
+        if (!varDsc->lvImplicitlyReferenced && !varDsc->lvIsParam && !varDsc->IsAddressExposed())
+        {
+            BitVecOps::AddElemD(&traits, hasNoGcValue, lclNum);
+        }
+    }
+
+    if (!anyPinned)
+    {
+        return PhaseStatus::MODIFIED_NOTHING;
+    }
+
+    unsigned iterations = 0;
+    bool     changed    = true;
+    while (changed)
+    {
+        changed = false;
+        iterations++;
+        for (BasicBlock* const block : Blocks())
+        {
+            for (Statement* const stmt : block->Statements())
+            {
+                for (GenTreeLclVarCommon* const lcl : stmt->LocalsTreeList())
+                {
+                    if (lcl->OperIs(GT_STORE_LCL_VAR))
+                    {
+                        unsigned const   dstLclNum = lcl->GetLclNum();
+                        LclVarDsc* const dstDsc    = lvaGetDesc(dstLclNum);
+                        GenTree* const   value     = lcl->Data();
+
+                        // A struct store to a promoted destination implicitly
+                        // defines each field. Propagate per-field from a
+                        // matching promoted source LCL_VAR; otherwise mark
+                        // each destination field as has-GC.
+                        //
+                        if (varTypeIsStruct(dstDsc->TypeGet()) && dstDsc->lvPromoted)
+                        {
+                            LclVarDsc* srcDsc      = nullptr;
+                            unsigned   srcLclN     = BAD_VAR_NUM;
+                            bool       srcEligible = false;
+                            if (value->OperIs(GT_LCL_VAR))
+                            {
+                                srcLclN     = value->AsLclVar()->GetLclNum();
+                                srcDsc      = lvaGetDesc(srcLclN);
+                                srcEligible = varTypeIsStruct(srcDsc->TypeGet()) && srcDsc->lvPromoted &&
+                                              (srcDsc->lvFieldCnt == dstDsc->lvFieldCnt);
+                            }
+
+                            for (unsigned i = 0; i < dstDsc->lvFieldCnt; i++)
+                            {
+                                unsigned const dstFieldLclNum = dstDsc->lvFieldLclStart + i;
+                                if (!BitVecOps::IsMember(&traits, hasNoGcValue, dstFieldLclNum))
+                                {
+                                    continue;
+                                }
+
+                                bool isNoGc = false;
+                                if (srcEligible)
+                                {
+                                    LclVarDsc* const dstFld         = lvaGetDesc(dstFieldLclNum);
+                                    unsigned const   srcFieldLclNum = srcDsc->lvFieldLclStart + i;
+                                    LclVarDsc* const srcFld         = lvaGetDesc(srcFieldLclNum);
+                                    if (dstFld->lvIsStructField && srcFld->lvIsStructField &&
+                                        (dstFld->lvParentLcl == dstLclNum) && (srcFld->lvParentLcl == srcLclN) &&
+                                        (dstFld->lvFldOffset == srcFld->lvFldOffset) &&
+                                        (dstFld->lvFldOrdinal == srcFld->lvFldOrdinal) &&
+                                        (dstFld->TypeGet() == srcFld->TypeGet()))
+                                    {
+                                        isNoGc = BitVecOps::IsMember(&traits, hasNoGcValue, srcFieldLclNum);
+                                    }
+                                }
+
+                                if (!isNoGc)
+                                {
+                                    BitVecOps::RemoveElemD(&traits, hasNoGcValue, dstFieldLclNum);
+                                    changed = true;
+                                }
+                            }
+
+                            // Mark the promoted parent as has-GC for tidiness;
+                            // it is never consulted as a pinned local.
+                            //
+                            if (BitVecOps::IsMember(&traits, hasNoGcValue, dstLclNum))
+                            {
+                                BitVecOps::RemoveElemD(&traits, hasNoGcValue, dstLclNum);
+                                changed = true;
+                            }
+
+                            continue;
+                        }
+
+                        if (!BitVecOps::IsMember(&traits, hasNoGcValue, dstLclNum))
+                        {
+                            continue;
+                        }
+
+                        bool isNoGc = value->IsNotGcDef();
+
+                        if (!isNoGc && value->OperIs(GT_LCL_VAR))
+                        {
+                            isNoGc = BitVecOps::IsMember(&traits, hasNoGcValue, value->AsLclVar()->GetLclNum());
+                        }
+
+                        if (!isNoGc)
+                        {
+                            BitVecOps::RemoveElemD(&traits, hasNoGcValue, dstLclNum);
+                            changed = true;
+                        }
+
+                        continue;
+                    }
+
+                    // Any other def we do not analyze (GT_STORE_LCL_FLD,
+                    // retbuf GT_LCL_ADDR, etc.): mark the destination as
+                    // has-GC, plus all fields if it is a promoted parent.
+                    //
+                    if ((lcl->gtFlags & GTF_VAR_DEF) != 0)
+                    {
+                        unsigned const   dstLclNum = lcl->GetLclNum();
+                        LclVarDsc* const dstDsc    = lvaGetDesc(dstLclNum);
+
+                        if (BitVecOps::IsMember(&traits, hasNoGcValue, dstLclNum))
+                        {
+                            BitVecOps::RemoveElemD(&traits, hasNoGcValue, dstLclNum);
+                            changed = true;
+                        }
+
+                        if (varTypeIsStruct(dstDsc->TypeGet()) && dstDsc->lvPromoted)
+                        {
+                            for (unsigned i = 0; i < dstDsc->lvFieldCnt; i++)
+                            {
+                                unsigned const dstFieldLclNum = dstDsc->lvFieldLclStart + i;
+                                if (BitVecOps::IsMember(&traits, hasNoGcValue, dstFieldLclNum))
+                                {
+                                    BitVecOps::RemoveElemD(&traits, hasNoGcValue, dstFieldLclNum);
+                                    changed = true;
+                                }
+                            }
+                        }
+                    }
+                }
+            }
+        }
+    }
+
+    unsigned unpinned = 0;
+    for (unsigned lclNum = 0; lclNum < lvaCount; lclNum++)
+    {
+        LclVarDsc* const varDsc = lvaGetDesc(lclNum);
+        if (varDsc->lvPinned && BitVecOps::IsMember(&traits, hasNoGcValue, lclNum))
+        {
+            varDsc->lvPinned = 0;
+            unpinned++;
+            JITDUMP("V%02u unpinned: all defs are no-gc\n", lclNum);
+        }
+    }
+
+    JITDUMP("fgUnpinNonMovableLocals: %u local%s unpinned after %u iteration%s\n", unpinned, unpinned == 1 ? "" : "s",
+            iterations, iterations == 1 ? "" : "s");
+
+    return PhaseStatus::MODIFIED_NOTHING;
+}
+
 //-----------------------------------------------------------------------------------
 // fgExposeUnpropagatedLocals:
 //   Expose the final set of locals that were computed to have their address

src/tests/JIT/opt/DSE/Regressions/PinnedStackAllocDoesNotEmitPinStore.cs

@@ -0,0 +1,77 @@
+// Licensed to the .NET Foundation under one or more agreements.
+// The .NET Foundation licenses this file to you under the MIT license.
+
+using System;
+using System.Runtime.CompilerServices;
+using Xunit;
+
+public unsafe class PinnedStackAllocDoesNotEmitPinStore
+{
+    [MethodImpl(MethodImplOptions.NoInlining)]
+    public static byte ReadFirst()
+    {
+        Span<byte> buf = stackalloc byte[16];
+        for (int i = 0; i < 16; i++)
+        {
+            buf[i] = (byte)i;
+        }
+        fixed (byte* p = buf)
+        {
+            return *p;
+        }
+    }
+
+    [MethodImpl(MethodImplOptions.NoInlining)]
+    public static int Sum()
+    {
+        Span<byte> buf = stackalloc byte[8];
+        for (int i = 0; i < 8; i++)
+        {
+            buf[i] = (byte)(i + 1);
+        }
+        int sum = 0;
+        fixed (byte* p = buf)
+        {
+            for (int i = 0; i < 8; i++)
+            {
+                sum += p[i];
+            }
+        }
+        return sum;
+    }
+
+    [MethodImpl(MethodImplOptions.NoInlining)]
+    public static int SumViaTwoFixed()
+    {
+        Span<byte> buf = stackalloc byte[8];
+        for (int i = 0; i < 8; i++)
+        {
+            buf[i] = (byte)(i + 1);
+        }
+        int sum;
+        fixed (byte* a = buf)
+        fixed (byte* b = buf)
+        {
+            sum = *a + *(b + 7);
+        }
+        return sum;
+    }
+
+    [Fact]
+    public static void ReadFirst_ReturnsZero()
+    {
+        Assert.Equal(0, ReadFirst());
+    }
+
+    [Fact]
+    public static void Sum_ReturnsExpected()
+    {
+        Assert.Equal(1 + 2 + 3 + 4 + 5 + 6 + 7 + 8, Sum());
+    }
+
+    [Fact]
+    public static void SumViaTwoFixed_ReturnsExpected()
+    {
+        Assert.Equal(1 + 8, SumViaTwoFixed());
+    }
+}

src/tests/JIT/opt/DSE/Regressions/PinnedStackAllocDoesNotEmitPinStore.csproj

@@ -0,0 +1,9 @@
+<Project Sdk="Microsoft.NET.Sdk">
+  <PropertyGroup>
+    <Optimize>True</Optimize>
+    <AllowUnsafeBlocks>true</AllowUnsafeBlocks>
+  </PropertyGroup>
+  <ItemGroup>
+    <Compile Include="PinnedStackAllocDoesNotEmitPinStore.cs" />
+  </ItemGroup>
+</Project>