JIT: Refactor gtExtractSideEffList and fix GTF_DEBUG_NODE_MORPHED propagation #79611
Conversation
Inline gtBuildCommaList (this is the only usage) and build it in the right order to allow doing it during the walk. Also fix propagation of GTF_DEBUG_NODE_MORPHED flag. Fix dotnet#79543
dotnet-issue-labeler
bot
added
the
area-CodeGen-coreclr
ghost
assigned 
|
Tagging subscribers to this area: @JulieLeeMSFT, @jakobbotsch Issue DetailsInline gtBuildCommaList (this is the only usage) and build it in the right order to allow doing it during the walk. Also fix propagation of GTF_DEBUG_NODE_MORPHED flag: only propagate it to the parent if the operands already have been morphed. I need the refactoring to be able to add support for QMARKs in #79346. Fix #79543
|
|
/azp run Fuzzlyn |
|
Azure Pipelines successfully started running 1 pipeline(s). |
|
Looks like the .zip file from Fuzzlyn did not make it properly to the artifacts, not sure why. I checked the logs manually and there were no errors. Will run some stress. |
|
/azp run runtime-coreclr jitstress, runtime-coreclr libraries-jitstress |
|
Azure Pipelines successfully started running 2 pipeline(s). |
|
cc @dotnet/jit-contrib PTAL @TIHan |
| // Either both should be morphed or neither should be. | ||
| assert((m_result->gtDebugFlags & GTF_DEBUG_NODE_MORPHED) == | ||
| (node->gtDebugFlags & GTF_DEBUG_NODE_MORPHED)); | ||
| comma->gtDebugFlags |= node->gtDebugFlags & GTF_DEBUG_NODE_MORPHED; |
There was a problem hiding this comment.
This is the fix for #79543
|
I need to look at the diff, I expect it is because we are creating a |
Is it possible to rewrite it to be zero diff? (i.e., keep the right-leaning tree)? (and would there be a benefit?) |
Yes it would be possible to create the right-leaning tree instead, with some added complexity. I don't think there's any benefit really.
Yeah, the diff looks inconsequential, I just want to double check the reason why it's there. |
|
One of the diffs: we now produce N030 ( 36, 56) [000548] -A-XGO----- ├──▌ COMMA void <l:$15c, c:$4c4>
N025 ( 32, 52) [000547] -A-XGO----- │ ├──▌ COMMA void <l:$15c, c:$4c4>
N020 ( 24, 41) [000546] -A-XGO----- │ │ ├──▌ COMMA void <l:$15c, c:$55e>
N012 ( 16, 26) [000545] -A-XGO----- │ │ │ ├──▌ COMMA void <l:$15c, c:$55a>
N007 ( 8, 15) [000485] -A-XG---R-- │ │ │ │ ├──▌ ASG ref <l:$14c, c:$4ad>
N006 ( 1, 1) [000484] D------N--- │ │ │ │ │ ├──▌ LCL_VAR ref V32 tmp28 d:2 $VN.Void
N005 ( 8, 15) [000085] ---XG------ │ │ │ │ │ └──▌ IND ref <l:$149, c:$4ac>
N004 ( 6, 13) [000500] ----G--N--- │ │ │ │ │ └──▌ ADD byref <l:$242, c:$252>
N002 ( 5, 12) [000084] n---G------ │ │ │ │ │ ├──▌ IND ref <l:$145, c:$121>
N001 ( 3, 10) [000083] H---------- │ │ │ │ │ │ └──▌ CNS_INT(h) long 0x20e34c06850 static Fseq[<unknown field>] $c4
N003 ( 1, 1) [000499] ----------- │ │ │ │ │ └──▌ CNS_INT long 8 Fseq[<unknown field>] $181
N011 ( 8, 11) [000490] ---X-O----- │ │ │ │ └──▌ BOUNDS_CHECK_Rng void <l:$158, c:$4b3>
N008 ( 1, 1) [000086] ----------- │ │ │ │ ├──▌ CNS_INT int 0 $41
N010 ( 3, 3) [000489] ---X------- │ │ │ │ └──▌ ARR_LENGTH int <l:$380, c:$390>
N009 ( 1, 1) [000486] ----------- │ │ │ │ └──▌ LCL_VAR ref V32 tmp28 u:2 <l:$146, c:$122>
N019 ( 8, 15) [000503] -A-XG---R-- │ │ │ └──▌ ASG ref <l:$14c, c:$4b7>
N018 ( 1, 1) [000502] D------N--- │ │ │ ├──▌ LCL_VAR ref V33 tmp29 d:2 $VN.Void
N017 ( 8, 15) [000091] ---XG------ │ │ │ └──▌ IND ref <l:$149, c:$4b6>
N016 ( 6, 13) [000518] ----G--N--- │ │ │ └──▌ ADD byref <l:$242, c:$254>
N014 ( 5, 12) [000090] n---G------ │ │ │ ├──▌ IND ref <l:$145, c:$125>
N013 ( 3, 10) [000089] H---------- │ │ │ │ └──▌ CNS_INT(h) long 0x20e34c06850 static Fseq[<unknown field>] $c4
N015 ( 1, 1) [000517] ----------- │ │ │ └──▌ CNS_INT long 8 Fseq[<unknown field>] $181
N024 ( 8, 11) [000508] ---X-O----- │ │ └──▌ BOUNDS_CHECK_Rng void <l:$158, c:$4bd>
N021 ( 1, 1) [000092] ----------- │ │ ├──▌ CNS_INT int 0 $41
N023 ( 3, 3) [000507] ---X------- │ │ └──▌ ARR_LENGTH int <l:$380, c:$391>
N022 ( 1, 1) [000504] ----------- │ │ └──▌ LCL_VAR ref V33 tmp29 u:2 <l:$146, c:$126>
N029 ( 4, 4) [000096] ---XG------ │ └──▌ IND long <l:$183, c:$40b>
N028 ( 2, 2) [000521] -------N--- │ └──▌ ADD byref $241
N026 ( 1, 1) [000095] ----------- │ ├──▌ LCL_VAR ref V00 loc0 u:2 $140
N027 ( 1, 1) [000520] ----------- │ └──▌ CNS_INT long 16 Fseq[<unknown field>] $182whereas before, we produced N030 ( 36, 56) [000548] -A-XGO----- ├──▌ COMMA void <l:$47a, c:$47c>
N007 ( 8, 15) [000485] -A-XG---R-- │ ├──▌ ASG ref <l:$14c, c:$4ad>
N006 ( 1, 1) [000484] D------N--- │ │ ├──▌ LCL_VAR ref V32 tmp28 d:2 $VN.Void
N005 ( 8, 15) [000085] ---XG------ │ │ └──▌ IND ref <l:$149, c:$4ac>
N004 ( 6, 13) [000500] ----G--N--- │ │ └──▌ ADD byref <l:$242, c:$252>
N002 ( 5, 12) [000084] n---G------ │ │ ├──▌ IND ref <l:$145, c:$121>
N001 ( 3, 10) [000083] H---------- │ │ │ └──▌ CNS_INT(h) long 0x20e34c06850 static Fseq[<unknown field>] $c4
N003 ( 1, 1) [000499] ----------- │ │ └──▌ CNS_INT long 8 Fseq[<unknown field>] $181
N029 ( 28, 41) [000547] -A-XGO----- │ └──▌ COMMA void <l:$47a, c:$47b>
N011 ( 8, 11) [000490] ---X-O----- │ ├──▌ BOUNDS_CHECK_Rng void <l:$158, c:$4b3>
N008 ( 1, 1) [000086] ----------- │ │ ├──▌ CNS_INT int 0 $41
N010 ( 3, 3) [000489] ---X------- │ │ └──▌ ARR_LENGTH int <l:$380, c:$390>
N009 ( 1, 1) [000486] ----------- │ │ └──▌ LCL_VAR ref V32 tmp28 u:2 <l:$146, c:$122>
N028 ( 20, 30) [000546] -A-XGO----- │ └──▌ COMMA void <l:$47a, c:$479>
N018 ( 8, 15) [000503] -A-XG---R-- │ ├──▌ ASG ref <l:$14c, c:$4b7>
N017 ( 1, 1) [000502] D------N--- │ │ ├──▌ LCL_VAR ref V33 tmp29 d:2 $VN.Void
N016 ( 8, 15) [000091] ---XG------ │ │ └──▌ IND ref <l:$149, c:$4b6>
N015 ( 6, 13) [000518] ----G--N--- │ │ └──▌ ADD byref <l:$242, c:$254>
N013 ( 5, 12) [000090] n---G------ │ │ ├──▌ IND ref <l:$145, c:$125>
N012 ( 3, 10) [000089] H---------- │ │ │ └──▌ CNS_INT(h) long 0x20e34c06850 static Fseq[<unknown field>] $c4
N014 ( 1, 1) [000517] ----------- │ │ └──▌ CNS_INT long 8 Fseq[<unknown field>] $181
N027 ( 12, 15) [000545] ---XGO----- │ └──▌ COMMA void <l:$478, c:$477>
N022 ( 8, 11) [000508] ---X-O----- │ ├──▌ BOUNDS_CHECK_Rng void <l:$158, c:$4bd>
N019 ( 1, 1) [000092] ----------- │ │ ├──▌ CNS_INT int 0 $41
N021 ( 3, 3) [000507] ---X------- │ │ └──▌ ARR_LENGTH int <l:$380, c:$391>
N020 ( 1, 1) [000504] ----------- │ │ └──▌ LCL_VAR ref V33 tmp29 u:2 <l:$146, c:$126>
N026 ( 4, 4) [000096] ---XG------ │ └──▌ IND long <l:$183, c:$40b>
N025 ( 2, 2) [000521] -------N--- │ └──▌ ADD byref $241
N023 ( 1, 1) [000095] ----------- │ ├──▌ LCL_VAR ref V00 loc0 u:2 $140
N024 ( 1, 1) [000520] ----------- │ └──▌ CNS_INT long 16 Fseq[<unknown field>] $182After we do some CSEs (the same in both trees) we end up with N016 ( 9, 12) [000551] ---XGO----- │ └──▌ COMMA void <l:$478, c:$47d>
N014 ( 8, 11) [000449] ---X-O----- │ ├──▌ BOUNDS_CHECK_Rng void <l:$158, c:$4d7>
N011 ( 1, 1) [000130] ----------- │ │ ├──▌ CNS_INT int 0 $41
N013 ( 3, 3) CSE #04 (use)[000448] ---X------- │ │ └──▌ ARR_LENGTH int <l:$380, c:$395>
N012 ( 1, 1) [000445] ----------- │ │ └──▌ LCL_VAR ref V30 tmp26 u:2 <l:$146, c:$12e>
N015 ( 1, 1) [000600] ----------- │ └──▌ LCL_VAR long V37 cse2 u:1 <l:$183, c:$402>before, while we end up with N016 ( 8, 11) [000449] ---X-O----- │ │ └──▌ BOUNDS_CHECK_Rng void <l:$158, c:$4d7>
N013 ( 1, 1) [000130] ----------- │ │ ├──▌ CNS_INT int 0 $41
N015 ( 3, 3) CSE #04 (use)[000448] ---X------- │ │ └──▌ ARR_LENGTH int <l:$380, c:$395>
N014 ( 1, 1) [000445] ----------- │ │ └──▌ LCL_VAR ref V30 tmp26 u:2 <l:$146, c:$12e>
N018 ( 1, 1) [000600] ----------- │ └──▌ LCL_VAR long V37 cse2 u:1 <l:$183, c:$402>after. Then, range check is able to recognize this in the 'before' case, but not in the 'after' case: Before optRemoveRangeCheck:
N014 ( 7, 10) [000551] ---XGO----- ▌ COMMA void <l:$478, c:$47d>
N012 ( 6, 9) [000449] ---X-O----- ├──▌ BOUNDS_CHECK_Rng void <l:$158, c:$4d7>
N010 ( 1, 1) [000130] ----------- │ ├──▌ CNS_INT int 0 $41
N011 ( 1, 1) [000613] ----------- │ └──▌ LCL_VAR int V38 cse3 u:1 <l:$340, c:$341>
N013 ( 1, 1) [000600] ----------- └──▌ LCL_VAR long V37 cse2 u:1 <l:$183, c:$402>
After optRemoveRangeCheck for [000551]:
N019 ( 16, 26) [000550] -A-XGO----- ▌ COMMA int
N017 ( 15, 25) [000554] -A-XGO----- ├──▌ COMMA void <l:$47a, c:$5c0>
N003 ( 1, 3) [000426] -A--G---R-- │ ├──▌ ASG ref <l:$14c, c:$4c7>
N002 ( 1, 1) [000425] D------N--- │ │ ├──▌ LCL_VAR ref V29 tmp25 d:2 $VN.Void
N001 ( 1, 1) [000587] ----------- │ │ └──▌ LCL_VAR ref V36 cse1 u:1 <l:$146, c:$104>
N016 ( 14, 22) [000553] -A-XGO----- │ └──▌ COMMA void <l:$47a, c:$47f>
N006 ( 6, 9) [000431] ---X-O----- │ ├──▌ BOUNDS_CHECK_Rng void <l:$158, c:$4cd>
N004 ( 1, 1) [000124] ----------- │ │ ├──▌ CNS_INT int 0 $41
N005 ( 1, 1) [000612] ----------- │ │ └──▌ LCL_VAR int V38 cse3 u:1 <l:$340, c:$341>
N015 ( 8, 13) [000552] -A--GO----- │ └──▌ COMMA void <l:$47a, c:$47e>
N009 ( 1, 3) [000444] -A--G---R-- │ ├──▌ ASG ref <l:$14c, c:$4d1>
N008 ( 1, 1) [000443] D------N--- │ │ ├──▌ LCL_VAR ref V30 tmp26 d:2 $VN.Void
N007 ( 1, 1) [000588] ----------- │ │ └──▌ LCL_VAR ref V36 cse1 u:1 <l:$146, c:$104>
N014 ( 7, 10) [000551] ----GO-N--- │ └──▌ COMMA void <l:$478, c:$47d>
N012 ( 6, 9) [000449] ----------- │ ├──▌ NOP void
N013 ( 1, 1) [000600] ----------- │ └──▌ LCL_VAR long V37 cse2 u:1 <l:$183, c:$402>
N018 ( 1, 1) [000549] ----------- └──▌ CNS_INT int 1So the "bounds check as op1" pattern is what range check looks for in the embedded case. I don't think it would be particularly hard to teach it the new pattern, but I don't really think it's worth the effort given the diffs (and also, I would rather teach it to not be reliant on the shape here). libraries-jitstress failures are known. The Fuzzlyn failure is again not managing to pick up the right artifacts due to them having an ".Attempt.2" suffix, I've asked internally about how to resolve this, which seems like a relatively recent thing that has started occurring. |
Inline gtBuildCommaList (this is the only usage) and build it in the right order to allow doing it during the walk. Also fix propagation of GTF_DEBUG_NODE_MORPHED flag: only propagate it to the parent if the operands already have been morphed.
I need the refactoring to be able to add support for QMARKs in #79346.
Fix #79543