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.

Sign up for GitHub

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

Jump to bottom

Fix morph issue with fgOptimizeEqualityComparisonWithConst #78187

Merged
merged 2 commits into from
Nov 11, 2022
Merged

Fix morph issue with fgOptimizeEqualityComparisonWithConst #78187

merged 2 commits into from
Nov 11, 2022

Conversation

davidwrighton
Copy link
Member

@davidwrighton davidwrighton commented Nov 10, 2022 •
edited
Loading

  • On arm32 a shift of a long requires a helper call, but the optimization in fgOptimizeEqualityComparisonWithConst for converting a right shift into a left shift was not keeping the GTF_CALL flag on the GenTree node.

This fix unblocks #74886

The issue manifests in an assert

Assertion failed '!"Missing flags on tree"' in 'JIT.HardwareIntrinsics.Arm._ArmBase.Arm64.ScalarUnaryOpTest__LeadingZeroCount_Int64:ValidateResult(long,int,System.String):this' during 'Morph - Global' (IL size 175; hash 0x7f7fb7e2; FullOpts)

Missing flags on tree [000013]: -C---------
               [000013] -----+------                        *  LSH       long  
               [000015] -----+------                        +--*  CNS_LNG   long   0x0000000000000001
               [000012] -----+------                        \--*  AND       int   
               [000010] -----+------                           +--*  LCL_VAR   int    V06 loc2         
               [000011] -----+------                           \--*  CNS_INT   int    63

The effect on the morphed tree is instead of generating

Morphing BB03 of 'JIT.HardwareIntrinsics.Arm._ArmBase.Arm64.ScalarUnaryOpTest__LeadingZeroCount_Int64:ValidateResult(long,int,System.String):this'

fgMorphTree BB03, STMT00004 (before)
               [000025] --C---------                        *  JTRUE     void  
               [000024] --C---------                        \--*  NE        int   
               [000019] --C---------                           +--*  EQ        int   
               [000016] --C---------                           |  +--*  AND       long  
               [000013] --C---------                           |  |  +--*  RSZ       long  
               [000009] ------------                           |  |  |  +--*  LCL_VAR   long   V01 arg1         
               [000012] ------------                           |  |  |  \--*  AND       int   
               [000010] ------------                           |  |  |     +--*  LCL_VAR   int    V06 loc2         
               [000011] ------------                           |  |  |     \--*  CNS_INT   int    63
               [000015] ------------                           |  |  \--*  CNS_LNG   long   0x0000000000000001
               [000018] ------------                           |  \--*  CNS_LNG   long   0x0000000000000000
               [000023] ------------                           \--*  CNS_INT   int    0

fgMorphTree BB03, STMT00004 (after)
               [000025] --C--+------                        *  JTRUE     void  
               [000019] J-C--+-N--S-                        \--*  EQ        int   
               [000016] --C--+----S-                           +--*  AND       long  
               [000009] -----+------                           |  +--*  LCL_VAR   long   V01 arg1         
*               [000013] -----+------                           |  \--*  LSH       long  
               [000015] -----+------                           |     +--*  CNS_LNG   long   0x0000000000000001
               [000012] -----+------                           |     \--*  AND       int   
               [000010] -----+------                           |        +--*  LCL_VAR   int    V06 loc2         
               [000011] -----+------                           |        \--*  CNS_INT   int    63
               [000018] -----+------                           \--*  CNS_LNG   long   0x0000000000000000

With the fix

Morphing BB03 of 'JIT.HardwareIntrinsics.Arm._ArmBase.Arm64.ScalarUnaryOpTest__LeadingZeroCount_Int64:ValidateResult(long,int,System.String):this'

fgMorphTree BB03, STMT00004 (before)
               [000025] --C---------                        *  JTRUE     void  
               [000024] --C---------                        \--*  NE        int   
               [000019] --C---------                           +--*  EQ        int   
               [000016] --C---------                           |  +--*  AND       long  
               [000013] --C---------                           |  |  +--*  RSZ       long  
               [000009] ------------                           |  |  |  +--*  LCL_VAR   long   V01 arg1         
               [000012] ------------                           |  |  |  \--*  AND       int   
               [000010] ------------                           |  |  |     +--*  LCL_VAR   int    V06 loc2         
               [000011] ------------                           |  |  |     \--*  CNS_INT   int    63
               [000015] ------------                           |  |  \--*  CNS_LNG   long   0x0000000000000001
               [000018] ------------                           |  \--*  CNS_LNG   long   0x0000000000000000
               [000023] ------------                           \--*  CNS_INT   int    0

fgMorphTree BB03, STMT00004 (after)
               [000025] --C--+------                        *  JTRUE     void  
               [000019] J-C--+-N--S-                        \--*  EQ        int   
               [000016] --C--+----S-                           +--*  AND       long  
               [000009] -----+------                           |  +--*  LCL_VAR   long   V01 arg1         
*               [000013] --C--+------                           |  \--*  LSH       long  
               [000015] -----+------                           |     +--*  CNS_LNG   long   0x0000000000000001
               [000012] -----+------                           |     \--*  AND       int   
               [000010] -----+------                           |        +--*  LCL_VAR   int    V06 loc2         
               [000011] -----+------                           |        \--*  CNS_INT   int    63
               [000018] -----+------                           \--*  CNS_LNG   long   0x0000000000000000

@davidwrighton
Fix morph issue with fgOptimizeEqualityComparisonWithConst
a22d3f9 
- On arm32 a shift of a long requires a helper call, but the optimization in fgOptimizeEqualityComparisonWithConst for converting a right shift into a left shift was not keeping the GTF_CALL flag on the GenTree node.
@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 Nov 10, 2022
@ghost ghost assigned davidwrighton Nov 10, 2022
@ghost
Copy link

ghost commented Nov 10, 2022

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

Issue Details
  • On arm32 a shift of a long requires a helper call, but the optimization in fgOptimizeEqualityComparisonWithConst for converting a right shift into a left shift was not keeping the GTF_CALL flag on the GenTree node.

This fix unblocks #74886

The effect on the morphed tree is instead of generating

Morphing BB03 of 'JIT.HardwareIntrinsics.Arm._ArmBase.Arm64.ScalarUnaryOpTest__LeadingZeroCount_Int64:ValidateResult(long,int,System.String):this'

fgMorphTree BB03, STMT00004 (before)
               [000025] --C---------                        *  JTRUE     void  
               [000024] --C---------                        \--*  NE        int   
               [000019] --C---------                           +--*  EQ        int   
               [000016] --C---------                           |  +--*  AND       long  
               [000013] --C---------                           |  |  +--*  RSZ       long  
               [000009] ------------                           |  |  |  +--*  LCL_VAR   long   V01 arg1         
               [000012] ------------                           |  |  |  \--*  AND       int   
               [000010] ------------                           |  |  |     +--*  LCL_VAR   int    V06 loc2         
               [000011] ------------                           |  |  |     \--*  CNS_INT   int    63
               [000015] ------------                           |  |  \--*  CNS_LNG   long   0x0000000000000001
               [000018] ------------                           |  \--*  CNS_LNG   long   0x0000000000000000
               [000023] ------------                           \--*  CNS_INT   int    0

fgMorphTree BB03, STMT00004 (after)
               [000025] --C--+------                        *  JTRUE     void  
               [000019] J-C--+-N--S-                        \--*  EQ        int   
               [000016] --C--+----S-                           +--*  AND       long  
               [000009] -----+------                           |  +--*  LCL_VAR   long   V01 arg1         
*               [000013] -----+------                           |  \--*  LSH       long  
               [000015] -----+------                           |     +--*  CNS_LNG   long   0x0000000000000001
               [000012] -----+------                           |     \--*  AND       int   
               [000010] -----+------                           |        +--*  LCL_VAR   int    V06 loc2         
               [000011] -----+------                           |        \--*  CNS_INT   int    63
               [000018] -----+------                           \--*  CNS_LNG   long   0x0000000000000000

With the fix

Morphing BB03 of 'JIT.HardwareIntrinsics.Arm._ArmBase.Arm64.ScalarUnaryOpTest__LeadingZeroCount_Int64:ValidateResult(long,int,System.String):this'

fgMorphTree BB03, STMT00004 (before)
               [000025] --C---------                        *  JTRUE     void  
               [000024] --C---------                        \--*  NE        int   
               [000019] --C---------                           +--*  EQ        int   
               [000016] --C---------                           |  +--*  AND       long  
               [000013] --C---------                           |  |  +--*  RSZ       long  
               [000009] ------------                           |  |  |  +--*  LCL_VAR   long   V01 arg1         
               [000012] ------------                           |  |  |  \--*  AND       int   
               [000010] ------------                           |  |  |     +--*  LCL_VAR   int    V06 loc2         
               [000011] ------------                           |  |  |     \--*  CNS_INT   int    63
               [000015] ------------                           |  |  \--*  CNS_LNG   long   0x0000000000000001
               [000018] ------------                           |  \--*  CNS_LNG   long   0x0000000000000000
               [000023] ------------                           \--*  CNS_INT   int    0

fgMorphTree BB03, STMT00004 (after)
               [000025] --C--+------                        *  JTRUE     void  
               [000019] J-C--+-N--S-                        \--*  EQ        int   
               [000016] --C--+----S-                           +--*  AND       long  
               [000009] -----+------                           |  +--*  LCL_VAR   long   V01 arg1         
*               [000013] --C--+------                           |  \--*  LSH       long  
               [000015] -----+------                           |     +--*  CNS_LNG   long   0x0000000000000001
               [000012] -----+------                           |     \--*  AND       int   
               [000010] -----+------                           |        +--*  LCL_VAR   int    V06 loc2         
               [000011] -----+------                           |        \--*  CNS_INT   int    63
               [000018] -----+------                           \--*  CNS_LNG   long   0x0000000000000000
Author: davidwrighton
Assignees: davidwrighton
Labels:

area-CodeGen-coreclr
Milestone: -

@davidwrighton @SingleAccretion
Update src/coreclr/jit/morph.cpp
db6fa46 
Co-authored-by: SingleAccretion <62474226+SingleAccretion@users.noreply.github.com>
@davidwrighton davidwrighton merged commit 7db5a57 into dotnet:main Nov 11, 2022
@ghost ghost locked as resolved and limited conversation to collaborators Dec 11, 2022
@davidwrighton davidwrighton deleted the fix_arm32_morph_opt branch April 13, 2023 18:54
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@SingleAccretion SingleAccretion SingleAccretion left review comments

@jakobbotsch jakobbotsch jakobbotsch approved these changes

Assignees

@davidwrighton davidwrighton

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.
3 participants
@davidwrighton @jakobbotsch @SingleAccretion