Skip to content

Conversation

@jakobbotsch
Copy link
Member

gtGetClassHandle has a comment about "Tunnel through commas", yet several of the cases then do not actually use the effective value, resulting in possible asserts when this function is passed a comma.

Fixes an issue I saw in #87265 when morph invokes gtFoldTypeCompare for the following IR:

fgMorphTree BB19, STMT00013 (before)
               [000045] -ACXG------                         *  JTRUE     void
               [000044] -ACXG------                         \--*  NE        int
               [000623] -ACXG------                            +--*  COMMA     ref
               [000622] DA---------                            |  +--*  STORE_LCL_VAR ref    V29 tmp18
               [000621] -----------                            |  |  \--*  LCL_FLD   ref    V07 loc3         [+0]
               [000041] -ACXG------                            |  \--*  CALL nullcheck ref    Microsoft.Extensions.Configuration.Test.ConfigurationProviderTestBase+TestKeyValue:get_AsArray():System.String[]:this
               [000620] -A--------- this                       |     \--*  COMMA     ref
               [000619] DA---------                            |        +--*  STORE_LCL_VAR ref    V30 tmp19
               [000618] -----------                            |        |  \--*  LCL_FLD   ref    V07 loc3         [+8]
               [000617] -----------                            |        \--*  LCL_VAR   ref    V30 tmp19
               [000043] -----------                            \--*  CNS_INT   ref    null

gtGetClassHandle has comments about "Tunnel through commas", yet several
of the cases then do not actually use the effective value, resulting in
possible asserts when this function is passed a comma.

Fixes an issue I saw in dotnet#87265 when morph invokes gtFoldTypeCompare for
the following IR:

```
fgMorphTree BB19, STMT00013 (before)
               [000045] -ACXG------                         *  JTRUE     void
               [000044] -ACXG------                         \--*  NE        int
               [000623] -ACXG------                            +--*  COMMA     ref
               [000622] DA---------                            |  +--*  STORE_LCL_VAR ref    V29 tmp18
               [000621] -----------                            |  |  \--*  LCL_FLD   ref    V07 loc3         [+0]
               [000041] -ACXG------                            |  \--*  CALL nullcheck ref    Microsoft.Extensions.Configuration.Test.ConfigurationProviderTestBase+TestKeyValue:get_AsArray():System.String[]:this
               [000620] -A--------- this                       |     \--*  COMMA     ref
               [000619] DA---------                            |        +--*  STORE_LCL_VAR ref    V30 tmp19
               [000618] -----------                            |        |  \--*  LCL_FLD   ref    V07 loc3         [+8]
               [000617] -----------                            |        \--*  LCL_VAR   ref    V30 tmp19
               [000043] -----------                            \--*  CNS_INT   ref    null
```
@ghost ghost assigned jakobbotsch Jun 9, 2023
@ghost ghost added the area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI label Jun 9, 2023
@ghost
Copy link

ghost commented Jun 9, 2023

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

Issue Details

gtGetClassHandle has a comment about "Tunnel through commas", yet several of the cases then do not actually use the effective value, resulting in possible asserts when this function is passed a comma.

Fixes an issue I saw in #87265 when morph invokes gtFoldTypeCompare for the following IR:

fgMorphTree BB19, STMT00013 (before)
               [000045] -ACXG------                         *  JTRUE     void
               [000044] -ACXG------                         \--*  NE        int
               [000623] -ACXG------                            +--*  COMMA     ref
               [000622] DA---------                            |  +--*  STORE_LCL_VAR ref    V29 tmp18
               [000621] -----------                            |  |  \--*  LCL_FLD   ref    V07 loc3         [+0]
               [000041] -ACXG------                            |  \--*  CALL nullcheck ref    Microsoft.Extensions.Configuration.Test.ConfigurationProviderTestBase+TestKeyValue:get_AsArray():System.String[]:this
               [000620] -A--------- this                       |     \--*  COMMA     ref
               [000619] DA---------                            |        +--*  STORE_LCL_VAR ref    V30 tmp19
               [000618] -----------                            |        |  \--*  LCL_FLD   ref    V07 loc3         [+8]
               [000617] -----------                            |        \--*  LCL_VAR   ref    V30 tmp19
               [000043] -----------                            \--*  CNS_INT   ref    null
Author: jakobbotsch
Assignees: jakobbotsch
Labels:

area-CodeGen-coreclr

Milestone: -

@jakobbotsch
Copy link
Member Author

cc @dotnet/jit-contrib PTAL @EgorBo

@jakobbotsch jakobbotsch requested a review from EgorBo June 9, 2023 16:45
Copy link
Member

@AndyAyersMS AndyAyersMS left a comment

Choose a reason for hiding this comment

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

Thanks for spotting this.

@jakobbotsch jakobbotsch merged commit 79fda00 into dotnet:main Jun 9, 2023
@jakobbotsch jakobbotsch deleted the fix-commas-in-gtGetClassHandle branch June 9, 2023 17:57
@ghost ghost locked as resolved and limited conversation to collaborators Jul 9, 2023
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