Skip to content
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: Avoid duplicate hash table lookups in VN #104873

Merged
merged 2 commits into from
Jul 16, 2024

Conversation

jakobbotsch
Copy link
Member

We can switch to recently added LookupPointerOrAdd to avoid duplicate hash table lookups for the VNForFunc family of functions.

We can switch to recently added `LookupPointerOrAdd` to avoid duplicate
hash table lookups for the `VNForFunc` family of functions.
@dotnet-issue-labeler dotnet-issue-labeler bot added the area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI label Jul 14, 2024
Copy link
Contributor

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

@jakobbotsch
Copy link
Member Author

/azp run runtime-coreclr superpmi-replay

Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@jakobbotsch
Copy link
Member Author

Detailed tp diff on linux which has TP regressions:

Base: 256984672904, Diff: 257992462122, +0.3922%

1992662466 : NA           : 24.57% : +0.7754% : _ZN12JitHashTableIN13ValueNumStore12VNDefFuncAppILm2EEENS0_20VNDefFuncAppKeyFuncsILm2EEEj13CompAllocator20JitHashTableBehaviorE18LookupPointerOrAddES2_j
699279625  : +2849085.83% : 8.62%  : +0.2721% : _ZN13ValueNumStore9VNWithExcEjj                                                                                                                         
514754473  : +519.25%     : 6.35%  : +0.2003% : _ZN13ValueNumStore11VNForIntConEi                                                                                                                       
229501579  : +200.95%     : 2.83%  : +0.0893% : _ZN13ValueNumStore13VNExcSetUnionEjj                                                                                                                    
160819956  : NA           : 1.98%  : +0.0626% : _ZN12JitHashTableIN13ValueNumStore12VNDefFuncAppILm1EEENS0_20VNDefFuncAppKeyFuncsILm1EEEj13CompAllocator20JitHashTableBehaviorE18LookupPointerOrAddES2_j
125665414  : NA           : 1.55%  : +0.0489% : _ZN3LIR3UseC2ERNS_5RangeEPP7GenTreeS4_                                                                                                                  
121009266  : NA           : 1.49%  : +0.0471% : _ZN12JitHashTableIl25JitLargePrimitiveKeyFuncsIlEj13CompAllocator20JitHashTableBehaviorE18LookupPointerOrAddElj                                         
102284338  : NA           : 1.26%  : +0.0398% : _ZN12JitHashTableIN13ValueNumStore8VNHandleES1_j13CompAllocator20JitHashTableBehaviorE18LookupPointerOrAddES1_j                                         
78013216   : +20.96%      : 0.96%  : +0.0304% : _ZN13ValueNumStore13VNZeroForTypeE9var_types                                                                                                            
59685109   : NA           : 0.74%  : +0.0232% : _ZN13ValueNumStore5ChunkC2E13CompAllocatorPj9var_typesNS_17ChunkExtraAttribsE                                                                           
54909810   : NA           : 0.68%  : +0.0214% : _ZN12JitHashTableIN13ValueNumStore12VNDefFuncAppILm3EEENS0_20VNDefFuncAppKeyFuncsILm3EEEj13CompAllocator20JitHashTableBehaviorE18LookupPointerOrAddES2_j
51022378   : +27.87%      : 0.63%  : +0.0199% : _ZN8Compiler22fgValueNumberTreeConstEP7GenTree                                                                                                          
47611332   : NA           : 0.59%  : +0.0185% : _ZN13ValueNumStore22VNPairForFuncNoFoldingE9var_types6VNFunc12ValueNumPairS2_                                                                           
35476925   : NA           : 0.44%  : +0.0138% : _ZN13ValueNumStore13VNPairForFuncE9var_types6VNFunc12ValueNumPairS2_S2_                                                                                 
31522463   : +11.02%      : 0.39%  : +0.0123% : _ZN8Compiler42fgValueNumberAddExceptionSetForIndirectionEP7GenTreeS1_                                                                                   
28422024   : NA           : 0.35%  : +0.0111% : _ZN3LIR5RangeC2EP7GenTreeS2_                                                                                                                            
28312303   : NA           : 0.35%  : +0.0110% : _ZN16CodeGenInterface18VariableLiveKeeperC2EjjP8Compiler13CompAllocator                                                                                 
26201006   : +2.03%       : 0.32%  : +0.0102% : _ZN8Compiler17fgValueNumberTreeEP7GenTree                                                                                                               
25245669   : NA           : 0.31%  : +0.0098% : _ZN12JitHashTableIN13ValueNumStore12VNDefFuncAppILm4EEENS0_20VNDefFuncAppKeyFuncsILm4EEEj13CompAllocator20JitHashTableBehaviorE18LookupPointerOrAddES2_j
21469503   : +4.83%       : 0.26%  : +0.0084% : _ZN12JitHashTableIN13ValueNumStore12VNDefFuncAppILm2EEENS0_20VNDefFuncAppKeyFuncsILm2EEEj13CompAllocator20JitHashTableBehaviorE10ReallocateEj           
17544284   : NA           : 0.22%  : +0.0068% : _ZN13ValueNumStore13VNForMapStoreEjjj                                                                                                                   
14430528   : NA           : 0.18%  : +0.0056% : _ZN13ValueNumStoreC2EP8Compiler13CompAllocator                                                                                                          
14062434   : +80.42%      : 0.17%  : +0.0055% : _ZN8Compiler29fgValueNumberByrefExposedLoadE9var_typesj                                                                                                 
12946834   : +817.34%     : 0.16%  : +0.0050% : _ZN13ValueNumStore18VNPExcSetSingletonE12ValueNumPair                                                                                                   
8629427    : +3.28%       : 0.11%  : +0.0034% : _ZN8Compiler18fgValueNumberStoreEP7GenTree                                                                                                              
-10510003  : -2.45%       : 0.13%  : -0.0041% : _ZN13ValueNumStore18VNForMapSelectWorkE12ValueNumKind9var_typesjjPiPbR16SmallValueNumSet                                                                
-14430528  : -100.00%     : 0.18%  : -0.0056% : _ZN13ValueNumStoreC1EP8Compiler13CompAllocator                                                                                                          
-22723263  : -100.00%     : 0.28%  : -0.0088% : _ZN12JitHashTableIN13ValueNumStore12VNDefFuncAppILm4EEENS0_20VNDefFuncAppKeyFuncsILm4EEEj13CompAllocator20JitHashTableBehaviorE3SetES2_jNS7_7SetKindE   
-28312789  : -100.00%     : 0.35%  : -0.0110% : _ZN16CodeGenInterface18VariableLiveKeeperC1EjjP8Compiler13CompAllocator                                                                                 
-28422024  : -100.00%     : 0.35%  : -0.0111% : _ZN3LIR5RangeC1EP7GenTreeS2_                                                                                                                            
-30924736  : -90.03%      : 0.38%  : -0.0120% : _ZN13ValueNumStore9VNForFuncE9var_types6VNFuncjjjj                                                                                                      
-34550133  : -100.00%     : 0.43%  : -0.0134% : _ZN12JitHashTableIl25JitLargePrimitiveKeyFuncsIlEj13CompAllocator20JitHashTableBehaviorE3SetEljNS4_7SetKindE                                            
-50788777  : -100.00%     : 0.63%  : -0.0198% : _ZN16ValueNumberState22IsReachableThroughPredEP10BasicBlockS1_                                                                                          
-53158679  : -100.00%     : 0.66%  : -0.0207% : _ZN12JitHashTableIN13ValueNumStore12VNDefFuncAppILm3EEENS0_20VNDefFuncAppKeyFuncsILm3EEEj13CompAllocator20JitHashTableBehaviorE3SetES2_jNS7_7SetKindE   
-59680687  : -100.00%     : 0.74%  : -0.0232% : _ZN13ValueNumStore5ChunkC1E13CompAllocatorPj9var_typesNS_17ChunkExtraAttribsE                                                                           
-60942636  : -100.00%     : 0.75%  : -0.0237% : _ZN12JitHashTableIN13ValueNumStore8VNHandleES1_j13CompAllocator20JitHashTableBehaviorE3SetES1_jNS4_7SetKindE                                            
-64767679  : -25.22%      : 0.80%  : -0.0252% : _ZN13ValueNumStore9VNForFuncE9var_types6VNFuncj                                                                                                         
-67205348  : -8.59%       : 0.83%  : -0.0262% : _ZN8Compiler18fgValueNumberBlockEP10BasicBlock                                                                                                          
-71517680  : -99.98%      : 0.88%  : -0.0278% : _ZN13ValueNumStore9VNForFuncE9var_types6VNFuncjjj                                                                                                       
-85944920  : -100.00%     : 1.06%  : -0.0334% : _ZN12JitHashTableIN13ValueNumStore12VNDefFuncAppILm1EEENS0_20VNDefFuncAppKeyFuncsILm1EEEj13CompAllocator20JitHashTableBehaviorE3SetES2_jNS7_7SetKindE   
-106484259 : -97.57%      : 1.31%  : -0.0414% : _ZN13ValueNumStore11VNForHandleEl12GenTreeFlags                                                                                                         
-120756571 : -96.65%      : 1.49%  : -0.0470% : _ZN13ValueNumStore12VNForLongConEl                                                                                                                      
-125665414 : -100.00%     : 1.55%  : -0.0489% : _ZN3LIR3UseC1ERNS_5RangeEPP7GenTreeS4_                                                                                                                  
-148086939 : -100.00%     : 1.83%  : -0.0576% : _ZN13ValueNumStore10VnForConstIiNS_5VNMapIi25JitLargePrimitiveKeyFuncsIiEEEEEjT_PT0_9var_types                                                          
-316274965 : -65.25%      : 3.90%  : -0.1231% : _ZN13ValueNumStore10VNPWithExcE12ValueNumPairS0_                                                                                                        
-582693244 : -25.17%      : 7.18%  : -0.2267% : _ZN13ValueNumStore9VNForFuncE9var_types6VNFuncjj                                                                                                        
-691093791 : -100.00%     : 8.52%  : -0.2689% : _ZN13ValueNumStore18VNForFuncNoFoldingE9var_types6VNFuncjj                                                                                              
-744239870 : -100.00%     : 9.18%  : -0.2896% : _ZN12JitHashTableIN13ValueNumStore12VNDefFuncAppILm2EEENS0_20VNDefFuncAppKeyFuncsILm2EEEj13CompAllocator20JitHashTableBehaviorE3SetES2_jNS7_7SetKindE   

This just looks like missed inlining, probably related to link-time codegen not being enabled in our TP runs.

@jakobbotsch
Copy link
Member Author

cc @dotnet/jit-contrib PTAL @EgorBo

No diffs. Some (small-ish) TP improvements. Some TP regressions on Linux that I posted detailed stats of above.

@jakobbotsch jakobbotsch marked this pull request as ready for review July 15, 2024 18:48
@jakobbotsch jakobbotsch requested a review from EgorBo July 15, 2024 18:48
Copy link
Member

@EgorBo EgorBo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am wondering - is it possible to post "Detailed tp diff" on CI? could be on-demand

@jakobbotsch jakobbotsch merged commit d710379 into dotnet:main Jul 16, 2024
107 checks passed
@jakobbotsch jakobbotsch deleted the vn-func-no-duplicate-lookups branch July 16, 2024 16:41
@jakobbotsch
Copy link
Member Author

I am wondering - is it possible to post "Detailed tp diff" on CI? could be on-demand

Sure, it would be nice to have something like that. Ideally we would add support for parallelism and automatically filtering out TP from misses if we were to add it into CI.

@github-actions github-actions bot locked and limited conversation to collaborators Aug 16, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-CodeGen-coreclr CLR 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.

2 participants