JIT: handle x86 tail call via helper when control expression is spilled#120272
JIT: handle x86 tail call via helper when control expression is spilled#120272AndyAyersMS merged 3 commits intodotnet:mainfrom
Conversation
Make sure we order the control expression after the spill. Also add a range control to selectively enable instrumentation stress. Fixes dotnet#120170.
There was a problem hiding this comment.
Pull Request Overview
This PR fixes a bug in the JIT compiler's handling of x86 tail calls via helper functions when the control expression gets spilled to a temporary variable. The fix ensures proper ordering of code by inserting the call target range before the call rather than after an argument, and then reordering all putargs to be adjacent to the call.
- Moves call target range insertion from after arg0 to before the call in
LowerTailCallViaJitHelper - Adds a call to
MoveCFGCallArgsto properly reorder putargs after the range insertion - Introduces a new configuration option
JitInstrumentIfOptimizingRangeto selectively control instrumentation stress testing
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| src/coreclr/jit/lower.cpp | Updates tail call lowering logic to handle spilled control expressions by changing insertion order and adding putarg reordering |
| src/coreclr/jit/jitconfigvalues.h | Adds new configuration string for instrumentation range control |
| src/coreclr/jit/compiler.cpp | Implements range-based filtering for the instrumentation optimization feature |
|
@jakobbotsch PTAL Small handful of diffs. |
|
Tagging subscribers to this area: @JulieLeeMSFT, @jakobbotsch |
|
/azp run runtime-coreclr pgostress, runtime-coreclr libraries-pgo |
|
Azure Pipelines successfully started running 2 pipeline(s). |
|
Remaining failures look like timeouts / stackoverflow tester. Tier1+instr is likely fairly slow; I wonder if we should invest in making it faster. |
|
/azp run runtime-coreclr pgostress, runtime-coreclr libraries-pgo |
|
Azure Pipelines successfully started running 2 pipeline(s). |
|
pgostress failure is #110173 libraries-pgo is a bit messy, but failures are either timeouts (#120902), ooms (#120918), or one failure that seems timing related and unrelated to this PR: |
Make sure we order the control expression after the spill.
Also add a range control to selectively enable instrumentation stress.
Fixes #120170.