diff --git a/src/coreclr/jit/layout.cpp b/src/coreclr/jit/layout.cpp index 113414ddfd7f7..918fd4ab6521d 100644 --- a/src/coreclr/jit/layout.cpp +++ b/src/coreclr/jit/layout.cpp @@ -421,6 +421,7 @@ void ClassLayout::InitializeGCPtrs(Compiler* compiler) // // Return value: // true if at least one GC ByRef, false otherwise. +// bool ClassLayout::HasGCByRef() const { unsigned slots = GetSlotCount(); @@ -435,6 +436,39 @@ bool ClassLayout::HasGCByRef() const return false; } +//------------------------------------------------------------------------ +// IntersectsGCPtr: check if the specified interval intersects with a GC +// pointer. +// +// Parameters: +// offset - The start offset of the interval +// size - The size of the interval +// +// Return value: +// True if it does. +// +bool ClassLayout::IntersectsGCPtr(unsigned offset, unsigned size) const +{ + if (!HasGCPtr()) + { + return false; + } + + unsigned startSlot = offset / TARGET_POINTER_SIZE; + unsigned endSlot = (offset + size - 1) / TARGET_POINTER_SIZE; + assert((startSlot < GetSlotCount()) && (endSlot < GetSlotCount())); + + for (unsigned i = startSlot; i <= endSlot; i++) + { + if (IsGCPtr(i)) + { + return true; + } + } + + return false; +} + //------------------------------------------------------------------------ // AreCompatible: check if 2 layouts are the same for copying. // diff --git a/src/coreclr/jit/layout.h b/src/coreclr/jit/layout.h index 0e9d6ed65d03d..59ecaa9405485 100644 --- a/src/coreclr/jit/layout.h +++ b/src/coreclr/jit/layout.h @@ -216,6 +216,8 @@ class ClassLayout } } + bool IntersectsGCPtr(unsigned offset, unsigned size) const; + static bool AreCompatible(const ClassLayout* layout1, const ClassLayout* layout2); private: diff --git a/src/coreclr/jit/promotion.cpp b/src/coreclr/jit/promotion.cpp index e2c4e797a3c9d..5982ed7928335 100644 --- a/src/coreclr/jit/promotion.cpp +++ b/src/coreclr/jit/promotion.cpp @@ -621,6 +621,38 @@ class LocalUses bool EvaluateReplacement( Compiler* comp, unsigned lclNum, const Access& access, unsigned inducedCount, weight_t inducedCountWtd) { + // Verify that this replacement has proper GC ness compared to the + // layout. While reinterpreting GC fields to integers can be considered + // UB, there are scenarios where it can happen safely: + // + // * The user code could have guarded the access with a dynamic check + // that it doesn't contain a GC pointer, so that the access is actually + // in dead code. This happens e.g. in span functions in SPC. + // + // * For byrefs, reinterpreting as an integer could be ok in a + // restricted scope due to pinning. + // + // In theory we could allow these promotions in the restricted scope, + // but currently physical promotion works on a function-wide basis. + + LclVarDsc* lcl = comp->lvaGetDesc(lclNum); + ClassLayout* layout = lcl->GetLayout(); + if (layout->IntersectsGCPtr(access.Offset, genTypeSize(access.AccessType))) + { + if (((access.Offset % TARGET_POINTER_SIZE) != 0) || + (layout->GetGCPtrType(access.Offset / TARGET_POINTER_SIZE) != access.AccessType)) + { + return false; + } + } + else + { + if (varTypeIsGC(access.AccessType)) + { + return false; + } + } + unsigned countOverlappedCallArg = 0; unsigned countOverlappedStoredFromCall = 0; @@ -678,9 +710,8 @@ class LocalUses // Now look at the overlapping struct uses that promotion will make more expensive. - unsigned countReadBacks = 0; - weight_t countReadBacksWtd = 0; - LclVarDsc* lcl = comp->lvaGetDesc(lclNum); + unsigned countReadBacks = 0; + weight_t countReadBacksWtd = 0; // For parameters or OSR locals we always need one read back. if (lcl->lvIsParam || lcl->lvIsOSRLocal) {