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: Expand handling of non-value commas in gtSplitTree #83725

Merged
merged 1 commit into from
Mar 21, 2023

Conversation

jakobbotsch
Copy link
Member

We can have a 'value'-typed COMMA whose RHS operand is actually an assignment. gtSplitTree would create illegal IR for this case.

For example:

N018 ( 38, 33) [000327] -ACXGO----- arg1 setup              ├──▌  COMMA     simd32
N011 ( 28, 25) [000328] -ACXGO-----                         │  ├──▌  COMMA     void
N008 ( 24, 22) [000320] -ACXG---R--                         │  │  ├──▌  ASG       byref
N007 (  3,  2) [000319] D------N---                         │  │  │  ├──▌  LCL_VAR   byref  V50 tmp45
N006 ( 20, 19) [000126] --CXG------                         │  │  │  └──▌  CALL help byref  CORINFO_HELP_UNBOX
N004 (  3,  2) [000124] ----------- arg1 in rsi             │  │  │     ├──▌  LCL_VAR   ref    V02 loc1
N005 (  3, 10) [000125] H---------- arg0 in rdi             │  │  │     └──▌  CNS_INT(h) long   0x7f6e4bb0e500 class
N010 (  4,  3) [000322] ---X-O-----                         │  │  └──▌  NULLCHECK byte
N009 (  3,  2) [000321] -----------                         │  │     └──▌  LCL_VAR   byref  V50 tmp45
N017 ( 10,  8) [000331] -A-XGO--R--                         │  └──▌  ASG       simd32 (copy)
N016 (  3,  2) [000330] D------N---                         │     ├──▌  LCL_VAR   simd32 V51 tmp46
N015 (  6,  5) [000329] ---XGO-N---                         │     └──▌  IND       simd32
N014 (  4,  3) [000326] -----O-N---                         │        └──▌  ADD       byref
N012 (  3,  2) [000324] -----O-----                         │           ├──▌  LCL_VAR   byref  V50 tmp45
N013 (  1,  1) [000325] -----------                         │           └──▌  CNS_INT   long   32

gtSplitTree would believe that [000327] was a value and would create the following IR shape:

N016 ( 14, 11) [000428] -A-XGO--R--ASG       simd32 (copy)
N015 (  3,  2) [000427] D------N---                         ├──▌  LCL_VAR   simd32 V76 tmp71
N014 ( 10,  8) [000331] -A-XGO-NR--                         └──▌  ASG       simd32 (copy)
N013 (  3,  2) [000330] D------N---                            ├──▌  LCL_VAR   simd32 V51 tmp46
N012 (  6,  5) [000329] ---XGO-N---                            └──▌  IND       simd32
N011 (  4,  3) [000326] -----O-N---                               └──▌  ADD       byref
N009 (  3,  2) [000324] -----O-----                                  ├──▌  LCL_VAR   byref  V50 tmp45
N010 (  1,  1) [000325] -----------                                  └──▌  CNS_INT   long   32

Fix #83576

We can have a 'value'-typed COMMA whose RHS operand is actually an
assignment. gtSplitTree would create illegal IR for this case.

For example:

N018 ( 38, 33) [000327] -ACXGO----- arg1 setup              ├──▌  COMMA     simd32
N011 ( 28, 25) [000328] -ACXGO-----                         │  ├──▌  COMMA     void
N008 ( 24, 22) [000320] -ACXG---R--                         │  │  ├──▌  ASG       byref
N007 (  3,  2) [000319] D------N---                         │  │  │  ├──▌  LCL_VAR   byref  V50 tmp45
N006 ( 20, 19) [000126] --CXG------                         │  │  │  └──▌  CALL help byref  CORINFO_HELP_UNBOX
N004 (  3,  2) [000124] ----------- arg1 in rsi             │  │  │     ├──▌  LCL_VAR   ref    V02 loc1
N005 (  3, 10) [000125] H---------- arg0 in rdi             │  │  │     └──▌  CNS_INT(h) long   0x7f6e4bb0e500 class
N010 (  4,  3) [000322] ---X-O-----                         │  │  └──▌  NULLCHECK byte
N009 (  3,  2) [000321] -----------                         │  │     └──▌  LCL_VAR   byref  V50 tmp45
N017 ( 10,  8) [000331] -A-XGO--R--                         │  └──▌  ASG       simd32 (copy)
N016 (  3,  2) [000330] D------N---                         │     ├──▌  LCL_VAR   simd32 V51 tmp46
N015 (  6,  5) [000329] ---XGO-N---                         │     └──▌  IND       simd32
N014 (  4,  3) [000326] -----O-N---                         │        └──▌  ADD       byref
N012 (  3,  2) [000324] -----O-----                         │           ├──▌  LCL_VAR   byref  V50 tmp45
N013 (  1,  1) [000325] -----------                         │           └──▌  CNS_INT   long   32

gtSplitTree would believe that [000327] was a value and would create the
following IR shape:

N016 ( 14, 11) [000428] -A-XGO--R--                         ▌  ASG       simd32 (copy)
N015 (  3,  2) [000427] D------N---                         ├──▌  LCL_VAR   simd32 V76 tmp71
N014 ( 10,  8) [000331] -A-XGO-NR--                         └──▌  ASG       simd32 (copy)
N013 (  3,  2) [000330] D------N---                            ├──▌  LCL_VAR   simd32 V51 tmp46
N012 (  6,  5) [000329] ---XGO-N---                            └──▌  IND       simd32
N011 (  4,  3) [000326] -----O-N---                               └──▌  ADD       byref
N009 (  3,  2) [000324] -----O-----                                  ├──▌  LCL_VAR   byref  V50 tmp45
N010 (  1,  1) [000325] -----------                                  └──▌  CNS_INT   long   32

Fix dotnet#83576
@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 Mar 21, 2023
@ghost ghost assigned jakobbotsch Mar 21, 2023
@ghost
Copy link

ghost commented Mar 21, 2023

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

Issue Details

We can have a 'value'-typed COMMA whose RHS operand is actually an assignment. gtSplitTree would create illegal IR for this case.

For example:

N018 ( 38, 33) [000327] -ACXGO----- arg1 setup              ├──▌  COMMA     simd32
N011 ( 28, 25) [000328] -ACXGO-----                         │  ├──▌  COMMA     void
N008 ( 24, 22) [000320] -ACXG---R--                         │  │  ├──▌  ASG       byref
N007 (  3,  2) [000319] D------N---                         │  │  │  ├──▌  LCL_VAR   byref  V50 tmp45
N006 ( 20, 19) [000126] --CXG------                         │  │  │  └──▌  CALL help byref  CORINFO_HELP_UNBOX
N004 (  3,  2) [000124] ----------- arg1 in rsi             │  │  │     ├──▌  LCL_VAR   ref    V02 loc1
N005 (  3, 10) [000125] H---------- arg0 in rdi             │  │  │     └──▌  CNS_INT(h) long   0x7f6e4bb0e500 class
N010 (  4,  3) [000322] ---X-O-----                         │  │  └──▌  NULLCHECK byte
N009 (  3,  2) [000321] -----------                         │  │     └──▌  LCL_VAR   byref  V50 tmp45
N017 ( 10,  8) [000331] -A-XGO--R--                         │  └──▌  ASG       simd32 (copy)
N016 (  3,  2) [000330] D------N---                         │     ├──▌  LCL_VAR   simd32 V51 tmp46
N015 (  6,  5) [000329] ---XGO-N---                         │     └──▌  IND       simd32
N014 (  4,  3) [000326] -----O-N---                         │        └──▌  ADD       byref
N012 (  3,  2) [000324] -----O-----                         │           ├──▌  LCL_VAR   byref  V50 tmp45
N013 (  1,  1) [000325] -----------                         │           └──▌  CNS_INT   long   32

gtSplitTree would believe that [000327] was a value and would create the following IR shape:

N016 ( 14, 11) [000428] -A-XGO--R--ASG       simd32 (copy)
N015 (  3,  2) [000427] D------N---                         ├──▌  LCL_VAR   simd32 V76 tmp71
N014 ( 10,  8) [000331] -A-XGO-NR--                         └──▌  ASG       simd32 (copy)
N013 (  3,  2) [000330] D------N---                            ├──▌  LCL_VAR   simd32 V51 tmp46
N012 (  6,  5) [000329] ---XGO-N---                            └──▌  IND       simd32
N011 (  4,  3) [000326] -----O-N---                               └──▌  ADD       byref
N009 (  3,  2) [000324] -----O-----                                  ├──▌  LCL_VAR   byref  V50 tmp45
N010 (  1,  1) [000325] -----------                                  └──▌  CNS_INT   long   32

Fix #83576

Author: jakobbotsch
Assignees: -
Labels:

area-CodeGen-coreclr

Milestone: -

@jakobbotsch jakobbotsch requested a review from EgorBo March 21, 2023 13:23
@EgorBo
Copy link
Member

EgorBo commented Mar 21, 2023

@jakobbotsch does the assert come from gtSplitTree stress pass or runtime lookup expansion?

@jakobbotsch
Copy link
Member Author

@EgorBo From the stress mode.

@jakobbotsch
Copy link
Member Author

/azp run runtime-coreclr jitstress

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@jakobbotsch jakobbotsch merged commit 6439980 into dotnet:main Mar 21, 2023
@jakobbotsch jakobbotsch deleted the fix-83576 branch March 21, 2023 18:22
@ghost ghost locked as resolved and limited conversation to collaborators Apr 20, 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.

JIT: Assertion failed 'tree->IsCall() || tree->OperIs(GT_COMMA)' during 'Stress gtSplitTree'
2 participants