Skip to content

Commit

Permalink
Avoid creating result temp for switch-expressions
Browse files Browse the repository at this point in the history
  • Loading branch information
alrz committed Jul 24, 2023
1 parent 62b24d7 commit 18d940c
Show file tree
Hide file tree
Showing 13 changed files with 493 additions and 637 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -14,3 +14,12 @@ private partial void Validate()
Debug.Assert(this.Statements is [.., BoundGotoStatement or BoundSwitchDispatch]);
}
}

partial class BoundLoweredSwitchExpression
{
private partial void Validate()
{
Debug.Assert(this.Statements is [.., BoundGotoStatement or BoundSwitchDispatch]);
Debug.Assert(!this.SwitchArms.IsEmpty);
}
}
12 changes: 12 additions & 0 deletions src/Compilers/CSharp/Portable/BoundTree/BoundNodes.xml
Original file line number Diff line number Diff line change
Expand Up @@ -1504,6 +1504,18 @@
<Field Name="WasTargetTyped" Type="bool" />
</Node>

<Node Name="BoundLoweredSwitchExpression" Base="BoundExpression" HasValidate="true">
<Field Name="Type" Type="TypeSymbol" Override="true" Null="disallow"/>
<Field Name="Statements" Type="ImmutableArray&lt;BoundStatement&gt;"/>
<Field Name="SwitchArms" Type="ImmutableArray&lt;BoundLoweredSwitchExpressionArm&gt;"/>
</Node>

<Node Name="BoundLoweredSwitchExpressionArm" Base="BoundNode">
<Field Name="Locals" Type="ImmutableArray&lt;LocalSymbol&gt;"/>
<Field Name="Statements" Type="ImmutableArray&lt;BoundStatement&gt;"/>
<Field Name="Value" Type="BoundExpression" Null="disallow"/>
</Node>

<Node Name="BoundDecisionDag" Base="BoundNode">
<Field Name="RootNode" Type="BoundDecisionDagNode" Null="disallow"/>
</Node>
Expand Down
73 changes: 53 additions & 20 deletions src/Compilers/CSharp/Portable/CodeGen/EmitExpression.cs
Original file line number Diff line number Diff line change
Expand Up @@ -315,6 +315,10 @@ private void EmitExpressionCore(BoundExpression expression, bool used)
EmitRefValueOperator((BoundRefValueOperator)expression, used);
break;

case BoundKind.LoweredSwitchExpression:
EmitLoweredSwitchExpression((BoundLoweredSwitchExpression)expression, used);
break;

case BoundKind.LoweredIsPatternExpression:
EmitLoweredIsPatternExpression((BoundLoweredIsPatternExpression)expression, used);
break;
Expand Down Expand Up @@ -356,6 +360,42 @@ private void EmitExpressionCore(BoundExpression expression, bool used)
}
}

private void EmitLoweredSwitchExpression(BoundLoweredSwitchExpression node, bool used)
{
EmitSideEffects(node.Statements);

var doneLabel = new object();
for (int i = 0, n = node.SwitchArms.Length; i < n; i++)
{
var switchArm = node.SwitchArms[i];
if (!switchArm.Locals.IsEmpty)
{
_builder.OpenLocalScope();
DefineScopeLocals(switchArm.Locals);
}

EmitSideEffects(switchArm.Statements);
EmitExpression(switchArm.Value, used);
if (used)
{
EmitStaticCastIfNeeded(node, switchArm.Value);
}

if (!switchArm.Locals.IsEmpty)
{
_builder.CloseLocalScope();
}

_builder.EmitBranch(ILOpCode.Br, doneLabel);
if (used && i != n - 1)
{
_builder.AdjustStack(-1);
}
}

_builder.MarkLabel(doneLabel);
}

private void EmitLoweredIsPatternExpression(BoundLoweredIsPatternExpression node, bool used, bool sense = true)
{
EmitSideEffects(node.Statements);
Expand Down Expand Up @@ -3751,18 +3791,9 @@ private void EmitConditionalOperator(BoundConditionalOperator expr, bool used)
// it seems that either PEVerify or the runtime/JIT verifier will complain at you if you try to remove
// either of the casts.
//
var mergeTypeOfAlternative = StackMergeType(expr.Alternative);
if (used)
{
if (IsVarianceCast(expr.Type, mergeTypeOfAlternative))
{
EmitStaticCast(expr.Type, expr.Syntax);
mergeTypeOfAlternative = expr.Type;
}
else if (expr.Type.IsInterfaceType() && !TypeSymbol.Equals(expr.Type, mergeTypeOfAlternative, TypeCompareKind.ConsiderEverything2))
{
EmitStaticCast(expr.Type, expr.Syntax);
}
EmitStaticCastIfNeeded(expr, expr.Alternative);
}

_builder.EmitBranch(ILOpCode.Br, doneLabel);
Expand All @@ -3777,16 +3808,7 @@ private void EmitConditionalOperator(BoundConditionalOperator expr, bool used)

if (used)
{
var mergeTypeOfConsequence = StackMergeType(expr.Consequence);
if (IsVarianceCast(expr.Type, mergeTypeOfConsequence))
{
EmitStaticCast(expr.Type, expr.Syntax);
mergeTypeOfConsequence = expr.Type;
}
else if (expr.Type.IsInterfaceType() && !TypeSymbol.Equals(expr.Type, mergeTypeOfConsequence, TypeCompareKind.ConsiderEverything2))
{
EmitStaticCast(expr.Type, expr.Syntax);
}
EmitStaticCastIfNeeded(expr, expr.Consequence);
}

_builder.MarkLabel(doneLabel);
Expand Down Expand Up @@ -3819,6 +3841,17 @@ static bool hasIntegralValueZeroOrOne(BoundExpression expr, out bool isOne)
}
}

private void EmitStaticCastIfNeeded(BoundExpression node, BoundExpression expression)
{
Debug.Assert(node.Type is not null);
TypeSymbol stackMergeType = StackMergeType(expression);
if (IsVarianceCast(node.Type, stackMergeType) ||
(node.Type.IsInterfaceType() && !TypeSymbol.Equals(node.Type, stackMergeType, TypeCompareKind.ConsiderEverything2)))
{
EmitStaticCast(node.Type, node.Syntax);
}
}

/// <summary>
/// Emit code for a null-coalescing operator.
/// </summary>
Expand Down
18 changes: 12 additions & 6 deletions src/Compilers/CSharp/Portable/CodeGen/EmitStatement.cs
Original file line number Diff line number Diff line change
Expand Up @@ -796,20 +796,26 @@ private void EmitScope(BoundScope block)

_builder.OpenLocalScope();

foreach (var local in block.Locals)
DefineScopeLocals(block.Locals);

EmitStatements(block.Statements);

_builder.CloseLocalScope();
}

private void DefineScopeLocals(ImmutableArray<LocalSymbol> locals)
{
foreach (var local in locals)
{
Debug.Assert(local.Name != null);
Debug.Assert(local.SynthesizedKind == SynthesizedLocalKind.UserDefined &&
(local.ScopeDesignatorOpt?.Kind() == SyntaxKind.SwitchSection || local.ScopeDesignatorOpt?.Kind() == SyntaxKind.SwitchExpressionArm));
(local.ScopeDesignatorOpt?.Kind() == SyntaxKind.SwitchSection ||
local.ScopeDesignatorOpt?.Kind() == SyntaxKind.SwitchExpressionArm));
if (!local.IsConst && !IsStackLocal(local))
{
_builder.AddLocalToScope(_builder.LocalSlotManager.GetLocal(local));
}
}

EmitStatements(block.Statements);

_builder.CloseLocalScope();
}

private void EmitStateMachineScope(BoundStateMachineScope scope)
Expand Down
29 changes: 28 additions & 1 deletion src/Compilers/CSharp/Portable/CodeGen/Optimizer.cs
Original file line number Diff line number Diff line change
Expand Up @@ -575,7 +575,7 @@ public BoundNode VisitStatement(BoundNode node)
return VisitSideEffect(node);
}

public ImmutableArray<BoundStatement> VisitSideEffects(ImmutableArray<BoundStatement> statements)
public ImmutableArray<T> VisitSideEffects<T>(ImmutableArray<T> statements) where T : BoundNode
{
#if DEBUG
int prevStack = _expectedStackDepth;
Expand Down Expand Up @@ -1466,6 +1466,26 @@ public override BoundNode VisitSwitchDispatch(BoundSwitchDispatch node)
return node.Update(boundExpression, node.Cases, node.DefaultLabel, node.LengthBasedStringSwitchDataOpt);
}

#if DEBUG
public override BoundNode VisitLoweredSwitchExpression(BoundLoweredSwitchExpression node)
{
return node.Update(
VisitSideEffects(node.Statements),
VisitSideEffects(node.SwitchArms),
VisitType(node.Type));
}
#endif

public override BoundNode VisitLoweredSwitchExpressionArm(BoundLoweredSwitchExpressionArm node)
{
var statements = VisitSideEffects(node.Statements);
var value = VisitExpression(node.Value, ExprContext.Value);
PopEvalStack();
_counter++;
EnsureOnlyEvalStack();
return node.Update(node.Locals, statements, value);
}

public override BoundNode VisitLoweredIsPatternExpression(BoundLoweredIsPatternExpression node)
{
var statements = VisitSideEffects(node.Statements);
Expand Down Expand Up @@ -2299,6 +2319,13 @@ BoundExpression visitArgumentsAndUpdateCall(BoundCall node, BoundExpression? rec
}
#nullable disable

public override BoundNode VisitLoweredSwitchExpressionArm(BoundLoweredSwitchExpressionArm switchArm)
{
var result = base.VisitLoweredSwitchExpressionArm(switchArm);
_nodeCounter++;
return result;
}

public override BoundNode VisitCatchBlock(BoundCatchBlock node)
{
var exceptionSource = node.ExceptionSourceOpt;
Expand Down
12 changes: 12 additions & 0 deletions src/Compilers/CSharp/Portable/FlowAnalysis/AbstractFlowPass.cs
Original file line number Diff line number Diff line change
Expand Up @@ -2979,6 +2979,18 @@ public override BoundNode VisitLoweredIsPatternExpression(BoundLoweredIsPatternE
return null;
}

public override BoundNode VisitLoweredSwitchExpression(BoundLoweredSwitchExpression node)
{
VisitStatements(node.Statements);
foreach (var switchArm in node.SwitchArms)
{
VisitStatements(switchArm.Statements);
VisitRvalue(switchArm.Value);
}

return null;
}

public override BoundNode VisitComplexConditionalReceiver(BoundComplexConditionalReceiver node)
{
var savedState = this.State.Clone();
Expand Down
Loading

0 comments on commit 18d940c

Please sign in to comment.