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.

Sign up for GitHub

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

Jump to bottom

Consider using range-checking for contiguous subsets of switches leading to the same label. #14878

Closed
VSadov opened this issue Nov 2, 2016 · 0 comments · Fixed by #16054
Closed

Consider using range-checking for contiguous subsets of switches leading to the same label. #14878

VSadov opened this issue Nov 2, 2016 · 0 comments · Fixed by #16054
Labels
Area-Compilers Concept-Design Debt Engineering Debt, Design Debt, or poor product code quality Language-C#
Milestone
15.1

Comments

@VSadov
Copy link
Member

VSadov commented Nov 2, 2016 •
edited
Loading

Related to https://github.com/dotnet/coreclr/issues/7914

Becasue of cascaded dispatching into "try" regions , it is not uncommon to see async state machine to contain degenerate switches like

IL_0008:  switch     ( 
                          IL_003b,
                          IL_003b,
                          IL_003b,
                          IL_003b,
                          IL_003b,
                          IL_003b,
                          IL_003b,
                          IL_0729)

Similar code might happen when code is produced via a generator or even written by hand.

We could detect subranges of a switch bucket leading to the same label and at some threshold ( 4 or 5? ), split them off into separate buckets that only need to do a range check.
@VSadov VSadov added Language-C# Area-Compilers Concept-Design Debt Engineering Debt, Design Debt, or poor product code quality labels Nov 2, 2016
@VSadov VSadov added this to the 2.1 milestone Nov 2, 2016
VSadov added a commit to VSadov/roslyn that referenced this issue Nov 20, 2016
@VSadov
added a test for dotnet#14878
27de146
VSadov added a commit that referenced this issue Nov 21, 2016
@VSadov
Merge pull request #15397 from VSadov/test
b998d23 
added a test for #14878
VSadov added a commit that referenced this issue Dec 21, 2016
@VSadov
Better codegen quality when emitting degenerate switches.
a22c4d5 
Related to dotnet/coreclr#7914
Where the offending pattern was observed in the IL generated for the Kestrel web server and was recommended as an easy improvement.

Becasue of cascaded dispatching into "try" regions , it is not uncommon to see async state machine to contain degenerate switches like

IL_0008:  switch     (
                          IL_003b,
                          IL_003b,
                          IL_003b,
                          IL_003b,
                          IL_003b,
                          IL_003b,
                          IL_003b,
                          IL_0729)

Note numerous cases all leading to the same target.
We can trivially emit such switches as just a range-check (that switch would need to do anyways), without any actual "switching".
Since all cases lead to the same label anyways, there is no need to switch once we know the value is in range of the switch.

Fixes:#14878
@VSadov VSadov mentioned this issue Dec 21, 2016
Merged
VSadov added a commit that referenced this issue Dec 28, 2016
@VSadov
Better codegen quality when emitting degenerate switches.
9345ed4 
Related to dotnet/coreclr#7914
Where the offending pattern was observed in the IL generated for the Kestrel web server and was recommended as an easy improvement.

Becasue of cascaded dispatching into "try" regions , it is not uncommon to see async state machine to contain degenerate switches like

IL_0008:  switch     (
                          IL_003b,
                          IL_003b,
                          IL_003b,
                          IL_003b,
                          IL_003b,
                          IL_003b,
                          IL_003b,
                          IL_0729)

Note numerous cases all leading to the same target.
We can trivially emit such switches as just a range-check (that switch would need to do anyways), without any actual "switching".
Since all cases lead to the same label anyways, there is no need to switch once we know the value is in range of the switch.

Fixes:#14878
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
Area-Compilers Concept-Design Debt Engineering Debt, Design Debt, or poor product code quality Language-C#
Projects
None yet
Milestone
15.1
Development

Successfully merging a pull request may close this issue.

Better codegen quality when emitting degenerate switches. dotnet/roslyn
1 participant
@VSadov