Conversation
|
Azure Pipelines: 16 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 |
Co-authored-by: EgorBo <523221+EgorBo@users.noreply.github.com>
Co-authored-by: EgorBo <523221+EgorBo@users.noreply.github.com>
|
@copilot remove changes to StructWithGC_Zeroing tests, no tests needed. |
Co-authored-by: EgorBo <523221+EgorBo@users.noreply.github.com>
|
@jakobbotsch @AndyAyersMS @dotnet/jit-contrib so this PR removes hasExplicitInit entirely (with the flag) - see PR changes with "ignore whitespaces" mode. Diffs - not that bad for non-tests collections, but still a regression. @jakobbotsch pointed out that the current change is not correct - it assumes at the SSA liveness stage we know exactly what parts of code are no-gc areas which is not correct, so presumably there might be a scenario when we have a GC local, it's not zeroed yet and we hit an unexpected gc safe point (e.g. fully interruptible part) - it will crash. Should we take it as is given it's a correctness fix? I prototyped a fix that does the same analysis at the codegen phase that recovers the regressions, but it was a bit too complex for this PR IMO. |
|
It seems that regression happen moslty in small methods where the existing opt managed to avoid any zeroing in prolog, e.g. nuint Test(ref int value)
{
fixed (int* p = &value)
return (nuint)p & 1;
}Main: ; Method WriteBarrierBenchmarks:Test(byref):nuint (FullOpts)
G_M49291_IG01:
push rax
G_M49291_IG02:
mov bword ptr [rsp], rcx
mov rax, rcx
and rax, 1
G_M49291_IG03:
add rsp, 8
ret
; Total bytes of code: 17PR: ; Method WriteBarrierBenchmarks:Test(byref):nuint (FullOpts)
G_M49291_IG01:
push rax
xor eax, eax
mov qword ptr [rsp], rax
G_M49291_IG02:
mov bword ptr [rsp], rcx
mov rax, rcx
and rax, 1
G_M49291_IG03:
add rsp, 8
ret
; Total bytes of code: 23(in the real world, it's likely inlined to somewhere where we already zero locals with SIMD anyway) |
|
I'm a bit worried about the potential impact of this change. Wondering if we should consider something more surgical. Is this a regression in .NET 11? |
Uh oh!
There was an error while loading. Please reload this page.