-
Notifications
You must be signed in to change notification settings - Fork 4.9k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
JIT: suboptimal block layout #76872
Comments
Tagging subscribers to this area: @JulieLeeMSFT, @jakobbotsch Issue Detailsbool M(string a) => "b" == a; Emits: ; Method P:M(System.String):bool:this
G_M31338_IG01:
G_M31338_IG02:
test rdx, rdx
je SHORT G_M31338_IG06
G_M31338_IG03:
cmp dword ptr [rdx+08H], 1
jne SHORT G_M31338_IG05
G_M31338_IG04:
xor eax, eax
cmp word ptr [rdx+0CH], 98
sete al
jmp SHORT G_M31338_IG07
G_M31338_IG05:
xor eax, eax
jmp SHORT G_M31338_IG07
G_M31338_IG06:
xor eax, eax
G_M31338_IG07:
ret
; Total bytes of code: 30 It seems like IG05 (BB05) block is redundant and could use IG06 (BB06) instead:
|
cc @AndyAyersMS |
Related: #8795 |
is it? here we don't merge tails (multiple returns) but just try to merge equal blocks with the same successor, it doesn't have to be a tail 🙂 |
I don't think tail merging necessarily has to mean the tail of functions, it can cover merging the tails of two basic blocks in general (i.e. same instructions, same terminator) #8795 seems like it uses the more general description, doesn't it? I guess its particular wording still doesn't cover this case completely though. |
IMO it's too abstract to ever be closed while this one represents a concrete (popular) case that we can handle |
Yeah, I think considering the special cases is valuable. But we should just be mindful that we get the mileage out of the transformation we can, for example it might not be too difficult to generalize some parts of "fold identical BBs" to be more like general tail merging. |
It is good to keep the more general optimizations in mind. Tail merging (and its dual, head merging) require developing quick and reliable ways to compare trees, something we currently lack. Generalizations of tail merging that consider reorderable or semantically equivalent but syntactically different computations are probably too costly for us to consider. |
Note in debug we have |
Add a phase that looks for common tail statements in a block's predecessors and merges them. Run it both before and after morph. Closes dotnet#8795. Closes dotnet#76872.
Codegen with #77103: G_M64862_IG01: ;; offset=0000H
;; size=0 bbWeight=1 PerfScore 0.00
G_M64862_IG02: ;; offset=0000H
4885C9 test rcx, rcx
7412 je SHORT G_M64862_IG05
;; size=5 bbWeight=1 PerfScore 1.25
G_M64862_IG03: ;; offset=0005H
83790801 cmp dword ptr [rcx+08H], 1
750C jne SHORT G_M64862_IG05
;; size=6 bbWeight=0.25 PerfScore 1.00
G_M64862_IG04: ;; offset=000BH
33C0 xor eax, eax
6683790C62 cmp word ptr [rcx+0CH], 98
0F94C0 sete al
EB02 jmp SHORT G_M64862_IG06
;; size=12 bbWeight=0.12 PerfScore 0.78
G_M64862_IG05: ;; offset=0017H
33C0 xor eax, eax
;; size=2 bbWeight=0.25 PerfScore 0.06
G_M64862_IG06: ;; offset=0019H
C3 ret
;; size=1 bbWeight=1 PerfScore 1.00
; Total bytes of code 26 |
Add a phase that looks for common tail statements in a block's predecessors and merges them. Run it both before and after morph. Also * add range enable config and overall `JitEnableTailMerge` config * add indir flag checking to `GenTree::Compare` * remove an apparently unnecessary assert from loop recognition. Closes #8795. Closes #76872.
Emits:
It seems like IG05 (BB05) block is redundant:
BB04 and BB05 are exactly the same and have the same successor
The text was updated successfully, but these errors were encountered: