Skip to content

Fix morph issue with fgOptimizeEqualityComparisonWithConst #78187

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
Merged
davidwrighton merged 2 commits into from
Nov 11, 2022
Merged

Fix morph issue with fgOptimizeEqualityComparisonWithConst #78187

davidwrighton 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.
@ghost ghost 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: -

SingleAccretion
SingleAccretion reviewed Nov 10, 2022
View reviewed changes
@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

@jakobbotsch jakobbotsch jakobbotsch approved these changes

+1 more reviewer

@SingleAccretion SingleAccretion SingleAccretion left review comments

Reviewers whose approvals may not affect merge requirements

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