Skip to content

JIT: Allow early prop to reorder null checks - #71435

Closed
jakobbotsch wants to merge 1 commit into
dotnet:mainfrom
jakobbotsch:reorder-null-checks
Closed

JIT: Allow early prop to reorder null checks#71435
jakobbotsch wants to merge 1 commit into
dotnet:mainfrom
jakobbotsch:reorder-null-checks

Conversation

@jakobbotsch

@jakobbotschjakobbotsch commented Jun 29, 2022

Copy link
Copy Markdown
Member

We currently do not allow two null checks on separate values to be
reordered with one another. This should be fine to do as the exceptions
thrown are indistinguishable from one another up to debug information,
which we do not make guarantees about in optimized builds.
We can use the machinery added in #70893 to easily do this.

The net effect of this change is that we then allow removing a null
check if there is a later equivalent null check, even if it comes after
another unrelated null check.

We currently do not allow two null checks on separate values to be
reordered with one another. This should be fine to do as the exceptions
thrown are indistinguishable from one another up to debug information,
which we do not make guarantees about in optimized builds.
The net effect of this change is that we then allow removing a null
check if there is a later equivalent null check, even if it comes after
another unrelated null check.
@ghostghost added the area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI label Jun 29, 2022
@ghost

Copy link
Copy Markdown

Tagging subscribers to this area: @JulieLeeMSFT, @jakobbotsch
See info in area-owners.md if you want to be subscribed.

Issue Details

We currently do not allow two null checks on separate values to be
reordered with one another. This should be fine to do as the exceptions
thrown are indistinguishable from one another up to debug information,
which we do not make guarantees about in optimized builds.

The net effect of this change is that we then allow removing a null
check if there is a later equivalent null check, even if it comes after
another unrelated null check.

Author:jakobbotsch
Assignees:-
Labels:

area-CodeGen-coreclr

Milestone:-

@jakobbotsch

jakobbotsch commented Jun 29, 2022

Copy link
Copy Markdown
MemberAuthor

A few example diffs:

 ; gcrRegs -[rax]
; byrRegs +[rax]
mov rdi, gword ptr [rax]
; gcrRegs +[rdi]
- cmp byte ptr [rdi], dil
add rdi, 0x800
; gcrRegs -[rdi]
; byrRegs +[rdi]
mov ebp, dword ptr [rax+8]
dec ebp
imul ebp, dword ptr [rdi+20]
lea r15, bword ptr [rdi+24]
 mov rax, r14
; gcrRegs -[rax]
; byrRegs +[rax]
mov rdi, gword ptr [rax]
; gcrRegs +[rdi]
- cmp byte ptr [rdi], dil
lea rdx, bword ptr [rdi+0800H]
; byrRegs +[rdx]
mov edi, dword ptr [rax+8]
; gcrRegs -[rdi]
dec edi
imul edi, dword ptr [rdx+20]
G_M1099_IG01: ; gcrefRegs=00000000 {}, byrefRegs=00000000 {}, byref, nogc <-- Prolog IG
push rdi
push rsi
push rbx
sub rsp, 32
vzeroupper
;; size=10 bbWeight=1 PerfScore 4.25
G_M1099_IG02: ; gcrefRegs=00000000 {}, byrefRegs=00000006 {rcx rdx}, byref, isz
; byrRegs +[rcx rdx]
- cmp byte ptr [rcx], cl
lea rsi, bword ptr [rcx+16]
; byrRegs +[rsi]
add rdx, 16
mov edi, dword ptr [rdx]
vmovsd xmm0, qword ptr [rdx+8]
vmovsd xmm1, qword ptr [rsi+8]
vucomisd xmm0, xmm1
jbe SHORT G_M1099_IG04

@jakobbotsch

Copy link
Copy Markdown
MemberAuthor

This is a bit more tricky than this because of big offsets, e.g. in the failing test we have IR like

N002 ( 2, 2) [000058] ---X-------├──▌NULLCHECK byte N001 ( 1, 1) [000057] -----------└──▌LCL_VAR byref V01 loc0 u:2
...
N006 ( 4, 7) [000063] D--XG--N---└──▌IND int N005 ( 2, 5) [000061] -------N---└──▌ADD byref N003 ( 1, 1) [000059] -----------├──▌LCL_VAR byref V01 loc0 u:2N004 ( 1, 4) [000060] -----------└──▌CNS_INT long 0x31C2000 field offset Fseq[lastField]
...
N002 ( 2, 2) [000065] ---X-------├──▌NULLCHECK byte N001 ( 1, 1) [000064] -----------└──▌LCL_VAR byref V01 loc0 u:2

Today the only thing that prevents us from removing [000058] is the GTF_EXCEPT on [000063]; otherwise we would remove it because of the upcoming 000065. With this change we allow removing it, which is obviously not correct. We could maybe handle it by parsing the offset like in other places in the JIT, but that would be more involved.

@jakobbotsch
jakobbotsch deleted the reorder-null-checks branch June 29, 2022 20:00
@ghostghost locked as resolved and limited conversation to collaborators Jul 30, 2022
Sign up for freeto subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

area-CodeGen-coreclrCLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant

@jakobbotsch