JIT: Allow cloning addresses even before initobj - #79341
Conversation
Roslyn emits ldloca + dup + initobj when initializing structs. Normally we clone address trees instead of creating a local for them (which will address expose the local), but we treat this initobj pattern specially. Remove this special treatment. It means we sometimes end up with slightly larger code because we no longer have a register with the address in it (could potentially be fixed by CSE), but avoiding the address exposure seems like the right trade off to me. Fix dotnet#42354
ghost
added
the
area-CodeGen-coreclr
|
Tagging subscribers to this area: @JulieLeeMSFT, @jakobbotsch Issue DetailsRoslyn emits ldloca + dup + initobj when initializing structs. Normally we clone address trees instead of creating a local for them (which will address expose the local), but we treat this initobj pattern specially. Remove this special treatment. It means we sometimes end up with slightly larger code because we no longer have a register with the address in it (could potentially be fixed by CSE), but avoiding the address exposure seems like the right trade off to me. Fix #42354
|
|
Was it me who added it? I think I was trying to minimize number of size regressions in the diffs but didn't analyze them so LGTM assuming diffs are perf improvements |
Yes, looks like it was (although it's possible the two fixed issues also needed #72714 before they actually were fixed) All the regressions I spot checked looked like: - lea rcx, bword ptr [rsp+30H]
- ; byrRegs +[rcx]
- mov byte ptr [rcx], 0
- mov bword ptr [rsp+20H], rcx
- ; byr arg write
+ mov byte ptr [rsp+30H], 0
+ lea rcx, [rsp+30H]
+ mov qword ptr [rsp+20H], rcx
mov rcx, rdi
; gcrRegs +[rcx]
- ; byrRegs -[rcx]So we just get a slightly larger instruction for accessing some stack memory, which I doubt affects performance significantly. On the other hand, not having the local address exposed means we sometimes are able to completely eliminate them after inlining and other optimizations, so that seems worth it to me (and fixes those two issues). |
|
/azp run runtime-coreclr superpmi-replay |
|
Azure Pipelines successfully started running 1 pipeline(s). |
2 participants
Roslyn emits ldloca + dup + initobj when initializing structs. Normally we clone address trees instead of creating a local for them (which will address expose the local), but we treat this initobj pattern specially.
Remove this special treatment. It means we sometimes end up with slightly larger code because we no longer have a register with the address in it (could potentially be fixed by CSE), but avoiding the address exposure seems like the right trade off to me.
Fix #42354
Fix #57055