Skip to content

Commit

Permalink
Reapply "Avoid creating a result temp for is-expressions (dotnet#68694)…
Browse files Browse the repository at this point in the history
…" (dotnet#69582)

This reverts commit b4a9c62.
  • Loading branch information
alrz committed Mar 7, 2024
1 parent 3984868 commit bb65732
Show file tree
Hide file tree
Showing 16 changed files with 993 additions and 663 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.
// See the LICENSE file in the project root for more information.

using System.Diagnostics;

namespace Microsoft.CodeAnalysis.CSharp;

partial class BoundLoweredIsPatternExpression
{
private partial void Validate()
{
// Ensure fall-through is unreachable
Debug.Assert(this.Statements is [.., BoundGotoStatement or BoundSwitchDispatch]);
}
}
8 changes: 8 additions & 0 deletions src/Compilers/CSharp/Portable/BoundTree/BoundNodes.xml
Original file line number Diff line number Diff line change
Expand Up @@ -2340,6 +2340,7 @@
'is not Type t') will need to compensate for negated patterns. IsNegated is set if Pattern is the negated
form of the inner pattern represented by DecisionDag. -->
<Node Name="BoundIsPatternExpression" Base="BoundExpression">
<Field Name="Type" Type="TypeSymbol" Override="true" Null="disallow"/>
<Field Name="Expression" Type="BoundExpression" Null="disallow"/>
<Field Name="Pattern" Type="BoundPattern" Null="disallow"/>
<Field Name="IsNegated" Type="bool"/>
Expand All @@ -2348,6 +2349,13 @@
<Field Name="WhenFalseLabel" Type="LabelSymbol" Null="disallow"/>
</Node>

<Node Name="BoundLoweredIsPatternExpression" Base="BoundExpression" HasValidate="true">
<Field Name="Type" Type="TypeSymbol" Override="true" Null="disallow"/>
<Field Name="Statements" Type="ImmutableArray&lt;BoundStatement&gt;"/>
<Field Name="WhenTrueLabel" Type="LabelSymbol" Null="disallow"/>
<Field Name="WhenFalseLabel" Type="LabelSymbol" Null="disallow"/>
</Node>

<AbstractNode Name="BoundPattern" Base="BoundNode">
<Field Name="InputType" Type="TypeSymbol" Null="disallow"/>
<Field Name="NarrowedType" Type="TypeSymbol" Null="disallow"/>
Expand Down
51 changes: 49 additions & 2 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.LoweredIsPatternExpression:
EmitLoweredIsPatternExpression((BoundLoweredIsPatternExpression)expression, used);
break;

case BoundKind.LoweredConditionalAccess:
EmitLoweredConditionalAccessExpression((BoundLoweredConditionalAccess)expression, used);
break;
Expand Down Expand Up @@ -352,6 +356,42 @@ private void EmitExpressionCore(BoundExpression expression, bool used)
}
}

private void EmitLoweredIsPatternExpression(BoundLoweredIsPatternExpression node, bool used, bool sense = true)
{
EmitSideEffects(node.Statements);

if (!used)
{
_builder.MarkLabel(node.WhenTrueLabel);
_builder.MarkLabel(node.WhenFalseLabel);
}
else
{
var doneLabel = new object();
_builder.MarkLabel(node.WhenTrueLabel);
_builder.EmitBoolConstant(sense);
_builder.EmitBranch(ILOpCode.Br, doneLabel);
_builder.AdjustStack(-1);
_builder.MarkLabel(node.WhenFalseLabel);
_builder.EmitBoolConstant(!sense);
_builder.MarkLabel(doneLabel);
}
}

private void EmitSideEffects(ImmutableArray<BoundStatement> statements)
{
#if DEBUG
int prevStack = _expectedStackDepth;
int origStack = _builder.GetStackDepth();
_expectedStackDepth = origStack;
#endif
EmitStatements(statements);
#if DEBUG
Debug.Assert(_expectedStackDepth == origStack);
_expectedStackDepth = prevStack;
#endif
}

private void EmitThrowExpression(BoundThrowExpression node, bool used)
{
this.EmitThrow(node.Expression);
Expand Down Expand Up @@ -830,7 +870,7 @@ private void EmitSequencePoint(BoundSequencePointExpression node)
}
}

private void EmitSequenceExpression(BoundSequence sequence, bool used)
private void EmitSequenceExpression(BoundSequence sequence, bool used, bool sense = true)
{
DefineLocals(sequence);
EmitSideEffects(sequence);
Expand All @@ -844,7 +884,14 @@ private void EmitSequenceExpression(BoundSequence sequence, bool used)
Debug.Assert(sequence.Value.Kind != BoundKind.TypeExpression || !used);
if (sequence.Value.Kind != BoundKind.TypeExpression)
{
EmitExpression(sequence.Value, used);
if (used && sequence.Type.SpecialType == SpecialType.System_Boolean)
{
EmitCondExpr(sequence.Value, sense: sense);
}
else
{
EmitExpression(sequence.Value, used: used);
}
}

// sequence is used as a value, can release all locals
Expand Down
21 changes: 16 additions & 5 deletions src/Compilers/CSharp/Portable/CodeGen/EmitOperators.cs
Original file line number Diff line number Diff line change
Expand Up @@ -484,14 +484,25 @@ private void EmitCondExpr(BoundExpression condition, bool sense)
return;
}

if (condition.Kind == BoundKind.BinaryOperator)
switch (condition.Kind)
{
var binOp = (BoundBinaryOperator)condition;
if (IsConditional(binOp.OperatorKind))
{
case BoundKind.BinaryOperator:
var binOp = (BoundBinaryOperator)condition;
if (!IsConditional(binOp.OperatorKind))
{
break;
}

EmitBinaryCondOperator(binOp, sense);
return;
}

case BoundKind.Sequence:
EmitSequenceExpression((BoundSequence)condition, used: true, sense);
return;

case BoundKind.LoweredIsPatternExpression:
EmitLoweredIsPatternExpression((BoundLoweredIsPatternExpression)condition, used: true, sense);
return;
}

EmitExpression(condition, true);
Expand Down
28 changes: 27 additions & 1 deletion src/Compilers/CSharp/Portable/CodeGen/EmitStatement.cs
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,10 @@ namespace Microsoft.CodeAnalysis.CSharp.CodeGen
{
internal partial class CodeGenerator
{
#if DEBUG
private int _expectedStackDepth = 0;
#endif

private void EmitStatement(BoundStatement statement)
{
switch (statement.Kind)
Expand Down Expand Up @@ -108,7 +112,7 @@ private void EmitStatement(BoundStatement statement)
#if DEBUG
if (_stackLocals == null || _stackLocals.Count == 0)
{
_builder.AssertStackEmpty();
_builder.AssertStackDepth(_expectedStackDepth);
}
#endif

Expand Down Expand Up @@ -572,6 +576,28 @@ top.condition is BoundBinaryOperator binary &&
}
return;

case BoundKind.LoweredIsPatternExpression:
{
var loweredIs = (BoundLoweredIsPatternExpression)condition;
dest ??= new object();

EmitSideEffects(loweredIs.Statements);

if (sense)
{
_builder.MarkLabel(loweredIs.WhenTrueLabel);
_builder.EmitBranch(ILOpCode.Br, dest);
_builder.MarkLabel(loweredIs.WhenFalseLabel);
}
else
{
_builder.MarkLabel(loweredIs.WhenFalseLabel);
_builder.EmitBranch(ILOpCode.Br, dest);
_builder.MarkLabel(loweredIs.WhenTrueLabel);
}
}
return;

case BoundKind.UnaryOperator:
var unOp = (BoundUnaryOperator)condition;
if (unOp.OperatorKind == UnaryOperatorKind.BoolLogicalNegation)
Expand Down
33 changes: 30 additions & 3 deletions src/Compilers/CSharp/Portable/CodeGen/Optimizer.cs
Original file line number Diff line number Diff line change
Expand Up @@ -415,6 +415,10 @@ internal sealed class StackOptimizerPass1 : BoundTreeRewriter

private int _recursionDepth;

#if DEBUG
private int _expectedStackDepth = 0;
#endif

private StackOptimizerPass1(Dictionary<LocalSymbol, LocalDefUseInfo> locals,
ArrayBuilder<ValueTuple<BoundExpression, ExprContext>> evalStack,
bool debugFriendly)
Expand Down Expand Up @@ -565,10 +569,27 @@ private void PopEvalStack()

public BoundNode VisitStatement(BoundNode node)
{
Debug.Assert(node == null || EvalStackIsEmpty());
#if DEBUG
Debug.Assert(node == null || StackDepth() == _expectedStackDepth);
#endif
return VisitSideEffect(node);
}

public ImmutableArray<BoundStatement> VisitSideEffects(ImmutableArray<BoundStatement> statements)
{
#if DEBUG
int prevStack = _expectedStackDepth;
int origStack = StackDepth();
_expectedStackDepth = origStack;
#endif
var result = this.VisitList(statements);
#if DEBUG
Debug.Assert(_expectedStackDepth == origStack);
_expectedStackDepth = prevStack;
#endif
return result;
}

public BoundNode VisitSideEffect(BoundNode node)
{
var origStack = StackDepth();
Expand Down Expand Up @@ -1416,8 +1437,6 @@ public override BoundNode VisitConditionalGoto(BoundConditionalGoto node)

public override BoundNode VisitSwitchDispatch(BoundSwitchDispatch node)
{
Debug.Assert(EvalStackIsEmpty());

// switch dispatch needs a byval local or a parameter as a key.
// if this is already a fitting local, let's keep it that way
BoundExpression boundExpression = node.Expression;
Expand Down Expand Up @@ -1447,6 +1466,14 @@ public override BoundNode VisitSwitchDispatch(BoundSwitchDispatch node)
return node.Update(boundExpression, node.Cases, node.DefaultLabel, node.LengthBasedStringSwitchDataOpt);
}

public override BoundNode VisitLoweredIsPatternExpression(BoundLoweredIsPatternExpression node)
{
var statements = VisitSideEffects(node.Statements);
RecordBranch(node.WhenTrueLabel);
RecordBranch(node.WhenFalseLabel);
return node.Update(statements, node.WhenTrueLabel, node.WhenFalseLabel, VisitType(node.Type));
}

public override BoundNode VisitConditionalOperator(BoundConditionalOperator node)
{
var origStack = StackDepth();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3039,6 +3039,12 @@ public override BoundNode VisitConditionalReceiver(BoundConditionalReceiver node
return null;
}

public override BoundNode VisitLoweredIsPatternExpression(BoundLoweredIsPatternExpression node)
{
VisitStatements(node.Statements);
return null;
}

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

0 comments on commit bb65732

Please sign in to comment.