Skip to content

Commit

Permalink
Prevent hoisting nodes with order side effects (#100160)
Browse files Browse the repository at this point in the history
If a node has an order side effect, we can't hoist it at all:
we don't know what the order dependence actually is. For example,
assertion prop might have determined a node can't throw an exception,
and eliminated the `GTF_EXCEPT` flag, replacing it with `GTF_ORDER_SIDEEFF`.
We can't hoist because we might then hoist above the expression that
led assertion prop to make that decision. This can happen in JitOptRepeat,
where hoisting can follow assertion prop.
  • Loading branch information
BruceForstall authored Mar 26, 2024
1 parent 9083a3b commit 05f5a50
Showing 1 changed file with 11 additions and 2 deletions.
13 changes: 11 additions & 2 deletions src/coreclr/jit/optimizer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -4616,6 +4616,15 @@ void Compiler::optHoistLoopBlocks(FlowGraphNaturalLoop* loop,
// hence this check is not present in optIsCSEcandidate().
return true;
}
else if ((node->gtFlags & GTF_ORDER_SIDEEFF) != 0)
{
// If a node has an order side effect, we can't hoist it at all: we don't know what the order
// dependence actually is. For example, assertion prop might have determined a node can't throw
// an exception, and eliminated the GTF_EXCEPT flag, replacing it with GTF_ORDER_SIDEEFF. We
// can't hoist because we might then hoist above the expression that led assertion prop to make
// that decision. This can happen in JitOptRepeat, where hoisting can follow assertion prop.
return false;
}

// Tree must be a suitable CSE candidate for us to be able to hoist it.
return m_compiler->optIsCSEcandidate(node);
Expand Down Expand Up @@ -4806,7 +4815,7 @@ void Compiler::optHoistLoopBlocks(FlowGraphNaturalLoop* loop,
}
else if (!top.m_hoistable)
{
top.m_failReason = "not handled by cse";
top.m_failReason = "not handled by hoisting or CSE";
}
#endif

Expand Down Expand Up @@ -4889,7 +4898,7 @@ void Compiler::optHoistLoopBlocks(FlowGraphNaturalLoop* loop,
treeIsHoistable = IsNodeHoistable(tree);
if (!treeIsHoistable)
{
INDEBUG(failReason = "not handled by cse";)
INDEBUG(failReason = "not handled by hoisting or CSE";)
}
}

Expand Down

0 comments on commit 05f5a50

Please sign in to comment.