Skip to content

Commit 4490de2

Browse files
author
maximv
committed
fixed the IL for the TryEmitComparison for numbers;
added the il.EmitPopIfIgnoreResult to simplify code; added method name output to PrintIL; fixed: ToExpressionString output to not produce the /**/ comments because the prevent the user block comment /**/; -46loc
1 parent d0451bc commit 4490de2

File tree

5 files changed

+111
-147
lines changed

5 files changed

+111
-147
lines changed

‎src/FastExpressionCompiler/FastExpressionCompiler.cs‎

Lines changed: 56 additions & 102 deletions
Original file line numberDiff line numberDiff line change
@@ -1679,6 +1679,13 @@ internal enum ParentFlags : ushort
16791679

16801680
internal static bool IgnoresResult(this ParentFlags parent) => (parent & ParentFlags.IgnoreResult) != 0;
16811681

1682+
internal static bool EmitPopIfIgnoreResult(this ILGenerator il, ParentFlags parent)
1683+
{
1684+
if ((parent & ParentFlags.IgnoreResult) != 0)
1685+
il.Emit(OpCodes.Pop);
1686+
return true;
1687+
}
1688+
16821689
/// <summary>Supports emitting of selected expressions, e.g. lambdaExpr are not supported yet.
16831690
/// When emitter find not supported expression it will return false from <see cref="TryEmit"/>, so I could fallback
16841691
/// to normal and slow Expression.Compile.</summary>
@@ -2226,11 +2233,7 @@ private static bool TryEmitCoalesceOperator(BinaryExpression exprObj, IReadOnlyL
22262233
il.MarkLabel(labelDone);
22272234
}
22282235
}
2229-
2230-
if ((parent & ParentFlags.IgnoreResult) != 0)
2231-
il.Emit(OpCodes.Pop);
2232-
2233-
return true;
2236+
return il.EmitPopIfIgnoreResult(parent);
22342237
}
22352238

22362239
private static void EmitDefault(Type type, ILGenerator il)
@@ -2556,22 +2559,13 @@ private static bool TryEmitSimpleUnaryExpression(UnaryExpression expr, IReadOnly
25562559
}
25572560
}
25582561
else if (expr.NodeType == ExpressionType.OnesComplement)
2559-
{
25602562
il.Emit(OpCodes.Not);
2561-
}
25622563
else if (expr.NodeType == ExpressionType.Unbox)
2563-
{
25642564
il.Emit(OpCodes.Unbox_Any, exprType);
2565-
}
2566-
else if (expr.NodeType == ExpressionType.IsTrue)
2567-
{ }
2568-
else if (expr.NodeType == ExpressionType.UnaryPlus)
2569-
{ }
2570-
2571-
if ((parent & ParentFlags.IgnoreResult) != 0)
2572-
il.Emit(OpCodes.Pop);
2565+
// else if (expr.NodeType == ExpressionType.IsTrue) { }
2566+
// else if (expr.NodeType == ExpressionType.UnaryPlus) { }
25732567

2574-
return true;
2568+
return il.EmitPopIfIgnoreResult(parent);
25752569
}
25762570

25772571
#if LIGHT_EXPRESSION
@@ -2651,11 +2645,7 @@ private static bool TryEmitConvert(UnaryExpression expr, IReadOnlyList<PE> param
26512645
if (!TryEmit(opExpr, paramExprs, il, ref closure, setup,
26522646
parent & ~ParentFlags.IgnoreResult | ParentFlags.InstanceCall, -1))
26532647
return false;
2654-
2655-
il.Emit(method.IsVirtual ? OpCodes.Callvirt : OpCodes.Call, method);
2656-
if ((parent & ParentFlags.IgnoreResult) != 0 && method.ReturnType != typeof(void))
2657-
il.Emit(OpCodes.Pop);
2658-
return true;
2648+
return EmitMethodCall(il, method);
26592649
}
26602650

26612651
var sourceType = opExpr.Type;
@@ -2673,10 +2663,7 @@ private static bool TryEmitConvert(UnaryExpression expr, IReadOnlyList<PE> param
26732663
EmitStoreLocalVariableAndLoadItsAddress(il, sourceType);
26742664

26752665
il.Emit(OpCodes.Call, sourceType.FindValueGetterMethod());
2676-
2677-
if ((parent & ParentFlags.IgnoreResult) != 0)
2678-
il.Emit(OpCodes.Pop);
2679-
return true;
2666+
return il.EmitPopIfIgnoreResult(parent);
26802667
}
26812668

26822669
if (!TryEmit(opExpr, paramExprs, il, ref closure, setup, parent & ~ParentFlags.IgnoreResult & ~ParentFlags.InstanceAccess))
@@ -2694,9 +2681,7 @@ private static bool TryEmitConvert(UnaryExpression expr, IReadOnlyList<PE> param
26942681
{
26952682
if (targetType == typeof(object) && sourceType.IsValueType)
26962683
il.Emit(OpCodes.Box, sourceType);
2697-
if (IgnoresResult(parent))
2698-
il.Emit(OpCodes.Pop);
2699-
return true;
2684+
return il.EmitPopIfIgnoreResult(parent);
27002685
}
27012686

27022687
// check implicit / explicit conversion operators on source and target types
@@ -2708,14 +2693,9 @@ private static bool TryEmitConvert(UnaryExpression expr, IReadOnlyList<PE> param
27082693
if (convertOpMethod != null)
27092694
{
27102695
il.Emit(OpCodes.Call, convertOpMethod);
2711-
27122696
if (targetTypeIsNullable)
27132697
il.Emit(OpCodes.Newobj, targetType.GetTypeInfo().DeclaredConstructors.GetFirst());
2714-
2715-
if ((parent & ParentFlags.IgnoreResult) != 0) // todo: @simplify move this thing into the method and refactor the usages
2716-
il.Emit(OpCodes.Pop);
2717-
2718-
return true;
2698+
return il.EmitPopIfIgnoreResult(parent);
27192699
}
27202700
}
27212701
else if (!targetTypeIsNullable)
@@ -2739,9 +2719,7 @@ private static bool TryEmitConvert(UnaryExpression expr, IReadOnlyList<PE> param
27392719
}
27402720

27412721
il.Emit(OpCodes.Call, convertOpMethod);
2742-
if ((parent & ParentFlags.IgnoreResult) != 0)
2743-
il.Emit(OpCodes.Pop);
2744-
return true;
2722+
return il.EmitPopIfIgnoreResult(parent);
27452723
}
27462724
}
27472725

@@ -2750,9 +2728,7 @@ private static bool TryEmitConvert(UnaryExpression expr, IReadOnlyList<PE> param
27502728
if (method != null && method.DeclaringType == targetType && method.GetParameters()[0].ParameterType == sourceType)
27512729
{
27522730
il.Emit(OpCodes.Call, method);
2753-
if ((parent & ParentFlags.IgnoreResult) != 0)
2754-
il.Emit(OpCodes.Pop);
2755-
return true;
2731+
return il.EmitPopIfIgnoreResult(parent);
27562732
}
27572733

27582734
var actualSourceType = sourceTypeIsNullable ? underlyingNullableSourceType : sourceType;
@@ -2767,10 +2743,7 @@ private static bool TryEmitConvert(UnaryExpression expr, IReadOnlyList<PE> param
27672743
}
27682744

27692745
il.Emit(OpCodes.Call, convertOpMethod);
2770-
if ((parent & ParentFlags.IgnoreResult) != 0)
2771-
il.Emit(OpCodes.Pop);
2772-
2773-
return true;
2746+
return il.EmitPopIfIgnoreResult(parent);
27742747
}
27752748
}
27762749
else if (!sourceTypeIsNullable)
@@ -2780,14 +2753,10 @@ private static bool TryEmitConvert(UnaryExpression expr, IReadOnlyList<PE> param
27802753
if (convertOpMethod != null)
27812754
{
27822755
il.Emit(OpCodes.Call, convertOpMethod);
2783-
27842756
if (targetTypeIsNullable)
27852757
il.Emit(OpCodes.Newobj, targetType.GetTypeInfo().DeclaredConstructors.GetFirst());
27862758

2787-
if ((parent & ParentFlags.IgnoreResult) != 0)
2788-
il.Emit(OpCodes.Pop);
2789-
2790-
return true;
2759+
return il.EmitPopIfIgnoreResult(parent);
27912760
}
27922761
}
27932762

@@ -2859,10 +2828,7 @@ private static bool TryEmitConvert(UnaryExpression expr, IReadOnlyList<PE> param
28592828
}
28602829
}
28612830

2862-
if ((parent & ParentFlags.IgnoreResult) != 0)
2863-
il.Emit(OpCodes.Pop);
2864-
2865-
return true;
2831+
return il.EmitPopIfIgnoreResult(parent);
28662832
}
28672833

28682834
private static bool TryEmitValueConvert(Type targetType, ILGenerator il, bool isChecked)
@@ -2884,7 +2850,7 @@ private static bool TryEmitValueConvert(Type targetType, ILGenerator il, bool is
28842850
else if (targetType == typeof(long))
28852851
il.Emit(isChecked ? OpCodes.Conv_Ovf_I8 : OpCodes.Conv_I8);
28862852
else if (targetType == typeof(ulong))
2887-
il.Emit(isChecked ? OpCodes.Conv_Ovf_U8 : OpCodes.Conv_U8);
2853+
il.Emit(isChecked ? OpCodes.Conv_Ovf_U8 : OpCodes.Conv_U8); // should we consider if sourceType.IsUnsigned == false and using the OpCodes.Conv_I8 (seems like the System.Compile does it)
28882854
else if (targetType == typeof(double))
28892855
il.Emit(OpCodes.Conv_R8);
28902856
else
@@ -3987,7 +3953,7 @@ private static bool TryEmitMemberAccess(MemberExpression expr, IReadOnlyList<PE>
39873953
var p = (parent | ParentFlags.MemberAccess | ParentFlags.InstanceAccess)
39883954
& ~ParentFlags.IgnoreResult & ~ParentFlags.DupMemberOwner;
39893955

3990-
if (!TryEmit(instanceExpr, paramExprs, il, ref closure, setup, p, -1/*pi*/))
3956+
if (!TryEmit(instanceExpr, paramExprs, il, ref closure, setup, p, -1))
39913957
return false;
39923958

39933959
if ((parent & ParentFlags.DupMemberOwner) != 0)
@@ -4322,24 +4288,17 @@ private static bool TryEmitComparison(Expression exprLeft, Expression exprRight,
43224288
(leftOpType == typeof(object) || rightOpType == typeof(object)))
43234289
{
43244290
if (expressionType == ExpressionType.Equal)
4325-
{
43264291
il.Emit(OpCodes.Ceq);
4327-
if ((parent & ParentFlags.IgnoreResult) != 0)
4328-
il.Emit(OpCodes.Pop);
4329-
}
43304292
else if (expressionType == ExpressionType.NotEqual)
43314293
{
43324294
il.Emit(OpCodes.Ceq);
4333-
il.Emit(OpCodes.Ldc_I4_0); // todo: @perf may there is optimization for comparison with IL, like the `Bne_Un`
4295+
il.Emit(OpCodes.Ldc_I4_0); // todo: @perf Currently it produces the same code a System Compile but I wonder if we can use OpCodes.Not
43344296
il.Emit(OpCodes.Ceq);
43354297
}
43364298
else
43374299
return false;
43384300

4339-
if ((parent & ParentFlags.IgnoreResult) != 0)
4340-
il.Emit(OpCodes.Pop);
4341-
4342-
return true;
4301+
return il.EmitPopIfIgnoreResult(parent);
43434302
}
43444303
}
43454304

@@ -4395,10 +4354,7 @@ var methodName
43954354
if (leftIsNullable)
43964355
goto nullCheck;
43974356

4398-
if ((parent & ParentFlags.IgnoreResult) != 0)
4399-
il.Emit(OpCodes.Pop);
4400-
4401-
return true;
4357+
return il.EmitPopIfIgnoreResult(parent);
44024358
}
44034359

44044360
// handle primitives comparison
@@ -4409,7 +4365,7 @@ var methodName
44094365
break;
44104366

44114367
case ExpressionType.NotEqual:
4412-
il.Emit(OpCodes.Ceq); // todo: @perf optimize the IL chain
4368+
il.Emit(OpCodes.Ceq);
44134369
il.Emit(OpCodes.Ldc_I4_0);
44144370
il.Emit(OpCodes.Ceq);
44154371
break;
@@ -4423,22 +4379,23 @@ var methodName
44234379
break;
44244380

44254381
case ExpressionType.GreaterThanOrEqual:
4426-
case ExpressionType.LessThanOrEqual:
4427-
var ifTrueLabel = il.DefineLabel();
4428-
if (rightOpType == typeof(uint) || rightOpType == typeof(ulong) ||
4429-
rightOpType == typeof(ushort) || rightOpType == typeof(byte))
4430-
il.Emit(expressionType == ExpressionType.GreaterThanOrEqual ? OpCodes.Bge_Un_S : OpCodes.Ble_Un_S, ifTrueLabel);
4431-
else
4432-
il.Emit(expressionType == ExpressionType.GreaterThanOrEqual ? OpCodes.Bge_S : OpCodes.Ble_S, ifTrueLabel);
4433-
4382+
// simplifying by using the LessThen (Clt) and comparing with negative outcome (Ceq 0)
4383+
if (leftOpType.IsUnsigned() && rightOpType.IsUnsigned())
4384+
il.Emit(OpCodes.Clt_Un);
4385+
else
4386+
il.Emit(OpCodes.Clt);
44344387
il.Emit(OpCodes.Ldc_I4_0);
4435-
var doneLabel = il.DefineLabel();
4436-
il.Emit(OpCodes.Br_S, doneLabel);
4437-
4438-
il.MarkLabel(ifTrueLabel);
4439-
il.Emit(OpCodes.Ldc_I4_1);
4388+
il.Emit(OpCodes.Ceq);
4389+
break;
44404390

4441-
il.MarkLabel(doneLabel);
4391+
case ExpressionType.LessThanOrEqual:
4392+
// simplifying by using the GreaterThen (Cgt) and comparing with negative outcome (Ceq 0)
4393+
if (leftOpType.IsUnsigned() && rightOpType.IsUnsigned())
4394+
il.Emit(OpCodes.Cgt_Un);
4395+
else
4396+
il.Emit(OpCodes.Cgt);
4397+
il.Emit(OpCodes.Ldc_I4_0);
4398+
il.Emit(OpCodes.Ceq);
44424399
break;
44434400

44444401
default:
@@ -4486,10 +4443,7 @@ var methodName
44864443
}
44874444
}
44884445

4489-
if ((parent & ParentFlags.IgnoreResult) != 0)
4490-
il.Emit(OpCodes.Pop);
4491-
4492-
return true;
4446+
return il.EmitPopIfIgnoreResult(parent);
44934447
}
44944448

44954449
#if LIGHT_EXPRESSION
@@ -5048,7 +5002,10 @@ private static int EmitStoreLocalVariableAndLoadItsAddress(ILGenerator il, Type
50485002
internal static class Tools
50495003
{
50505004
internal static bool IsUnsigned(this Type type) =>
5051-
type == typeof(byte) || type == typeof(ushort) || type == typeof(uint) || type == typeof(ulong);
5005+
type == typeof(byte) ||
5006+
type == typeof(ushort) ||
5007+
type == typeof(uint) ||
5008+
type == typeof(ulong);
50525009

50535010
internal static bool IsNullable(this Type type) =>
50545011
type.IsGenericType && type.GetGenericTypeDefinition() == typeof(Nullable<>);
@@ -5352,11 +5309,6 @@ static ILGeneratorHacks()
53525309
// our own helper - always available
53535310
var postIncMethod = typeof(ILGeneratorHacks).GetTypeInfo().GetDeclaredMethod(nameof(PostInc));
53545311

5355-
// now let's compile the following method without allocating the LocalBuilder class:
5356-
/*
5357-
il.m_localSignature.AddArgument(type);
5358-
return PostInc(ref il.LocalCount);
5359-
*/
53605312
var efficientMethod = new DynamicMethod(string.Empty,
53615313
typeof(int), new[] { typeof(ExpressionCompiler.ArrayClosure), typeof(ILGenerator), typeof(Type) },
53625314
typeof(ExpressionCompiler.ArrayClosure), skipVisibility: true);
@@ -5498,13 +5450,15 @@ internal static StringBuilder ToExpressionString(this ParameterExpression pe, St
54985450
}
54995451

55005452
internal static StringBuilder ToExpressionString(this LabelTarget lt, StringBuilder sb, List<LabelTarget> labelTargets,
5501-
bool stripNamespace = false, Func<Type, string, string> printType = null)
5453+
int lineIdent = 0, bool stripNamespace = false, Func<Type, string, string> printType = null)
55025454
{
55035455
var i = labelTargets.Count - 1;
55045456
while (i != -1 && !ReferenceEquals(labelTargets[i], lt)) --i;
55055457
if (i != -1)
5506-
return sb.Append("l[").Append(i).Append("]/* ").AppendName(lt.Name, lt.Type, lt).Append(" */");
5507-
5458+
return sb.Append("l[").Append(i)
5459+
.Append(" // (").AppendName(lt.Name, lt.Type, lt).Append(')')
5460+
.NewLineIdent(lineIdent).Append(']');
5461+
55085462
labelTargets.Add(lt);
55095463
sb.Append("l[").Append(labelTargets.Count - 1).Append("]=Label(");
55105464
sb.AppendTypeof(lt.Type, stripNamespace, printType);
@@ -5666,7 +5620,7 @@ internal static StringBuilder CreateExpressionString(this Expression e, StringBu
56665620
if (args.Count == 0 && e.Type.IsValueType)
56675621
return sb.Append("New(").AppendTypeof(e.Type, stripNamespace, printType).Append(')');
56685622

5669-
sb.Append("New(/*").Append(args.Count).Append(" args*/");
5623+
sb.Append("New( // ").Append(args.Count).Append(" args");
56705624
var ctorIndex = x.Constructor.DeclaringType.GetTypeInfo().DeclaredConstructors.ToArray().GetFirstIndex(x.Constructor);
56715625
sb.NewLineIdent(lineIdent).AppendTypeof(x.Type, stripNamespace, printType)
56725626
.Append(".GetTypeInfo().DeclaredConstructors.ToArray()[").Append(ctorIndex).Append("],");
@@ -5785,10 +5739,10 @@ internal static StringBuilder CreateExpressionString(this Expression e, StringBu
57855739
sb.NewLineIdentExpr(x.Body, paramsExprs, uniqueExprs, lts, lineIdent, stripNamespace, printType, identSpaces);
57865740

57875741
if (x.BreakLabel != null)
5788-
x.BreakLabel.ToExpressionString(sb.Append(',').NewLineIdent(lineIdent), lts, stripNamespace, printType);
5742+
x.BreakLabel.ToExpressionString(sb.Append(',').NewLineIdent(lineIdent), lts, lineIdent, stripNamespace, printType);
57895743

57905744
if (x.ContinueLabel != null)
5791-
x.ContinueLabel.ToExpressionString(sb.Append(',').NewLineIdent(lineIdent), lts, stripNamespace, printType);
5745+
x.ContinueLabel.ToExpressionString(sb.Append(',').NewLineIdent(lineIdent), lts, lineIdent, stripNamespace, printType);
57925746

57935747
return sb.Append(')');
57945748
}
@@ -5836,7 +5790,7 @@ internal static StringBuilder CreateExpressionString(this Expression e, StringBu
58365790
{
58375791
var x = (LabelExpression)e;
58385792
sb.Append("Label(");
5839-
x.Target.ToExpressionString(sb, lts, stripNamespace, printType);
5793+
x.Target.ToExpressionString(sb, lts, lineIdent, stripNamespace, printType);
58405794
if (x.DefaultValue != null)
58415795
sb.Append(',').NewLineIdentExpr(x.DefaultValue, paramsExprs, uniqueExprs, lts, lineIdent, stripNamespace, printType, identSpaces);
58425796
return sb.Append(')');
@@ -5847,7 +5801,7 @@ internal static StringBuilder CreateExpressionString(this Expression e, StringBu
58475801
sb.Append("MakeGoto(").AppendEnum(x.Kind, stripNamespace, printType).Append(',');
58485802

58495803
sb.NewLineIdent(lineIdent);
5850-
x.Target.ToExpressionString(sb, lts, stripNamespace, printType).Append(',');
5804+
x.Target.ToExpressionString(sb, lts, lineIdent, stripNamespace, printType).Append(',');
58515805

58525806
sb.NewLineIdentExpr(x.Value, paramsExprs, uniqueExprs, lts, lineIdent, stripNamespace, printType, identSpaces).Append(',');
58535807
sb.NewLineIdent(lineIdent).AppendTypeof(x.Type, stripNamespace, printType);

0 commit comments

Comments
 (0)