Fix suboptimal IL for small `is` patterns by jjonescz · Pull Request #84961 · dotnet/roslyn

jjonescz · 2026-08-19T15:01:33Z

This PR is a refined version of #82429 (which got stale).

Benchmark: https://gist.github.com/jjonescz/4c35cef07ad198a8da9b17d480da5a99. It also tests sparse vs dense values (so it could be more efficient to consider that too when deciding the lowering strategy), but for starters I think we can simply choose between linear vs DAG lowering on a simple threshold for the number of tests.

See also dotnet/runtime#132452 and dotnet/runtime#132504.

Microsoft Reviewers: Open in CodeFlow

azure-pipelines · 2026-08-19T15:01:53Z

Azure Pipelines: Successfully started running 1 pipeline(s). 1 pipeline(s) were filtered out due to trigger conditions. There may be pipelines that require an authorized user to comment /azp run to run.

Copilot

Pull request overview

Optimizes C# lowering for small disjunctive is patterns to avoid temporary booleans and inefficient branches.

Changes:

Uses inverted linear lowering for binding-free patterns with up to four tests.
Updates affected IL baselines.
Adds regression, threshold, binding, and async tests.

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.

Show a summary per file

File	Description
`LocalRewriter_IsPatternOperator.cs`	Adds the optimized lowering path.
`PatternTests.cs`	Adds regression and boundary tests.
`CodeGenAsyncTests.cs`	Covers async lowering scenarios.
`PatternMatchingTests.cs`	Updates Span pattern IL baselines.
`PatternMatchingTests5.cs`	Updates numeric, string, and Span IL baselines.

azure-pipelines · 2026-08-20T15:18:42Z

Azure Pipelines: Successfully started running 1 pipeline(s). 1 pipeline(s) were filtered out due to trigger conditions. There may be pipelines that require an authorized user to comment /azp run to run.

Copilot

Pull request overview

Copilot reviewed 6 out of 6 changed files in this pull request and generated no new comments.

RikkiGibson · 2026-08-20T21:14:39Z

I was wondering whether the increased overhead of the DAG codegen, is inherent to it or not?

Basically, is there is some adjustment we can instead make to the DAG codegen, which would allow it to generate code generally as good or better than the "naive" sequence of linear tests?

This could either be an optimization of the lowered DAG code itself, or, some shape change to the lowered code which allows a subsequent optimization pass (e.g. temp reuse) to be able to optimize some code it couldn't before.

Note that I'm not deeply opposed to the current approach or to being pragmatic about making sure we can solve the key scenario for .NET 11. Just wondering if it's possible for us to pursue a better general solution.

333fred · 2026-08-20T22:31:26Z

Basically, is there is some adjustment we can instead make to the DAG codegen, which would allow it to generate code generally as good or better than the "naive" sequence of linear tests?

I was also thinking about this. While we should do something pragmatic right now, I'd much rather see a general fix.

jjonescz · 2026-08-21T08:51:34Z

#68694 and #72273 already attempted a more general solution but both had to be reverted. I can file an issue to follow up on that if this PR is merged.

Copilot

Pull request overview

Copilot reviewed 6 out of 6 changed files in this pull request and generated no new comments.

Copilot

Pull request overview

Copilot reviewed 6 out of 6 changed files in this pull request and generated no new comments.

RikkiGibson · 2026-08-24T19:01:55Z

    internal sealed partial class LocalRewriter
    {
+        /// <summary>
+        /// Benchmark results (see https://github.com/dotnet/roslyn/pull/84961) show that short patterns can be more


It would be desirable to commit at least a subset of the benchmarks to src/Tools/Benchmarks/Benchmarks.csproj

Good idea. I think I will leave that to a follow up if that's okay.

RikkiGibson · 2026-08-24T19:11:36Z

-                isPatternRewriter.Free();
+                result = LowerIsPatternAsLinearSequence(node, decisionDag, whenTrueLabel: node.WhenTrueLabel, whenFalseLabel: node.WhenFalseLabel);
            }
            else if (IsFailureNode(decisionDag.RootNode, node.WhenFalseLabel))


nit: It seems like this block could be deleted, I would expect the newly added block on line 40 to have the same effect

This block also handles patterns with bindings which the block below does not.

I didn't understand this part, are you saying that a case like x is var y is handled by this?

x is var y would be handled by the first if. This branch handles a very specific case where the input is constant and so we can determine it is false at compile time (but there can still be a binding), e.g., const int x = 2; x is 3 and int y.

RikkiGibson · 2026-08-24T19:24:26Z

+                whenTrueLabel: node.WhenFalseLabel,
+                whenFalseLabel: node.WhenTrueLabel,
+                maxTests: MaxTestsForInvertedLinearSequence) &&
+                !containsBindings(decisionDag))


What's the reason for needing to bailout when bindings are used, but not in the positive case? I guess when we start inverting things, then, keeping track of when the binding is actually assigned gets more complicated?

The inverted pattern could leave the bindings unassigned.

For example, consider a pattern value is (B or C) and var match. Swapping true/false labels produces value is not B && value is not C. Then we negate it, so the final lowered form is !(value is not B && value is not C). It's equivalent to the original except it is missing the binding; we would need some special logic to preserve it.

Fix suboptimal IL for small is patterns

42ab28d

jjonescz requested a balanced review from Copilot August 19, 2026 15:01

jjonescz added the Area-Compilers label Aug 19, 2026

Copilot started reviewing on behalf of jjonescz August 19, 2026 15:02 View session

Copilot AI reviewed Aug 19, 2026

View reviewed changes

Comment thread src/Compilers/CSharp/Portable/Lowering/LocalRewriter/LocalRewriter_IsPatternOperator.cs Outdated

jjonescz added 3 commits August 20, 2026 14:55

Change threshold to 3

6542492

Fixup IL baselines

dfdb057

Add a comment

1525a03

jjonescz marked this pull request as ready for review August 20, 2026 15:18

jjonescz requested a review from a team as a code owner August 20, 2026 15:18

Copilot AI review requested due to automatic review settings August 20, 2026 15:18

Copilot AI reviewed Aug 20, 2026

View reviewed changes

Copilot started reviewing on behalf of jjonescz August 20, 2026 17:50 View session

333fred reviewed Aug 20, 2026

View reviewed changes

Improve tests

4570a91

Copilot AI review requested due to automatic review settings August 21, 2026 11:18

Copilot started reviewing on behalf of jjonescz August 21, 2026 11:18 View session

Copilot AI reviewed Aug 21, 2026

View reviewed changes

333fred reviewed Aug 21, 2026

View reviewed changes

Comment thread src/Compilers/CSharp/Test/Emit/CodeGen/CodeGenAsyncTests.cs

Verify runtime async

478bf1a

Copilot AI review requested due to automatic review settings August 24, 2026 11:22

Copilot started reviewing on behalf of jjonescz August 24, 2026 11:23 View session

Copilot AI reviewed Aug 24, 2026

View reviewed changes

jjonescz requested a review from 333fred August 24, 2026 13:06

333fred approved these changes Aug 24, 2026

View reviewed changes

jjonescz requested a review from RikkiGibson August 24, 2026 16:59

RikkiGibson reviewed Aug 24, 2026

View reviewed changes

RikkiGibson approved these changes Aug 24, 2026

View reviewed changes

jjonescz merged commit a89bfc7 into dotnet:main Aug 25, 2026
21 checks passed

jjonescz deleted the 80052-is-codegen branch August 25, 2026 11:08

This was referenced Aug 25, 2026

Investigate whether we can improve DAG lowering of is patterns #85025

Open

Fix suboptimal codegen for short or patterns in is-expressions #82429

Closed

Add is pattern benchmarks #85027

Merged

This was referenced Aug 25, 2026

[release/11.0.1xx] Source code updates from dotnet/roslyn dotnet/dotnet#8652

Merged

[main] Source code updates from dotnet/roslyn dotnet/dotnet#8669

Merged

CyrusNajmabadi mentioned this pull request Aug 26, 2026

Merge main into features/target-typed-static-member-access #85056

Merged

dibarbet mentioned this pull request Aug 28, 2026

Update roslyn to 5.12.0-1.26428.1 dotnet/vscode-csharp#9712

Merged

dotnet-bot mentioned this pull request Sep 3, 2026

[Automated] PRs inserted in VS build main-12202.72 #85140

Closed

Conversation

jjonescz commented Aug 19, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Microsoft Reviewers: Open in CodeFlow

Uh oh!

azure-pipelines Bot commented Aug 19, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

azure-pipelines Bot commented Aug 20, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

RikkiGibson commented Aug 20, 2026

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

333fred commented Aug 20, 2026

Uh oh!

jjonescz commented Aug 21, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

jjonescz commented Aug 19, 2026 •

edited

Loading