JIT: Reorder physical promotion and forward sub #87265
Conversation
Physical promotion breaks more forward sub opportunities than it introduces, so reorder these phases. Then teach physical promotion to recognize when it creates new opportunities and selectively reinvoke forward sub for just those cases.
ghost
assigned 
dotnet-issue-labeler
bot
added
the
area-CodeGen-coreclr
|
Tagging subscribers to this area: @JulieLeeMSFT, @jakobbotsch Issue DetailsPhysical promotion breaks more forward sub opportunities than it introduces, so reorder these phases. Then teach physical promotion to recognize when it creates new opportunities and selectively reinvoke forward sub for just those cases.
|
|
Many of the regressions seem to be because the new forward subs sometimes mean we stop physically promoting in some beneficial cases. Often because the candidate struct became a call arg. |
|
/azp run runtime-coreclr jitstress, runtime-coreclr libraries-jitstress, runtime-jit-experimental |
|
Azure Pipelines successfully started running 3 pipeline(s). |
|
Pending CI runs are linux ones that I expect will time out due to the current CI issues. The jitstress failures are #84006, #84911. cc @dotnet/jit-contrib PTAL @AndyAyersMS Quite good diffs. With physical promotion.. Without old promotion. As mentioned above, many of the regressions are because of cases where we now no longer physically promote, often because we forward sub a struct use into an argument position, and that makes it more expensive to promote in the eyes of physical promotion. Other cases are more usual ones that come from more forward sub. |
|
CI is hitting an assert in some tier0-FullOpts code that I need to investigate. |
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 ```
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 #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 ```
|
/azp run runtime-coreclr libraries-jitstress |
|
Azure Pipelines successfully started running 1 pipeline(s). |
Physical promotion breaks more forward sub opportunities than it introduces, so reorder these phases. Then teach physical promotion to recognize when it creates new opportunities and selectively reinvoke forward sub for just those cases.