JIT: Improve VN for (EQ/NE (RELOP ...), 0/1) #60943
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Try and re-express these VNs in terms of the RELOP or its complement.
dotnet-issue-labeler
bot
added
the
area-CodeGen-coreclr
|
Tagging subscribers to this area: @JulieLeeMSFT Issue DetailsTry and re-express these VNs in terms of the RELOP or its complement.
|
|
cc @dotnet/jit-contrib Next installment in the ongoing saga of removing branch redundancy. Surprising number and amount of diffs. Will post some diff snippets shortly. aspnet.run.windows.x64.checked.mch:Detail diffsbenchmarks.run.windows.x64.checked.mch:Detail diffscoreclr_tests.pmi.windows.x64.checked.mch:Detail diffslibraries.crossgen2.windows.x64.checked.mch:Detail diffslibraries.pmi.windows.x64.checked.mch:Detail diffslibraries_tests.pmi.windows.x64.checked.mch:Detail diffs |
|
Sample diffs ;; Assembly listing for method Microsoft.CodeAnalysis.CSharp.DiagnosticsPass:IsInterlockedAPI(Microsoft.CodeAnalysis.CSharp.Symbol):bool:this
;; before
G_M7814_IG03: ; gcrefRegs=000000C0 {rsi rdi}, byrefRegs=00000000 {}, byref, isz
; gcrRegs -[rax]
mov rcx, rsi
; gcrRegs +[rcx]
mov rax, qword ptr [rsi]
mov rax, qword ptr [rax+80]
call [rax+56]hackishModuleName:hackishMethodName():Microsoft.CodeAnalysis.CSharp.Symbols.NamedTypeSymbol:this
; gcrRegs -[rcx rsi] +[rax]
; gcr arg pop 0
test rax, rax
jne SHORT G_M7814_IG04
xor esi, esi
je SHORT G_M7814_IG07
jmp SHORT G_M7814_IG05
;; bbWeight=0.50 PerfScore 5.88
G_M7814_IG04: ; gcrefRegs=00000081 {rax rdi}, byrefRegs=00000000 {}, byref, isz
cmp rdi, rax
je SHORT G_M7814_IG05
;; after
G_M7814_IG03: ; gcrefRegs=000000C0 {rsi rdi}, byrefRegs=00000000 {}, byref, isz
; gcrRegs -[rax]
mov rcx, rsi
; gcrRegs +[rcx]
mov rax, qword ptr [rsi]
mov rax, qword ptr [rax+80]
call [rax+56]hackishModuleName:hackishMethodName():Microsoft.CodeAnalysis.CSharp.Symbols.NamedTypeSymbol:this
; gcrRegs -[rcx rsi] +[rax]
; gcr arg pop 0
test rax, rax
je SHORT G_M7814_IG06
cmp rdi, rax
je SHORT G_M7814_IG04Here's a fairly common pattern. Haven't yet mapped this back to see where it comes from. ;; Assembly listing for method System.Threading.ThreadPool:UnsafeQueueUserWorkItem(System.Action`1[Byte],ubyte,bool):bool
;; before
mov r14, rax
; gcrRegs +[r14]
mov ecx, 1
mov rdx, 0xD1FFAB1E ; string handle
mov rdx, gword ptr [rdx]
; gcrRegs +[rdx]
test ecx, ecx
jne SHORT G_M35947_IG04
;; bbWeight=1 PerfScore 14.50
G_M35947_IG03: ; gcrefRegs=00004064 {rdx rbp rsi r14}, byrefRegs=00000000 {}, byref
; gcrRegs -[rax]
mov rcx, rdx
; gcrRegs +[rcx]
call hackishModuleName:hackishMethodName()
; gcrRegs -[rcx rdx]
; gcr arg pop 0
;; bbWeight=0.50 PerfScore 0.62
G_M35947_IG04: ; gcrefRegs=00004060 {rbp rsi r14}, byrefRegs=00000000 {}, byref
lea rcx, bword ptr [r14+16]
;; after
mov r14, rax
; gcrRegs +[r14]
lea rcx, bword ptr [r14+16] |
It's also a little curious that there are so many more PMI diffs compared to crossgen2 diffs, do you know why? |
No, I don't. Could be we're seeing a runtime idiom that is hidden/abstracted in R2R. Or else the patterns in R2R are just different. I'll see if I can track this down. |
because of generics? like every generic call-site is instantiated with one of 8 predefined types in PMI mode (or I am wrong?) |
|
Hmm, the second pattern above looks like it comes (in part) from So perhaps the diffs here are overstated as we're evidently doing a PMI collection of a checked SPC? We really need to make sure we're not capturing checked SPC methods as part of SPMI. |
|
Let me run regular PMI diffs here and see what it says... |
|
PMI diffs suggest that many of the sizeable diffs above are indeed from eliminating various |
|
@dotnet/jit-contrib ping -- this is still interesting, just not as impactful as it first seemed. |
BruceForstall
approved these changes
Oct 28, 2021
|
/azp run runtime-coreclr jitstress |
|
Azure Pipelines successfully started running 1 pipeline(s). |
|
/azp run runtime-coreclr outerloop |
|
Azure Pipelines successfully started running 1 pipeline(s). |
ghost
locked as resolved and limited conversation to collaborators
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Try and re-express these VNs in terms of the RELOP or its complement.