Always expose parents of exposed promoted fields (#76181)

* Remove the special case

* Simplify code

* Add tests

diff --git a/src/coreclr/jit/assertionprop.cpp b/src/coreclr/jit/assertionprop.cpp
index 8ec78dd..5432b3c 100644 (file)
--- a/src/coreclr/jit/assertionprop.cpp
+++ b/src/coreclr/jit/assertionprop.cpp
@@ -1489,22 +1489,6 @@ AssertionIndex Compiler::optCreateAssertion(GenTree*         op1,
             goto DONE_ASSERTION; // Don't make an assertion
         }
 
-        // If the local is a promoted struct and has an exposed field then bail.
-        //
-        if (lclVar->lvPromoted)
-        {
-            for (unsigned childLclNum = lclVar->lvFieldLclStart;
-                 childLclNum < lclVar->lvFieldLclStart + lclVar->lvFieldCnt; ++childLclNum)
-            {
-                LclVarDsc* const childVar = lvaGetDesc(childLclNum);
-
-                if (childVar->IsAddressExposed())
-                {
-                    goto DONE_ASSERTION;
-                }
-            }
-        }
-
         if (helperCallArgs)
         {
             //
@@ -1724,22 +1708,6 @@ AssertionIndex Compiler::optCreateAssertion(GenTree*         op1,
                         goto DONE_ASSERTION; // Don't make an assertion
                     }
 
-                    // If the local is a promoted struct and has an exposed field then bail.
-                    //
-                    if (lclVar2->lvPromoted)
-                    {
-                        for (unsigned childLclNum = lclVar2->lvFieldLclStart;
-                             childLclNum < lclVar2->lvFieldLclStart + lclVar2->lvFieldCnt; ++childLclNum)
-                        {
-                            LclVarDsc* const childVar = lvaGetDesc(childLclNum);
-
-                            if (childVar->IsAddressExposed())
-                            {
-                                goto DONE_ASSERTION;
-                            }
-                        }
-                    }
-
                     assertion.op2.kind       = O2K_LCLVAR_COPY;
                     assertion.op2.vn         = optConservativeNormalVN(op2);
                     assertion.op2.lcl.lclNum = lclNum2;

diff --git a/src/coreclr/jit/lclmorph.cpp b/src/coreclr/jit/lclmorph.cpp
index b344f37..fa6b179 100644 (file)
--- a/src/coreclr/jit/lclmorph.cpp
+++ b/src/coreclr/jit/lclmorph.cpp
@@ -606,18 +606,8 @@ private:
     {
         assert(val.IsAddress());
 
-        LclVarDsc* varDsc = m_compiler->lvaGetDesc(val.LclNum());
-
-        // In general we don't know how an exposed struct field address will be used - it may be used to
-        // access only that specific field or it may be used to access other fields in the same struct
-        // by using pointer/ref arithmetic. It seems reasonable to make an exception for the "this" arg
-        // of calls - it would be highly unusual for a struct member method to attempt to access memory
-        // beyond "this" instance. And calling struct member methods is common enough that attempting to
-        // mark the entire struct as address exposed results in CQ regressions.
-        GenTreeCall* callTree  = user->IsCall() ? user->AsCall() : nullptr;
-        bool         isThisArg = (callTree != nullptr) && callTree->gtArgs.HasThisPointer() &&
-                         (val.Node() == callTree->gtArgs.GetThisArg()->GetNode());
-        bool exposeParentLcl = varDsc->lvIsStructField && !isThisArg;
+        unsigned   lclNum = val.LclNum();
+        LclVarDsc* varDsc = m_compiler->lvaGetDesc(lclNum);
 
         bool hasHiddenStructArg = false;
         if (m_compiler->opts.compJitOptimizeStructHiddenBuffer)
@@ -627,20 +617,20 @@ private:
             // b) Do not later turn into indirections.
             //
             bool isSuitableLocal =
-                varTypeIsStruct(varDsc) && varDsc->lvIsTemp && !m_compiler->lvaIsImplicitByRefLocal(val.LclNum());
+                varTypeIsStruct(varDsc) && varDsc->lvIsTemp && !m_compiler->lvaIsImplicitByRefLocal(lclNum);
 #ifdef TARGET_X86
-            if (m_compiler->lvaIsArgAccessedViaVarArgsCookie(val.LclNum()))
+            if (m_compiler->lvaIsArgAccessedViaVarArgsCookie(lclNum))
             {
                 isSuitableLocal = false;
             }
 #endif // TARGET_X86
 
+            GenTreeCall* callTree = user->IsCall() ? user->AsCall() : nullptr;
+
             if (isSuitableLocal && (callTree != nullptr) && callTree->gtArgs.HasRetBuffer() &&
                 (val.Node() == callTree->gtArgs.GetRetBufferArg()->GetNode()))
             {
-                assert(!exposeParentLcl);
-
-                m_compiler->lvaSetHiddenBufferStructArg(val.LclNum());
+                m_compiler->lvaSetHiddenBufferStructArg(lclNum);
                 hasHiddenStructArg = true;
                 callTree->gtCallMoreFlags |= GTF_CALL_M_RETBUFFARG_LCLOPT;
             }
@@ -649,7 +639,7 @@ private:
         if (!hasHiddenStructArg)
         {
             m_compiler->lvaSetVarAddrExposed(
-                exposeParentLcl ? varDsc->lvParentLcl : val.LclNum() DEBUGARG(AddressExposedReason::ESCAPE_ADDRESS));
+                varDsc->lvIsStructField ? varDsc->lvParentLcl : lclNum DEBUGARG(AddressExposedReason::ESCAPE_ADDRESS));
         }
 #ifdef TARGET_64BIT
         // If the address of a variable is passed in a call and the allocation size of the variable

diff --git a/src/tests/JIT/Regression/JitBlue/Runtime_76096/Runtime_76096.cs b/src/tests/JIT/Regression/JitBlue/Runtime_76096/Runtime_76096.cs
new file mode 100644 (file)
index 0000000..605cbb5
--- /dev/null
+++ b/src/tests/JIT/Regression/JitBlue/Runtime_76096/Runtime_76096.cs
@@ -0,0 +1,63 @@
+// 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.Diagnostics.CodeAnalysis;
+
+unsafe class Runtime_76096
+{
+    public static int Main(string[] args)
+    {
+        int result = 100;
+        if (ProblemWithMemoryNumbering())
+        {
+            result++;
+        }
+        if (ProblemWithRefArithmetic())
+        {
+            result++;
+        }
+
+        return result;
+    }
+
+    [MethodImpl(MethodImplOptions.NoInlining)]
+    private static bool ProblemWithMemoryNumbering(byte zero = 0)
+    {
+        TwoFieldStruct a = new TwoFieldStruct { FieldOne = { Value = 1 }, FieldTwo = 2 };
+
+        ref int fieldOneRef = ref a.FieldOne.Addr();
+        var fieldOne = fieldOneRef;
+
+        Unsafe.InitBlock(&a, zero, (uint)sizeof(TwoFieldStruct));
+
+        return fieldOne + fieldOneRef != 1;
+    }
+
+    [MethodImpl(MethodImplOptions.NoInlining)]
+    private static bool ProblemWithRefArithmetic()
+    {
+        TwoFieldStruct a = new TwoFieldStruct { FieldOne = { Value = 1 }, FieldTwo = 2 };
+
+        ref int fieldOneRef = ref a.FieldOne.Addr();
+        ref int fieldTwoRef = ref Unsafe.Add(ref fieldOneRef, 1);
+        a.FieldTwo = 0;
+
+        return fieldTwoRef != 0;
+    }
+}
+
+struct OneFieldStruct
+{
+    public int Value;
+
+    [UnscopedRef]
+    [MethodImpl(MethodImplOptions.NoInlining)]
+    public ref int Addr() => ref Value;
+}
+
+struct TwoFieldStruct
+{
+    public OneFieldStruct FieldOne;
+    public int FieldTwo;
+}

diff --git a/src/tests/JIT/Regression/JitBlue/Runtime_76096/Runtime_76096.csproj b/src/tests/JIT/Regression/JitBlue/Runtime_76096/Runtime_76096.csproj
new file mode 100644 (file)
index 0000000..0f9eec3
--- /dev/null
+++ b/src/tests/JIT/Regression/JitBlue/Runtime_76096/Runtime_76096.csproj
@@ -0,0 +1,10 @@
+<Project Sdk="Microsoft.NET.Sdk">
+  <PropertyGroup>
+    <OutputType>Exe</OutputType>
+    <Optimize>True</Optimize>
+    <AllowUnsafeBlocks>True</AllowUnsafeBlocks>
+  </PropertyGroup>
+  <ItemGroup>
+    <Compile Include="$(MSBuildProjectName).cs" />
+  </ItemGroup>
+</Project>
\ No newline at end of file