Skip to content

JIT: Avoid duplicate hash table lookups in VN#104873

Merged
jakobbotsch merged 2 commits intodotnet:mainfrom
jakobbotsch:vn-func-no-duplicate-lookups
Jul 16, 2024
Merged

JIT: Avoid duplicate hash table lookups in VN#104873
jakobbotsch merged 2 commits intodotnet:mainfrom
jakobbotsch:vn-func-no-duplicate-lookups

Conversation

@jakobbotsch
Copy link
Member

@jakobbotsch jakobbotsch commented Jul 14, 2024

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

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

dotnet-policy-service bot commented Jul 14, 2024

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

@build-analysis build-analysis bot mentioned this pull request Jul 14, 2024
Closed
3 tasks
@jakobbotsch jakobbotsch mentioned this pull request Jul 15, 2024
Closed
@jakobbotsch
Copy link
Member Author

jakobbotsch commented Jul 15, 2024

/azp run runtime-coreclr superpmi-replay

@azure-pipelines
Copy link

azure-pipelines bot commented Jul 15, 2024

Azure Pipelines successfully started running 1 pipeline(s).

@jakobbotsch
Copy link
Member Author

jakobbotsch commented Jul 15, 2024

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

jakobbotsch commented Jul 15, 2024

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
EgorBo
EgorBo approved these changes Jul 16, 2024
View reviewed changes
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
@jakobbotsch jakobbotsch deleted the vn-func-no-duplicate-lookups branch July 16, 2024 16:41
@jakobbotsch
Copy link
Member Author

jakobbotsch commented Jul 16, 2024

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.

Reviewers

@EgorBo EgorBo EgorBo approved these changes

Assignees

@jakobbotsch jakobbotsch

Labels

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

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@jakobbotsch @EgorBo