JIT: Fold unreachable cases for switch in importer. #82793

JulieLeeMSFT · 2023-02-28T21:14:09Z

This PR folds unreachable cases in Switch early in Import phase when switch value is GT_CNT_INT.
Fixes #65368.

Below trees show the optimization example for the below scenario:

static void Foo() => Bar(3);

static void Bar(int i)
{
    switch (i)
    {
        case 0:
            Console.WriteLine();
            break;
        case 1:
            Console.WriteLine();
            break;
        case 2:
            Console.WriteLine();
            break;
        case 3:
            Console.WriteLine();
            break;
        case 4:
            Console.WriteLine();
            break;
        case 5:
            // no-op
            break;
    }
}

Trees before Switch folding:

*************** Inline @[000003] Finishing PHASE Profile incorporation
Trees after Profile incorporation

-----------------------------------------------------------------------------------------------------------------------------------------
BBnum BBid ref try hnd preds           weight    lp [IL range]     [jump]      [EH region]         [flags]
-----------------------------------------------------------------------------------------------------------------------------------------
BB03 [0002]  1                             1       [000..01E)-> BB05,BB06,BB07,BB08,BB09,BB10,BB04[def] (switch)                     
BB04 [0003]  1       BB03                  1       [01E..01F)        (return)                     
BB05 [0004]  1       BB03                  1       [01F..025)        (return)                     
BB06 [0005]  1       BB03                  1       [025..02B)        (return)                     
BB07 [0006]  1       BB03                  1       [02B..031)        (return)                     
BB08 [0007]  1       BB03                  1       [031..037)        (return)                     
BB09 [0008]  1       BB03                  1       [037..03C)                                     
BB10 [0009]  2       BB03,BB09             1       [03C..03D)        (return)                     
-----------------------------------------------------------------------------------------------------------------------------------------

After Importation, BB03 always jumps to BB08 and other basic blocks are folded away:

*************** Inline @[000003] Finishing PHASE Importation
Trees after Importation

-----------------------------------------------------------------------------------------------------------------------------------------
BBnum BBid ref try hnd preds           weight    lp [IL range]     [jump]      [EH region]         [flags]
-----------------------------------------------------------------------------------------------------------------------------------------
BB03 [0002]  1                             1       [000..01E)-> BB08 (always)                     i 
BB04 [0003]  0                             1       [01E..01F)        (return)                     
BB05 [0004]  0                             1       [01F..025)        (return)                     
BB06 [0005]  0                             1       [025..02B)        (return)                     
BB07 [0006]  0                             1       [02B..031)        (return)                     
BB08 [0007]  1       BB03                  1       [031..037)        (return)                     i 
BB09 [0008]  0                             1       [037..03C)                                     
BB10 [0009]  1       BB09                  1       [03C..03D)        (return)                     
-----------------------------------------------------------------------------------------------------------------------------------------

*************** Before renumbering the basic blocks

-----------------------------------------------------------------------------------------------------------------------------------------
BBnum BBid ref try hnd preds           weight    lp [IL range]     [jump]      [EH region]         [flags]
-----------------------------------------------------------------------------------------------------------------------------------------
BB03 [0002]  1                             1       [000..01E)-> BB08 (always)                     i 
BB08 [0007]  1       BB03                  1       [031..037)        (return)                     i 
-----------------------------------------------------------------------------------------------------------------------------------------

***************  Exception Handling table is empty
Renumber BB08 to BB04

*************** Inline @[000003] Finishing PHASE Post-import
Trees after Post-import

-----------------------------------------------------------------------------------------------------------------------------------------
BBnum BBid ref try hnd preds           weight    lp [IL range]     [jump]      [EH region]         [flags]
-----------------------------------------------------------------------------------------------------------------------------------------
BB03 [0002]  1                             1       [000..01E)-> BB04 (always)                     i 
BB04 [0007]  1       BB03                  1       [031..037)        (return)                     i 
-----------------------------------------------------------------------------------------------------------------------------------------

------------ BB03 [000..01E) -> BB04 (always), preds={} succs={BB04}

***** BB03
STMT00003 ( 0x000[E-] ... ??? ) <- INL01 @ 0x000[E-] <- INLRT @ 0x000[E-]
               [000006] -----------                         *  NOP       void  

------------ BB04 [031..037) (return), preds={BB03} succs={}

***** BB04
STMT00004 ( 0x031[E-] ... ??? ) <- INL01 @ 0x000[E-] <- INLRT @ 0x000[E-]
               [000007] --C-G------                         *  CALL      void   System.Console:WriteLine()

ghost · 2023-02-28T21:14:20Z

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

Issue Details

Draft PR.

Author:	JulieLeeMSFT
Assignees:	-
Labels:	`area-CodeGen-coreclr`
Milestone:	-

AndyAyersMS · 2023-03-01T22:30:54Z

@JulieLeeMSFT is this ready for review?

AndyAyersMS · 2023-03-01T22:34:41Z

Diffs show some unexpected size increases, eg

;; x64 windows libraries_tests.pmi.windows.x64.checked.mch

Top method regressions (percentages):
         800 (37.54 % of base) : 175521.dasm - System.Runtime.Serialization.Xml.XsdDataContractExporterTests.ExporterApiTests+<>c:<Export_MemberData>b__7_7(System.String,System.Xml.Schema.XmlSchemaSet):this
         630 (37.21 % of base) : 57285.dasm - System.Text.Json.Serialization.Tests.PolymorphicTests:<NestedObjectAsRootObject>g__Verify|6_0(System.String)
         420 (31.37 % of base) : 56898.dasm - System.Text.Json.Serialization.Tests.NullTests:NullReferences()

Can you investigate?

Also some diffs in minopts which we probably don't want.

JulieLeeMSFT · 2023-03-02T03:15:41Z

Yes, I am working on it.

Can you investigate?

AndyAyersMS · 2023-03-06T16:52:02Z

src/coreclr/jit/importer.cpp

                /* Pop the switch value off the stack */
                op1 = impPopStack().val;
                assertImp(genActualTypeIsIntOrI(op1->TypeGet()));

-                /* We can create a switch node */
+                // Fold Switch for GT_CNS_INT
+                if ((op1->gtOper == GT_CNS_INT) && !compIsForImportOnly())


Suggested change

if ((op1->gtOper == GT_CNS_INT) && !compIsForImportOnly())

if (opts.OptimizationEnabled() && (op1->gtOper == GT_CNS_INT) && !compIsForImportOnly())

We don't want to do this unless we're optimizing the method.

This should fix the diffs you are seeing with minopts.

AndyAyersMS · 2023-03-06T16:53:39Z

src/coreclr/jit/importer.cpp

                /* Pop the switch value off the stack */
                op1 = impPopStack().val;
                assertImp(genActualTypeIsIntOrI(op1->TypeGet()));

-                /* We can create a switch node */
+                // Fold Switch for GT_CNS_INT


You should try constant-folding the op1 here, to increase the number of times we see constants.

op1 = gtFoldExpr(impPopStack().val);

(replace line 8004 above)

AndyAyersMS · 2023-03-06T16:55:51Z

src/coreclr/jit/importer.cpp

+                            printf(" to " FMT_BB, block->bbJumpDest->bbNum);
+                        }
+                        printf("\n");
+                        fgDispBasicBlocks(true);


We probably don't need to show all BBs here?

… resolve regression in minopts.

JulieLeeMSFT · 2023-03-14T22:42:02Z

Diff

Size: -6216/+10973 
Throughput: up to -0.06% (improved)

JulieLeeMSFT · 2023-03-14T22:47:34Z

Size regressions are good diffs or incidental changes. So, all look good. Some examples:

Case 1: In diff, some inlines in Switch arm got folded away. And, later there are inlines in the diff that were not done in the base becasue the diff compiler did not do those extra inlines, it has fewer local vars, and that made a later inline more attractive. So, this is good diffs.
Case 2: 2 jumps are removed. So, it is a good diff.
Case 3: Incidental size increase due to removed "SHORT".

JulieLeeMSFT · 2023-03-14T23:14:05Z

@AndyAyersMS, it is ready for review.
CC @dotnet/jit-contrib.
All tests passed in the commit f9b19b7.

Diffs

AndyAyersMS

Looks good!

dotnet-issue-labeler bot added the area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI label Feb 28, 2023

ghost assigned JulieLeeMSFT Feb 28, 2023

JulieLeeMSFT added this to the 8.0.0 milestone Feb 28, 2023

AndyAyersMS reviewed Mar 6, 2023

View reviewed changes

JulieLeeMSFT added 2 commits March 14, 2023 07:29

Fold unreachable cases for switch in importer.

063e8c4

Enable Switch Folding only when opts.OptimizationEnabled() is true to…

f9b19b7

… resolve regression in minopts.

JulieLeeMSFT force-pushed the 65368 branch from fd1614a to f9b19b7 Compare March 14, 2023 14:41

Code cleanup.

443e2a8

AndyAyersMS approved these changes Mar 15, 2023

View reviewed changes

JulieLeeMSFT merged commit 8ab7a2f into dotnet:main Mar 15, 2023

JulieLeeMSFT deleted the 65368 branch March 15, 2023 16:15

JulieLeeMSFT mentioned this pull request Apr 4, 2023

What's new in .NET 8 Preview 3 dotnet/core#8135

Open

3 tasks

ghost locked as resolved and limited conversation to collaborators Apr 14, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

JIT: Fold unreachable cases for switch in importer. #82793

JIT: Fold unreachable cases for switch in importer. #82793

JulieLeeMSFT commented Feb 28, 2023 •

edited

Loading

ghost commented Feb 28, 2023

AndyAyersMS commented Mar 1, 2023

AndyAyersMS commented Mar 1, 2023

JulieLeeMSFT commented Mar 2, 2023

AndyAyersMS Mar 6, 2023

JulieLeeMSFT Mar 14, 2023

AndyAyersMS Mar 6, 2023

JulieLeeMSFT Mar 14, 2023

AndyAyersMS Mar 6, 2023

JulieLeeMSFT Mar 14, 2023

JulieLeeMSFT commented Mar 14, 2023

JulieLeeMSFT commented Mar 14, 2023

JulieLeeMSFT commented Mar 14, 2023 •

edited by AndyAyersMS

Loading

AndyAyersMS left a comment

	if ((op1->gtOper == GT_CNS_INT) && !compIsForImportOnly())
	if (opts.OptimizationEnabled() && (op1->gtOper == GT_CNS_INT) && !compIsForImportOnly())

JIT: Fold unreachable cases for switch in importer. #82793

JIT: Fold unreachable cases for switch in importer. #82793

Conversation

JulieLeeMSFT commented Feb 28, 2023 • edited Loading

ghost commented Feb 28, 2023

AndyAyersMS commented Mar 1, 2023

AndyAyersMS commented Mar 1, 2023

JulieLeeMSFT commented Mar 2, 2023

AndyAyersMS Mar 6, 2023

Choose a reason for hiding this comment

JulieLeeMSFT Mar 14, 2023

Choose a reason for hiding this comment

AndyAyersMS Mar 6, 2023

Choose a reason for hiding this comment

JulieLeeMSFT Mar 14, 2023

Choose a reason for hiding this comment

AndyAyersMS Mar 6, 2023

Choose a reason for hiding this comment

JulieLeeMSFT Mar 14, 2023

Choose a reason for hiding this comment

JulieLeeMSFT commented Mar 14, 2023

JulieLeeMSFT commented Mar 14, 2023

JulieLeeMSFT commented Mar 14, 2023 • edited by AndyAyersMS Loading

AndyAyersMS left a comment

Choose a reason for hiding this comment

JulieLeeMSFT commented Feb 28, 2023 •

edited

Loading

JulieLeeMSFT commented Mar 14, 2023 •

edited by AndyAyersMS

Loading