Skip to content

Commit dbb330a

Browse files
Merge main
2 parents 0e41b35 + 8df86ac commit dbb330a

16 files changed

Lines changed: 830 additions & 74 deletions

File tree

src/coreclr/interpreter/compiler.cpp

Lines changed: 11 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -883,7 +883,12 @@ int32_t InterpCompiler::GetLiveStartOffset(int32_t var)
883883
else
884884
{
885885
assert(m_pVars[var].liveStart != NULL);
886-
return m_pVars[var].liveStart->nativeOffset;
886+
// The value of the var is not valid at the start of the instruction that sets it.
887+
// We don't set it as the start of the next instruction because a live range needs
888+
// to have start != end, which wouldn't be the case if the var is always dead. Ignoring
889+
// the live range for such a var could be problematic if this var is the return of a call,
890+
// with its address being passed directly to the a jit-ed method.
891+
return m_pVars[var].liveStart->nativeOffset + GetInsLength(m_pVars[var].liveStart) - 1;
887892
}
888893
}
889894

@@ -1399,16 +1404,16 @@ class InterpGcSlotAllocator
13991404
if (existingRange.OverlapsOrAdjacentTo(start, end))
14001405
{
14011406
ConservativeRange updatedRange = existingRange;
1402-
INTERP_DUMP("Merging with existing range [%u - %u]\n", existingRange.startOffset, existingRange.endOffset);
1407+
INTERP_DUMP("Merging with existing range [%04x - %04x]\n", existingRange.startOffset, existingRange.endOffset);
14031408
updatedRange.MergeWith(start, end);
14041409
while ((i + 1 < m_liveRanges.GetSize()) && m_liveRanges.Get(i + 1).OverlapsOrAdjacentTo(start, end))
14051410
{
14061411
ConservativeRange otherExistingRange = m_liveRanges.Get(i + 1);
1407-
INTERP_DUMP("Merging with existing range [%u - %u]\n", otherExistingRange.startOffset, otherExistingRange.endOffset);
1412+
INTERP_DUMP("Merging with existing range [%04x - %04x]\n", otherExistingRange.startOffset, otherExistingRange.endOffset);
14081413
updatedRange.MergeWith(otherExistingRange.startOffset, otherExistingRange.endOffset);
14091414
m_liveRanges.RemoveAt(i + 1);
14101415
}
1411-
INTERP_DUMP("Final merged range [%u - %u]\n", updatedRange.startOffset, updatedRange.endOffset);
1416+
INTERP_DUMP("Final merged range [%04x - %04x]\n", updatedRange.startOffset, updatedRange.endOffset);
14121417
m_liveRanges.Set(i, updatedRange);
14131418
return;
14141419
}
@@ -1497,7 +1502,7 @@ class InterpGcSlotAllocator
14971502
uint32_t startOffset = ConvertOffset(m_compiler->GetLiveStartOffset(varIndex)),
14981503
endOffset = ConvertOffset(m_compiler->GetLiveEndOffset(varIndex));
14991504
INTERP_DUMP(
1500-
"Slot %u (%s var #%d offset %u) live [IR_%04x - IR_%04x] [%u - %u]\n",
1505+
"Slot %u (%s var #%d offset %u) live [IR_%04x - IR_%04x] [%04x - %04x]\n",
15011506
slot, pVar->global ? "global" : "local",
15021507
varIndex, pVar->offset,
15031508
m_compiler->GetLiveStartOffset(varIndex), m_compiler->GetLiveEndOffset(varIndex),
@@ -1532,7 +1537,7 @@ class InterpGcSlotAllocator
15321537
ConservativeRange range = ranges->m_liveRanges.Get(iRange);
15331538

15341539
INTERP_DUMP(
1535-
"Conservative range for slot %u at %u [%u - %u]\n",
1540+
"Conservative range for slot %u at %u [%04x - %04x]\n",
15361541
*pSlot,
15371542
(unsigned)(iSlot * sizeof(void *)),
15381543
range.startOffset,

src/coreclr/jit/CMakeLists.txt

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -106,6 +106,7 @@ set( JIT_SOURCES
106106
asyncanalysis.cpp
107107
bitset.cpp
108108
block.cpp
109+
boundscheckcoalesce.cpp
109110
buildstring.cpp
110111
codegencommon.cpp
111112
codegenlinear.cpp
Lines changed: 273 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,273 @@
1+
// Licensed to the .NET Foundation under one or more agreements.
2+
// The .NET Foundation licenses this file to you under the MIT license.
3+
4+
//
5+
// Bounds Check Coalescing
6+
//
7+
// Within a single block, when multiple GT_BOUNDS_CHECK nodes share the same
8+
// length VN and use constant indices, only the bounds check with the largest
9+
// constant index is actually needed. This pass finds such groups and
10+
// strengthens the first bounds check in the group by replacing its constant
11+
// index with the maximum constant index in the group. Forward assertion prop
12+
// then drops the now-redundant later bounds checks.
13+
//
14+
// Example: `a[0] + a[1] + a[2] + a[3]` produces four bounds checks with
15+
// indices 0, 1, 2, 3 and the same length. We rewrite the first BC's index
16+
// to 3; forward assertion prop then drops the other three as redundant.
17+
//
18+
// We ensure no observable side effects (other than a bounds check exception)
19+
// can occur between the original check and the subsequent checks.
20+
//
21+
// This phase runs before assertion prop, which then optimizes away
22+
// the trailing, now-redundant checks.
23+
//
24+
25+
#include "jitpch.h"
26+
27+
#ifdef _MSC_VER
28+
#pragma hdrstop
29+
#endif
30+
31+
namespace
32+
{
33+
struct BoundsCheckCandidate
34+
{
35+
// leading bounds check in a candidate set
36+
GenTreeBoundsChk* m_bc;
37+
38+
// array length being checked
39+
ValueNum m_lenVN;
40+
41+
// Max index being checked
42+
int m_offset;
43+
44+
BoundsCheckCandidate(GenTreeBoundsChk* bc, ValueNum lenVN, int offset)
45+
: m_bc(bc)
46+
, m_lenVN(lenVN)
47+
, m_offset(offset)
48+
{
49+
}
50+
};
51+
52+
//------------------------------------------------------------------------
53+
// IsSideEffectBarrier: check if a node blocks bounds check coalescing
54+
//
55+
// Arguments:
56+
// comp - the compiler instance
57+
// node - the node to check
58+
// blockHasEHSuccs - whether the block containing the node has reachable EH successors
59+
//
60+
// Returns:
61+
// true if a node may have a side effect that should prevent us from
62+
// coalescing bounds checks across it. Uses the per-node
63+
// (non-summary) effect flags from GenTree::OperEffects.
64+
//
65+
bool IsSideEffectBarrier(Compiler* comp, GenTree* node, bool blockHasEHSuccs)
66+
{
67+
// A node that lowers to a helper call requires the call flag but is not a
68+
// GT_CALL (for example, a variable-distance long shift on 32-bit targets).
69+
// Treat such nodes as barriers up front: they are effectively calls, and
70+
// OperEffects/OperRequiresGlobRefFlag do not expect to be queried for them.
71+
//
72+
if (node->OperRequiresCallFlag(comp))
73+
{
74+
return true;
75+
}
76+
77+
ExceptionSetFlags exSet;
78+
GenTreeFlags const effects = node->OperEffects(comp, &exSet);
79+
80+
// Calls are barriers, as are nodes whose evaluation order is explicitly
81+
// constrained (GTF_ORDER_SIDEEFF): coalescing must not move a strengthened
82+
// check across an operation the IR says cannot be reordered.
83+
//
84+
if ((effects & (GTF_CALL | GTF_ORDER_SIDEEFF)) != 0)
85+
{
86+
return true;
87+
}
88+
89+
if ((effects & GTF_EXCEPT) != 0)
90+
{
91+
if ((exSet & ~ExceptionSetFlags::IndexOutOfRangeException) != ExceptionSetFlags::None)
92+
{
93+
return true;
94+
}
95+
}
96+
97+
if ((effects & GTF_ASG) != 0)
98+
{
99+
if (!node->OperIsLocalStore())
100+
{
101+
return true;
102+
}
103+
if (!blockHasEHSuccs)
104+
{
105+
return false;
106+
}
107+
LclVarDsc const* const dsc = comp->lvaGetDesc(node->AsLclVarCommon());
108+
return !dsc->lvTracked || dsc->IsLiveInOutOfHandler();
109+
}
110+
111+
return false;
112+
}
113+
} // namespace
114+
115+
//------------------------------------------------------------------------
116+
// optBoundsCheckCoalesce: Coalesce bounds checks within each block.
117+
//
118+
// Returns:
119+
// Suitable phase status.
120+
//
121+
PhaseStatus Compiler::optBoundsCheckCoalesce()
122+
{
123+
if (!doesMethodHaveBoundsChecks())
124+
{
125+
JITDUMP("Method has no bounds checks\n");
126+
return PhaseStatus::MODIFIED_NOTHING;
127+
}
128+
129+
if (fgSsaPassesCompleted == 0)
130+
{
131+
return PhaseStatus::MODIFIED_NOTHING;
132+
}
133+
134+
// Track the current maximum offset seen for a given length VN
135+
// optimization barrier count.
136+
//
137+
struct Key
138+
{
139+
int m_barrierCount;
140+
ValueNum m_lengthVN;
141+
142+
Key(int barrierCount, ValueNum lengthVN)
143+
: m_barrierCount(barrierCount)
144+
, m_lengthVN(lengthVN)
145+
{
146+
}
147+
148+
bool operator==(const Key& other) const
149+
{
150+
return (m_barrierCount == other.m_barrierCount) && (m_lengthVN == other.m_lengthVN);
151+
}
152+
153+
static bool Equals(const Key& x, const Key& y)
154+
{
155+
return (x.m_barrierCount == y.m_barrierCount) && (x.m_lengthVN == y.m_lengthVN);
156+
}
157+
158+
static unsigned GetHashCode(const Key& x)
159+
{
160+
return (unsigned)x.m_lengthVN ^ (unsigned)x.m_barrierCount;
161+
}
162+
};
163+
164+
typedef JitHashTable<Key, Key, int> GroupMap;
165+
166+
bool modified = false;
167+
CompAllocator alloc(getAllocator(CMK_AssertionProp));
168+
169+
ArrayStack<BoundsCheckCandidate> candidates(alloc);
170+
GroupMap groupMap(alloc);
171+
172+
for (BasicBlock* const block : Blocks())
173+
{
174+
candidates.Reset();
175+
groupMap.RemoveAll();
176+
int barrierCount = 0;
177+
bool const blockHasEHSuccs = block->HasPotentialEHSuccs(this);
178+
179+
for (Statement* const stmt : block->Statements())
180+
{
181+
for (GenTree* const node : stmt->TreeList())
182+
{
183+
if (node->OperIs(GT_BOUNDS_CHECK))
184+
{
185+
GenTreeBoundsChk* const bc = node->AsBoundsChk();
186+
if (bc->gtThrowKind != SCK_RNGCHK_FAIL)
187+
{
188+
// A bounds check with a different throw kind throws a
189+
// different exception. Treat it as a barrier so we do not
190+
// reorder a strengthened range check across it and change
191+
// which exception is observed.
192+
//
193+
barrierCount++;
194+
continue;
195+
}
196+
197+
GenTree* const idx = bc->GetIndex();
198+
if (!idx->IsIntCnsFitsInI32())
199+
{
200+
continue;
201+
}
202+
203+
int const offset = static_cast<int>(idx->AsIntCon()->IconValue());
204+
if (offset < 0)
205+
{
206+
continue;
207+
}
208+
209+
// Look through comma-wrapped length nodes.
210+
//
211+
GenTree* const lenNode = bc->GetArrayLength()->gtEffectiveVal();
212+
ValueNum const lenVN = vnStore->VNConservativeNormalValue(lenNode->gtVNPair);
213+
if (lenVN == ValueNumStore::NoVN)
214+
{
215+
continue;
216+
}
217+
218+
Key key(barrierCount, lenVN);
219+
int headIndex;
220+
if (!groupMap.Lookup(key, &headIndex))
221+
{
222+
// First constant-index bounds check with this length VN and this barrier count.
223+
// Start a new group.
224+
//
225+
groupMap.Set(key, candidates.Height());
226+
candidates.Emplace(bc, lenVN, offset);
227+
continue;
228+
}
229+
230+
// Following bounds check. See if this is a new max index.
231+
//
232+
BoundsCheckCandidate& head = candidates.BottomRef(headIndex);
233+
JITDUMP("BC coalesce in " FMT_BB ": [%06u] (offset %d) is redundant given [%06u]\n", block->bbNum,
234+
dspTreeID(bc), offset, dspTreeID(head.m_bc));
235+
if (offset > head.m_offset)
236+
{
237+
head.m_offset = offset;
238+
}
239+
continue;
240+
}
241+
242+
if (IsSideEffectBarrier(this, node, blockHasEHSuccs))
243+
{
244+
barrierCount++;
245+
continue;
246+
}
247+
}
248+
}
249+
250+
// Revise the check made by the first entry in each group, if we
251+
// found a subsequent check at a higher constant index.
252+
//
253+
for (int i = 0; i < candidates.Height(); i++)
254+
{
255+
BoundsCheckCandidate& head = candidates.BottomRef(i);
256+
GenTreeIntCon* const idxCns = head.m_bc->GetIndex()->AsIntCon();
257+
int const original = static_cast<int>(idxCns->IconValue());
258+
if (head.m_offset == original)
259+
{
260+
continue;
261+
}
262+
263+
JITDUMP("BC coalesce in " FMT_BB ": strengthen [%06u] offset %d -> %d (lenVN " FMT_VN ")\n", block->bbNum,
264+
dspTreeID(head.m_bc), original, head.m_offset, head.m_lenVN);
265+
266+
idxCns->SetIconValue(head.m_offset);
267+
idxCns->gtVNPair.SetBoth(vnStore->VNForIntCon(head.m_offset));
268+
modified = true;
269+
}
270+
}
271+
272+
return modified ? PhaseStatus::MODIFIED_EVERYTHING : PhaseStatus::MODIFIED_NOTHING;
273+
}

src/coreclr/jit/compiler.cpp

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4821,6 +4821,10 @@ void Compiler::compCompile(void** methodCodePtr, uint32_t* methodCodeSize, JitFl
48214821

48224822
if (doAssertionProp)
48234823
{
4824+
// Coalesce groups of constant-indexed bounds checks.
4825+
//
4826+
DoPhase(this, PHASE_BOUNDS_CHECK_COALESCE, &Compiler::optBoundsCheckCoalesce);
4827+
48244828
// Assertion propagation
48254829
//
48264830
DoPhase(this, PHASE_ASSERTION_PROP_MAIN, &Compiler::optAssertionPropMain);

src/coreclr/jit/compiler.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7557,6 +7557,7 @@ class Compiler
75577557

75587558
PhaseStatus optCloneLoops();
75597559
PhaseStatus optRangeCheckCloning();
7560+
PhaseStatus optBoundsCheckCoalesce();
75607561
void optCloneLoop(FlowGraphNaturalLoop* loop, LoopCloneContext* context);
75617562
PhaseStatus optUnrollLoops(); // Unrolls loops (needs to have cost info)
75627563
bool optTryUnrollLoop(FlowGraphNaturalLoop* loop, bool* changedIR);

src/coreclr/jit/compphases.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -103,6 +103,7 @@ CompPhaseNameMacro(PHASE_OPTIMIZE_VALNUM_CSES, "Optimize Valnum CSEs",
103103
CompPhaseNameMacro(PHASE_VN_COPY_PROP, "VN based copy prop", false, -1, false)
104104
CompPhaseNameMacro(PHASE_VN_BASED_INTRINSIC_EXPAND, "VN based intrinsic expansion", false, -1, false)
105105
CompPhaseNameMacro(PHASE_OPTIMIZE_BRANCHES, "Redundant branch opts", false, -1, false)
106+
CompPhaseNameMacro(PHASE_BOUNDS_CHECK_COALESCE, "Coalesce bounds checks", false, -1, false)
106107
CompPhaseNameMacro(PHASE_ASSERTION_PROP_MAIN, "Assertion prop", false, -1, false)
107108
CompPhaseNameMacro(PHASE_RANGE_CHECK_CLONING, "Clone blocks with range checks", false, -1, false)
108109
CompPhaseNameMacro(PHASE_IF_CONVERSION, "If conversion", false, -1, false)

0 commit comments

Comments
 (0)