Fix null vnStore dereference in optAssertionProp_HWIntrinsic - #134441
Conversation
|
Azure Pipelines: Successfully started running 5 pipeline(s). 11 pipeline(s) were filtered out due to trigger conditions. There may be pipelines that require an authorized user to comment /azp run to run. |
|
Tagging subscribers to this area: @JulieLeeMSFT, @jakobbotsch |
Agent-Logs-Url: https://github.com/dotnet/runtime/sessions/202c3ee7-28cd-4f3a-8f1b-8abc368ed682 Co-authored-by: EgorBo <523221+EgorBo@users.noreply.github.com>
|
@copilot fix the bug |
Co-authored-by: EgorBo <523221+EgorBo@users.noreply.github.com>
Co-authored-by: EgorBo <523221+EgorBo@users.noreply.github.com>
Co-authored-by: EgorBo <523221+EgorBo@users.noreply.github.com>
Fixed in ef7978d (comment tweak in 4ec109c).
Reproduced on x64 rather than arm64 — for After the fix it's clean, and inverting the assert confirms the helper is still reached during the real I also audited the rest of that switch: every other helper already bails before touching I dropped the speculative test the previous session had added and did not add a regression test, per your standing preference — say the word if you'd like one. |
vnStore dereference in optAssertionProp_HWIntrinsicvnStore dereference in optAssertionProp_HWIntrinsic
There was a problem hiding this comment.
Copilot review overview
🔵 Needs a closer look
Add focused regression coverage for the local assertion-propagation case.
Review effort: Lite
Findings: None
What changed in this PR
Fixes a JIT null vnStore dereference by guarding hardware-intrinsic assertion propagation during local propagation.
Changes:
- Adds a
vnStoreprecondition assertion. - Skips the VN-dependent optimization during local assertion propagation.
| File | Summary |
|---|---|
src/coreclr/jit/assertionprop.cpp |
Guards VN-dependent hardware-intrinsic handling; focused regression coverage is still needed. |
|
PTAL @tannergooding @dotnet/jit-contrib a quick fix to unblock the perf team |
|
/ba-g unrelated LA64 build failure |
|
Should this also be backported to release 11? I think we are currently using 11 feeds for opt so I will see if we can update to 12 to get this fix once it flows to VMR. |
|
After some more looking, I think this should also be backported to release/11.0 as the net12 feeds don't seem to have everything we will need before move optimization over yet, and based on last year, it was not until December that we did the first steps toward moving to running with net11's feeds. |
|
/backport to release/11.0 |
|
Started backporting to |
…Intrinsic` (#134665) Backport of #134441 to release/11.0 /cc @LoopedBard3 @Copilot ## Customer Impact - [ ] Customer reported - [x] Found internally The JIT can crash while compiling SIMD `ExtractMostSignificantBits` operations because local assertion propagation invokes an optimization before its value-number store exists. This blocks Linux arm64 JIT PGO training for .NET 11, preventing collection of fresh profiles for the shipped arm64 JIT. The reported repro passes with the ordinary shipping JIT. See #134435. ## Regression - [x] Yes - [ ] No Introduced during .NET 11 development by #129688, which added the hardware-intrinsic assertion-propagation helper and its unguarded call during local assertion propagation. ## Testing In the original PR, the failure was reproduced with a Checked x64 JIT using a temporary `vnStore != nullptr` assertion, then verified fixed. All 22 `JIT/opt/InstructionCombining` tests passed with default settings and with `DOTNET_EnableAVX512=0`. Validation also confirmed that the optimization still runs during global assertion propagation. No dedicated regression test was added. The issue was missed because the PGO pipeline had been pinned to an SDK predating the regression, and the ordinary shipping JIT did not expose the crash. ## Risk Low. The change only adds a phase guard and a precondition assertion. It skips the value-number-dependent optimization during local assertion propagation, where value numbers are unavailable, matching the neighboring array-length case. Global assertion propagation and its optimization are unchanged. Co-authored-by: Copilot <198982749+Copilot@users.noreply.github.com> Co-authored-by: EgorBo <523221+EgorBo@users.noreply.github.com>
vnStoredereference inoptAssertionProp_HWIntrinsicduring local assertion prop (arm64) #134435vnStoredereference inoptAssertionProp_HWIntrinsicduring local assertion prop (arm64) #134435