Skip to content

Commit 3d9b089

Browse files
AndyAyersMSCopilot
andcommitted
[release/8.0-staging] Fix loop hoist memory-dependence tracking (#122057)
Port the .NET 9 fix in `optRecordLoopMemoryDependence` so that memory dependencies coming from nested loops are recorded against the closest ancestor loop that contains the consuming tree, rather than being silently discarded. Without this fix, when the loop hoister considers an IND whose value VN has been folded to a constant via VN's MapStore/MapSelect machinery, the `IsTreeLoopMemoryInvariant` check finds no entry in `NodeToLoopMemoryBlockMap` and returns `true` vacuously. The IND is then relocated to the loop preheader where it loads a stale value instead of the post-store value the VN-folded constant represents. Example pattern from the linked issue: ```csharp for (sbyte i = -126; i > -128; i--) { for (byte j = 2; j > 0; j--) { s_rt.Checksum(""c_0"", var4); // virtual call inside inner loop } s_2 = 1; s_2 = s_2--; // hoisted IND reads stale s_2 } ``` Fix: walk up the parent chain of the update loop until one is found that contains the tree's block (the .NET 9 `FlowGraphNaturalLoops` version generalizes this; the 8.0 port uses `optLoopTable[..].lpParent`). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1 parent 29db7c7 commit 3d9b089

3 files changed

Lines changed: 879 additions & 6 deletions

File tree

src/coreclr/jit/optimizer.cpp

Lines changed: 13 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -7150,14 +7150,21 @@ void Compiler::optRecordLoopMemoryDependence(GenTree* tree, BasicBlock* block, V
71507150
updateLoopNum = updateParentLoopNum;
71517151
}
71527152

7153-
// If the update block is not the header of a loop containing
7154-
// block, we can also ignore the update.
7153+
// If the memory definition is part of an ancestor loop of `loopNum` then
7154+
// `tree` depends on memory defined in that ancestor loop. Walk up the parent
7155+
// chain of `updateLoopNum` looking for such an ancestor. Otherwise ignore
7156+
// the update.
71557157
//
7156-
if (!optLoopContains(updateLoopNum, loopNum))
7158+
while ((updateLoopNum != BasicBlock::NOT_IN_LOOP) && !optLoopContains(updateLoopNum, loopNum))
71577159
{
7158-
JITDUMP(" ==> Not updating loop memory dependence of [%06u]/" FMT_LP ", memory " FMT_VN "/" FMT_LP
7159-
" is not defined in an enclosing loop\n",
7160-
dspTreeID(tree), loopNum, memoryVN, updateLoopNum);
7160+
updateLoopNum = optLoopTable[updateLoopNum].lpParent;
7161+
}
7162+
7163+
if (updateLoopNum == BasicBlock::NOT_IN_LOOP)
7164+
{
7165+
JITDUMP(" ==> Not updating loop memory dependence of [%06u]/" FMT_LP ", memory " FMT_VN
7166+
" is not dependent on an ancestor loop\n",
7167+
dspTreeID(tree), loopNum, memoryVN);
71617168
return;
71627169
}
71637170

0 commit comments

Comments
 (0)