Fix for the fields.

dotnet · Aug 11, 2021 · 2af4408 · 2af4408
1 parent 5cb7245
commit 2af4408
Show file tree

Hide file tree

Showing 2 changed files with 27 additions and 17 deletions.
diff --git a/src/coreclr/jit/importer.cpp b/src/coreclr/jit/importer.cpp
@@ -17284,9 +17284,10 @@ bool Compiler::impReturnInstruction(int prefixFlags, OPCODE& opcode)
                     GenTree* addrChild = effectiveRetVal->gtGetOp1();
                     if (addrChild->OperGet() == GT_LCL_VAR)
                     {
-                        LclVarDsc* varDsc = lvaGetDesc(addrChild->AsLclVarCommon());
+                        GenTreeLclVar* lclVar = addrChild->AsLclVar();
+                        LclVarDsc*     varDsc = lvaGetDesc(lclVar);
 
-                        if (varTypeIsStruct(addrChild->TypeGet()) && !isOpaqueSIMDLclVar(varDsc))
+                        if (varTypeIsStruct(lclVar->TypeGet()) && !isOpaqueSIMDLclVar(varDsc))
                         {
                             CORINFO_CLASS_HANDLE referentClassHandle;
                             CorInfoType          referentType =
@@ -17300,15 +17301,13 @@ bool Compiler::impReturnInstruction(int prefixFlags, OPCODE& opcode)
                                 // to struct2. struct1 and struct2 are different so we are "reinterpreting" the struct.
                                 // This may happen in, for example, System.Runtime.CompilerServices.Unsafe.As<TFrom,
                                 // TTo>.
-                                // We need to mark the source struct variable as having overlapping fields because its
-                                // fields may be accessed using field handles of a different type, which may confuse
-                                // optimizations, in particular, value numbering.
+                                // We need to exclude the local from VN because VN optimizations don't expect such
+                                // transformations.
 
-                                JITDUMP("\nSetting lvOverlappingFields to true on V%02u because of struct "
-                                        "reinterpretation\n",
-                                        addrChild->AsLclVarCommon()->GetLclNum());
+                                JITDUMP("Setting DontVN() on [%06u] because of struct reinterpretation.\n",
+                                        dspTreeID(lclVar));
 
-                                varDsc->lvOverlappingFields = true;
+                                lclVar->SetDontVN();
                             }
                         }
                     }

diff --git a/src/coreclr/jit/lclmorph.cpp b/src/coreclr/jit/lclmorph.cpp
@@ -39,6 +39,10 @@ class LocalAddressVisitor final : public GenTreeVisitor<LocalAddressVisitor>
         unsigned      m_lclNum;
         unsigned      m_offset;
         bool          m_address;
+        bool          m_excludeFromVN; // This value should not participate in VN optimizations,
+                                       // for example, because it represents a reinterpreted struct.
+                                       // When we transform it we should set `DontVN()` for `LclVar`
+                                       // and `NotAField` for `LclFld`.
         INDEBUG(bool m_consumed;)
 
     public:
@@ -49,6 +53,7 @@ class LocalAddressVisitor final : public GenTreeVisitor<LocalAddressVisitor>
             , m_lclNum(BAD_VAR_NUM)
             , m_offset(0)
             , m_address(false)
+            , m_excludeFromVN(false)
 #ifdef DEBUG
             , m_consumed(false)
 #endif // DEBUG
@@ -112,6 +117,10 @@ class LocalAddressVisitor final : public GenTreeVisitor<LocalAddressVisitor>
             assert(!IsLocation() && !IsAddress());
 
             m_lclNum = lclVar->GetLclNum();
+            if (lclVar->DontVN())
+            {
+                m_excludeFromVN = true;
+            }
 
             assert(m_offset == 0);
             assert(m_fieldSeq == nullptr);
@@ -195,10 +204,11 @@ class LocalAddressVisitor final : public GenTreeVisitor<LocalAddressVisitor>
 
             if (val.IsLocation())
             {
-                m_address  = true;
-                m_lclNum   = val.m_lclNum;
-                m_offset   = val.m_offset;
-                m_fieldSeq = val.m_fieldSeq;
+                m_address       = true;
+                m_lclNum        = val.m_lclNum;
+                m_offset        = val.m_offset;
+                m_fieldSeq      = val.m_fieldSeq;
+                m_excludeFromVN = val.m_excludeFromVN;
             }
 
             INDEBUG(val.Consume();)
@@ -246,7 +256,7 @@ class LocalAddressVisitor final : public GenTreeVisitor<LocalAddressVisitor>
                 m_lclNum = val.m_lclNum;
                 m_offset = newOffset.Value();
 
-                if (field->gtFldMayOverlap)
+                if (field->gtFldMayOverlap || val.m_excludeFromVN)
                 {
                     m_fieldSeq = FieldSeqStore::NotAField();
                 }
@@ -287,9 +297,10 @@ class LocalAddressVisitor final : public GenTreeVisitor<LocalAddressVisitor>
 
             if (val.IsAddress())
             {
-                m_lclNum   = val.m_lclNum;
-                m_offset   = val.m_offset;
-                m_fieldSeq = val.m_fieldSeq;
+                m_lclNum        = val.m_lclNum;
+                m_offset        = val.m_offset;
+                m_fieldSeq      = val.m_fieldSeq;
+                m_excludeFromVN = val.m_excludeFromVN;
             }
 
             INDEBUG(val.Consume();)