Skip to content

Commit

Permalink
Revert "Avoid creating a result temp for is-expressions (#68694)"
Browse files Browse the repository at this point in the history 
This reverts commit e1031a7.
  • Loading branch information
@RikkiGibson
RikkiGibson authored Aug 17, 2023
1 parent 1686899 commit 794d6be
Show file tree
Hide file tree
Showing 16 changed files with 663 additions and 993 deletions.
16 changes: 0 additions & 16 deletions src/Compilers/CSharp/Portable/BoundTree/BoundLoweredIsPatternExpression.cs
View file

This file was deleted.

8 changes: 0 additions & 8 deletions src/Compilers/CSharp/Portable/BoundTree/BoundNodes.xml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2316,7 +2316,6 @@
'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 @@ -2325,13 +2324,6 @@
<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: 2 additions & 49 deletions src/Compilers/CSharp/Portable/CodeGen/EmitExpression.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -315,10 +315,6 @@ 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 @@ -356,42 +352,6 @@ 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 @@ -870,7 +830,7 @@ private void EmitSequencePoint(BoundSequencePointExpression node)
}
}

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

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

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

var binOp = (BoundBinaryOperator)condition;
if (IsConditional(binOp.OperatorKind))
{
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: 1 addition & 27 deletions src/Compilers/CSharp/Portable/CodeGen/EmitStatement.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,10 +24,6 @@ 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 @@ -112,7 +108,7 @@ private void EmitStatement(BoundStatement statement)
#if DEBUG
if (_stackLocals == null || _stackLocals.Count == 0)
{
_builder.AssertStackDepth(_expectedStackDepth);
_builder.AssertStackEmpty();
}
#endif

Expand Down Expand Up @@ -576,28 +572,6 @@ 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: 3 additions & 30 deletions src/Compilers/CSharp/Portable/CodeGen/Optimizer.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -415,10 +415,6 @@ 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 @@ -569,27 +565,10 @@ private void PopEvalStack()

public BoundNode VisitStatement(BoundNode node)
{
#if DEBUG
Debug.Assert(node == null || StackDepth() == _expectedStackDepth);
#endif
Debug.Assert(node == null || EvalStackIsEmpty());
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 @@ -1437,6 +1416,8 @@ 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 @@ -1466,14 +1447,6 @@ 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
6 changes: 0 additions & 6 deletions src/Compilers/CSharp/Portable/FlowAnalysis/AbstractFlowPass.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3031,12 +3031,6 @@ 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 794d6be

Please sign in to comment.