Skip to content

Commit d6c28d4

Browse files
authored
JIT: Handle remainder accesses more precisely in physical promotion liveness (#92651)
The liveness pass in physical promotion will currently handle any struct LCL_FLD access of a physically promoted struct as accessing the remainder. However, if the LCL_FLD only touches promoted fields then the remainder is not actually used. There was a TODO around this which this PR fixes as I stumbled upon a case this would improve.
1 parent 3741887 commit d6c28d4

File tree

4 files changed

+108
-14
lines changed

4 files changed

+108
-14
lines changed

src/coreclr/jit/jitstd/vector.h

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -145,6 +145,7 @@ class vector
145145

146146
// cctors
147147
vector(const vector& vec);
148+
vector(vector&& vec);
148149

149150
template <typename Alt, typename AltAllocator>
150151
explicit vector(const vector<Alt, AltAllocator>& vec);
@@ -195,6 +196,8 @@ class vector
195196
template <typename Alt, typename AltAllocator>
196197
vector<T, Allocator>& operator=(const vector<Alt, AltAllocator>& vec);
197198

199+
vector& operator=(vector&& vec);
200+
198201
reference operator[](size_type n);
199202
const_reference operator[](size_type n) const;
200203

@@ -328,6 +331,18 @@ vector<T, Allocator>::vector(const vector<T, Allocator>& vec)
328331
}
329332
}
330333

334+
template <typename T, typename Allocator>
335+
vector<T, Allocator>::vector(vector<T, Allocator>&& vec)
336+
: m_allocator(vec.m_allocator)
337+
, m_pArray(vec.m_pArray)
338+
, m_nSize(vec.m_nSize)
339+
, m_nCapacity(vec.m_nCapacity)
340+
{
341+
vec.m_pArray = nullptr;
342+
vec.m_nSize = 0;
343+
vec.m_nCapacity = 0;
344+
}
345+
331346
template <typename T, typename Allocator>
332347
vector<T, Allocator>::~vector()
333348
{
@@ -578,6 +593,20 @@ vector<T, Allocator>& vector<T, Allocator>::operator=(const vector<T, Allocator>
578593
return *this;
579594
}
580595

596+
template <typename T, typename Allocator>
597+
vector<T, Allocator>& vector<T, Allocator>::operator=(vector<T, Allocator>&& vec)
598+
{
599+
m_allocator = vec.m_allocator;
600+
m_pArray = vec.m_pArray;
601+
m_nSize = vec.m_nSize;
602+
m_nCapacity = vec.m_nCapacity;
603+
604+
vec.m_pArray = nullptr;
605+
vec.m_nSize = 0;
606+
vec.m_nCapacity = 0;
607+
608+
return *this;
609+
}
581610

582611
template <typename T, typename Allocator>
583612
typename vector<T, Allocator>::reference vector<T, Allocator>::operator[](size_type n)

src/coreclr/jit/promotion.cpp

Lines changed: 70 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1229,19 +1229,18 @@ class LocalsUseVisitor : public GenTreeVisitor<LocalsUseVisitor>
12291229
}
12301230
#endif
12311231

1232-
StructSegments unpromotedParts =
1233-
m_prom->SignificantSegments(m_compiler->lvaGetDesc(agg->LclNum)->GetLayout());
1232+
agg->Unpromoted = m_prom->SignificantSegments(m_compiler->lvaGetDesc(agg->LclNum)->GetLayout());
12341233
for (Replacement& rep : reps)
12351234
{
1236-
unpromotedParts.Subtract(StructSegments::Segment(rep.Offset, rep.Offset + genTypeSize(rep.AccessType)));
1235+
agg->Unpromoted.Subtract(StructSegments::Segment(rep.Offset, rep.Offset + genTypeSize(rep.AccessType)));
12371236
}
12381237

12391238
JITDUMP(" Unpromoted remainder: ");
1240-
DBEXEC(m_compiler->verbose, unpromotedParts.Dump());
1239+
DBEXEC(m_compiler->verbose, agg->Unpromoted.Dump());
12411240
JITDUMP("\n\n");
12421241

12431242
StructSegments::Segment unpromotedSegment;
1244-
if (unpromotedParts.CoveringSegment(&unpromotedSegment))
1243+
if (agg->Unpromoted.CoveringSegment(&unpromotedSegment))
12451244
{
12461245
agg->UnpromotedMin = unpromotedSegment.Start;
12471246
agg->UnpromotedMax = unpromotedSegment.End;
@@ -1495,6 +1494,31 @@ bool StructSegments::Segment::IntersectsOrAdjacent(const Segment& other) const
14951494
return true;
14961495
}
14971496

1497+
//------------------------------------------------------------------------
1498+
// Intersects:
1499+
// Check if this segment intersects another segment.
1500+
//
1501+
// Parameters:
1502+
// other - The other segment.
1503+
//
1504+
// Returns:
1505+
// True if so.
1506+
//
1507+
bool StructSegments::Segment::Intersects(const Segment& other) const
1508+
{
1509+
if (End <= other.Start)
1510+
{
1511+
return false;
1512+
}
1513+
1514+
if (other.End <= Start)
1515+
{
1516+
return false;
1517+
}
1518+
1519+
return true;
1520+
}
1521+
14981522
//------------------------------------------------------------------------
14991523
// Contains:
15001524
// Check if this segment contains another segment.
@@ -1586,7 +1610,7 @@ void StructSegments::Subtract(const Segment& segment)
15861610
return;
15871611
}
15881612

1589-
assert(m_segments[index].IntersectsOrAdjacent(segment));
1613+
assert(m_segments[index].Intersects(segment));
15901614

15911615
if (m_segments[index].Contains(segment))
15921616
{
@@ -1679,6 +1703,46 @@ bool StructSegments::CoveringSegment(Segment* result)
16791703
return true;
16801704
}
16811705

1706+
//------------------------------------------------------------------------
1707+
// Intersects:
1708+
// Check if a segment intersects with any segment in this segment tree.
1709+
//
1710+
// Parameters:
1711+
// segment - The segment.
1712+
//
1713+
// Returns:
1714+
// True if the input segment intersects with any segment in the tree;
1715+
// otherwise false.
1716+
//
1717+
bool StructSegments::Intersects(const Segment& segment)
1718+
{
1719+
size_t index = Promotion::BinarySearch<Segment, &Segment::End>(m_segments, segment.Start);
1720+
if ((ssize_t)index < 0)
1721+
{
1722+
index = ~index;
1723+
}
1724+
else
1725+
{
1726+
// Start == segment[index].End, which makes it non-interesting.
1727+
index++;
1728+
}
1729+
1730+
if (index >= m_segments.size())
1731+
{
1732+
return false;
1733+
}
1734+
1735+
// Here we know Start < segment[index].End. Do they not intersect at all?
1736+
if (m_segments[index].Start >= segment.End)
1737+
{
1738+
// Does not intersect any segment.
1739+
return false;
1740+
}
1741+
1742+
assert(m_segments[index].Intersects(segment));
1743+
return true;
1744+
}
1745+
16821746
#ifdef DEBUG
16831747
//------------------------------------------------------------------------
16841748
// Dump:

src/coreclr/jit/promotion.h

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -60,6 +60,7 @@ class StructSegments
6060
}
6161

6262
bool IntersectsOrAdjacent(const Segment& other) const;
63+
bool Intersects(const Segment& other) const;
6364
bool Contains(const Segment& other) const;
6465
void Merge(const Segment& other);
6566
};
@@ -68,14 +69,15 @@ class StructSegments
6869
jitstd::vector<Segment> m_segments;
6970

7071
public:
71-
StructSegments(CompAllocator allocator) : m_segments(allocator)
72+
explicit StructSegments(CompAllocator allocator) : m_segments(allocator)
7273
{
7374
}
7475

7576
void Add(const Segment& segment);
7677
void Subtract(const Segment& segment);
7778
bool IsEmpty();
7879
bool CoveringSegment(Segment* result);
80+
bool Intersects(const Segment& segment);
7981

8082
#ifdef DEBUG
8183
void Dump();
@@ -87,12 +89,14 @@ struct AggregateInfo
8789
{
8890
jitstd::vector<Replacement> Replacements;
8991
unsigned LclNum;
92+
// Unpromoted parts of the struct local.
93+
StructSegments Unpromoted;
9094
// Min offset in the struct local of the unpromoted part.
9195
unsigned UnpromotedMin = 0;
9296
// Max offset in the struct local of the unpromoted part.
9397
unsigned UnpromotedMax = 0;
9498

95-
AggregateInfo(CompAllocator alloc, unsigned lclNum) : Replacements(alloc), LclNum(lclNum)
99+
AggregateInfo(CompAllocator alloc, unsigned lclNum) : Replacements(alloc), LclNum(lclNum), Unpromoted(alloc)
96100
{
97101
}
98102

src/coreclr/jit/promotionliveness.cpp

Lines changed: 3 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -242,9 +242,8 @@ void PromotionLiveness::MarkUseDef(GenTreeLclVarCommon* lcl, BitVec& useSet, Bit
242242
}
243243

244244
bool isFullDefOfRemainder = isDef && (agg->UnpromotedMin >= offs) && (agg->UnpromotedMax <= (offs + size));
245-
// TODO-CQ: We could also try to figure out if a use actually touches the remainder, e.g. in some cases
246-
// a struct use may consist only of promoted fields and does not actually use the remainder.
247-
MarkIndex(baseIndex, isUse, isFullDefOfRemainder, useSet, defSet);
245+
bool isUseOfRemainder = isUse && agg->Unpromoted.Intersects(StructSegments::Segment(offs, offs + size));
246+
MarkIndex(baseIndex, isUseOfRemainder, isFullDefOfRemainder, useSet, defSet);
248247
}
249248
}
250249
else
@@ -609,11 +608,9 @@ void PromotionLiveness::FillInLiveness(BitVec& life, BitVec volatileVars, GenTre
609608
}
610609
else
611610
{
612-
// TODO-CQ: We could also try to figure out if a use actually touches the remainder, e.g. in some cases
613-
// a struct use may consist only of promoted fields and does not actually use the remainder.
614611
BitVecOps::AddElemD(&aggTraits, aggDeaths, 0);
615612

616-
if (isUse)
613+
if (isUse && agg->Unpromoted.Intersects(StructSegments::Segment(offs, offs + size)))
617614
{
618615
BitVecOps::AddElemD(m_bvTraits, life, baseIndex);
619616
}

0 commit comments

Comments
 (0)