Skip to content

Commit

Permalink
JIT: workaround for unreliable change detection in fgReorderBlocks (#…
Browse files Browse the repository at this point in the history
…48516)

This method costs trees, which can in turn modify the IR by swapping operands.
As a result the bool value returned doesn't properly reflect whether any
changes happened.

This impacts proper reporting phase status by `optOptimizeLayout'. Since phase
status just gates post-phase dumps and checks, we'll just claim this phase
always modifies IR.

Add similar workaround for `optInvertLoops`.

Fixes #48494.
Fixes #48495.
  • Loading branch information
AndyAyersMS authored Feb 20, 2021
1 parent 4d310fb commit da828a2
Showing 1 changed file with 19 additions and 1 deletion.
20 changes: 19 additions & 1 deletion src/coreclr/jit/optimizer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -4484,7 +4484,7 @@ PhaseStatus Compiler::optInvertLoops()
}
}

const bool madeChanges = fgModified;
bool madeChanges = fgModified;

if (madeChanges)
{
Expand All @@ -4493,6 +4493,15 @@ PhaseStatus Compiler::optInvertLoops()
fgModified = false;
}

// optInvertWhileLoop can cause IR changes even if it does not modify
// the flow graph. It calls gtPrepareCost which can cause operand swapping.
// Work around this for now.
//
// Note phase status only impacts dumping and checking done post-phase,
// it has no impact on a release build.
//
madeChanges = true;

return madeChanges ? PhaseStatus::MODIFIED_EVERYTHING : PhaseStatus::MODIFIED_NOTHING;
}

Expand All @@ -4514,6 +4523,15 @@ PhaseStatus Compiler::optOptimizeLayout()
madeChanges |= fgReorderBlocks();
madeChanges |= fgUpdateFlowGraph();

// fgReorderBlocks can cause IR changes even if it does not modify
// the flow graph. It calls gtPrepareCost which can cause operand swapping.
// Work around this for now.
//
// Note phase status only impacts dumping and checking done post-phase,
// it has no impact on a release build.
//
madeChanges = true;

return madeChanges ? PhaseStatus::MODIFIED_EVERYTHING : PhaseStatus::MODIFIED_NOTHING;
}

Expand Down

0 comments on commit da828a2

Please sign in to comment.