Skip to content

Commit

Permalink
Fix volatility checks in VN and loop side effects code (#85467)
Browse files Browse the repository at this point in the history
* Fix volatile check in loop side effects code

* Restore the volatility logic in VN

Volatile stores should be processed at the point of ASG.
  • Loading branch information
SingleAccretion authored Apr 28, 2023
1 parent e482bba commit 231f85f
Show file tree
Hide file tree
Showing 2 changed files with 8 additions and 11 deletions.
2 changes: 1 addition & 1 deletion src/coreclr/jit/optimizer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -8640,7 +8640,7 @@ bool Compiler::optComputeLoopSideEffectsOfBlock(BasicBlock* blk)
{
GenTree* arg = lhs->AsIndir()->Addr()->gtEffectiveVal(/*commaOnly*/ true);

if ((tree->gtFlags & GTF_IND_VOLATILE) != 0)
if ((lhs->gtFlags & GTF_IND_VOLATILE) != 0)
{
memoryHavoc |= memoryKindSet(GcHeap, ByrefExposed);
continue;
Expand Down
17 changes: 7 additions & 10 deletions src/coreclr/jit/valuenum.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -10721,8 +10721,13 @@ void Compiler::fgValueNumberTree(GenTree* tree)
ValueNumPair addrXvnp;
vnStore->VNPUnpackExc(addr->gtVNPair, &addrNvnp, &addrXvnp);

// To be able to propagate exception sets, we give location nodes the "Void" VN.
if ((tree->gtFlags & GTF_IND_ASG_LHS) != 0)
{
tree->gtVNPair = vnStore->VNPWithExc(vnStore->VNPForVoid(), addrXvnp);
}
// Is the dereference immutable? If so, model it as referencing the read-only heap.
if (tree->gtFlags & GTF_IND_INVARIANT)
else if (tree->gtFlags & GTF_IND_INVARIANT)
{
assert(!isVolatile); // We don't expect both volatile and invariant

Expand Down Expand Up @@ -10822,9 +10827,7 @@ void Compiler::fgValueNumberTree(GenTree* tree)
ValueNum newUniq = vnStore->VNForExpr(compCurBB, tree->TypeGet());
tree->gtVNPair = vnStore->VNPWithExc(ValueNumPair(newUniq, newUniq), addrXvnp);
}
// In general we skip GT_IND nodes on that are the LHS of an assignment. (We labeled these earlier.)
// We will "evaluate" this as part of the assignment.
else if ((tree->gtFlags & GTF_IND_ASG_LHS) == 0)
else
{
var_types loadType = tree->TypeGet();
ssize_t offset = 0;
Expand Down Expand Up @@ -10867,12 +10870,6 @@ void Compiler::fgValueNumberTree(GenTree* tree)

tree->gtVNPair = vnStore->VNPWithExc(tree->gtVNPair, addrXvnp);
}

// To be able to propagate exception sets, we give location nodes the "Void" VN.
if ((tree->gtFlags & GTF_IND_ASG_LHS) != 0)
{
tree->gtVNPair = vnStore->VNPWithExc(vnStore->VNPForVoid(), addrXvnp);
}
}
else if (tree->OperGet() == GT_CAST)
{
Expand Down

0 comments on commit 231f85f

Please sign in to comment.