Skip to content

Commit

Permalink
Consider side effects when evaluating manually substituted methods (#…
Browse files Browse the repository at this point in the history
  • Loading branch information
marek-safar committed May 19, 2022
1 parent 8e738bb commit 55c9a27
Show file tree
Hide file tree
Showing 4 changed files with 180 additions and 27 deletions.
24 changes: 11 additions & 13 deletions src/linker/Linker.Steps/UnreachableBlocksOptimizer.cs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
// Copyright (c) .NET Foundation and contributors. All rights reserved.
// Copyright (c) .NET Foundation and contributors. All rights reserved.
// Licensed under the MIT license. See LICENSE file in the project root for full license information.

using System;
Expand Down Expand Up @@ -202,7 +202,7 @@ static bool IsComparisonAlwaysTrue (OpCode opCode, int left, int right)
return null;
case MethodAction.ConvertToStub:
Instruction? constant = CodeRewriterStep.CreateConstantResultInstruction (_context, method);
return constant == null ? null : new MethodResult (constant, true);
return constant == null ? null : new MethodResult (constant, !HasSideEffects (method));
}

if (method.IsIntrinsic () || method.NoInlining)
Expand All @@ -212,13 +212,16 @@ static bool IsComparisonAlwaysTrue (OpCode opCode, int left, int right)
return null;

var analyzer = new ConstantExpressionMethodAnalyzer (this);
if (analyzer.Analyze (callee, callStack)) {
if (analyzer.Analyze (callee, callStack))
return new MethodResult (analyzer.Result, analyzer.SideEffectFreeResult);
}

return null;
}

static bool HasSideEffects (MethodDefinition method)
{
return !method.DeclaringType.IsBeforeFieldInit;
}

//
// Return expression with a value when method implementation can be
Expand Down Expand Up @@ -457,15 +460,8 @@ public bool RewriteBody ()
if (call_result is not MethodResult result)
break;

if (!result.IsSideEffectFree)
break;

TypeDefinition methodType = md.DeclaringType;
if (methodType != body.Method.DeclaringType && !methodType.IsBeforeFieldInit) {
//
// Figuring out at this point if the explicit static ctor will be used is hard
//
optimizer._context.LogMessage ($"Cannot inline result of '{md.GetDisplayName ()}' call due to presence of static constructor");
if (!result.IsSideEffectFree) {
optimizer._context.LogMessage ($"Cannot inline constant result of '{md.GetDisplayName ()}' call due to presence of side effects");
break;
}

Expand Down Expand Up @@ -1401,6 +1397,8 @@ public bool Analyze (in CalleePayload callee, Stack<MethodDefinition> callStack)
Instruction? linstr;
object? left, right, operand;

SideEffectFreeResult = !HasSideEffects (method);

//
// We could implement a full-blown interpreter here but for now, it handles
// cases used in runtime libraries
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,24 +15,11 @@ namespace Mono.Linker.Tests.Cases.FeatureSettings
[SetupLinkerArgument ("--feature", "MethodCondition", "false")]
[SetupLinkerArgument ("--feature", "FieldCondition", "true")]
[SetupLinkerArgument ("--feature", "ResourceCondition", "true")]
[SetupLinkerArgument ("--enable-opt", "ipconstprop")]
[RemovedResourceInAssembly ("test.exe", "ResourceFileRemoveWhenTrue.txt")]
[KeptResource ("ResourceFileRemoveWhenFalse.txt")]
public class FeatureSubstitutionsNested
{
[ExpectedInstructionSequence (new[] {
"nop",
"ldc.i4.1",
"pop",
"ldc.i4.0",
"pop",
"ldc.i4.1",
"pop",
"ldc.i4.0",
"pop",
"ldsfld System.Boolean Mono.Linker.Tests.Cases.FeatureSettings.FeatureSubstitutionsNested::FieldConditionField",
"pop",
"ret",
})]
public static void Main ()
{
GlobalConditionMethod ();
Expand All @@ -42,21 +29,41 @@ public static void Main ()
_ = FieldConditionField;
}

[Kept]
[ExpectedInstructionSequence (new[] {
"ldc.i4.1",
"ret",
})]
static bool GlobalConditionMethod ()
{
throw new NotImplementedException ();
}

[Kept]
[ExpectedInstructionSequence (new[] {
"ldc.i4.0",
"ret",
})]
static bool AssemblyConditionMethod ()
{
throw new NotImplementedException ();
}

[Kept]
[ExpectedInstructionSequence (new[] {
"ldc.i4.1",
"ret",
})]
static bool TypeConditionMethod ()
{
throw new NotImplementedException ();
}

[Kept]
[ExpectedInstructionSequence (new[] {
"ldc.i4.0",
"ret",
})]
static bool MethodConditionMethod ()
{
throw new NotImplementedException ();
Expand Down
136 changes: 136 additions & 0 deletions test/Mono.Linker.Tests.Cases/Substitutions/StubBodyWithStaticCtor.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,136 @@
using System;
using System.Runtime.CompilerServices;
using Mono.Linker.Tests.Cases.Expectations.Assertions;
using Mono.Linker.Tests.Cases.Expectations.Metadata;

namespace Mono.Linker.Tests.Cases.Substitutions
{
[SetupLinkerSubstitutionFile ("StubBodyWithStaticCtor.xml")]
[SetupCompileArgument ("/optimize+")]
[SetupLinkerArgument ("--enable-opt", "ipconstprop")]
public class StubBodyWithStaticCtor
{
public static void Main ()
{
TestMethod_1 ();
TestMethod_2 ();
TestMethod_3 ();
}

[Kept]
[ExpectedInstructionSequence (new[] {
"call System.Int32 Mono.Linker.Tests.Cases.Substitutions.StubBodyWithStaticCtorImpl::TestMethod()",
"ldc.i4.2",
"beq.s il_8",
"ldc.i4.3",
"ret",
})]
static int TestMethod_1 ()
{
if (StubBodyWithStaticCtorImpl.TestMethod () != 2) {
Console.WriteLine ();
return 1;
}

return 3;
}

[Kept]
[ExpectedInstructionSequence (new[] {
"call System.Int32 Mono.Linker.Tests.Cases.Substitutions.IntermediateClass::GetValue()",
"ldc.i4.2",
"beq.s il_8",
"ldc.i4.3",
"ret",
})]
static int TestMethod_2 ()
{
if (IntermediateClass.GetValue () != 2) {
Console.WriteLine ();
return 1;
}

return 3;
}

[Kept]
[ExpectedInstructionSequence (new[] {
"call System.Boolean Mono.Linker.Tests.Cases.Substitutions.WrappingClass::GetValue()",
"brfalse.s il_7",
"ldc.i4.3",
"ret",
})]
static int TestMethod_3 ()
{
if (WrappingClass.GetValue ()) {
Console.WriteLine ();
return 1;
}

return 3;
}
}

[Kept]
class WrappingClass
{
[Kept]
public static bool GetValue ()
{
return Settings.TestValue ();
}

static class Settings
{
[Kept]
static Settings ()
{
Console.WriteLine ();
}

[Kept]
[ExpectedInstructionSequence (new[] {
"ldc.i4.0",
"ret",
})]
public static bool TestValue ()
{
throw new NotImplementedException ();
}
}
}

[Kept]
class StubBodyWithStaticCtorImpl
{
[Kept]
public static int count;

[Kept]
static StubBodyWithStaticCtorImpl ()
{
count = 100;
}

[Kept]
[ExpectedInstructionSequence (new[] {
"ldc.i4 0x2",
"ret",
})]
public static int TestMethod ()
{
++count;
return Environment.ExitCode;
}
}

[Kept]
class IntermediateClass
{
[Kept]
public static int GetValue ()
{
return StubBodyWithStaticCtorImpl.TestMethod ();
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
<linker>
<assembly fullname="test, Version=0.0.0.0, Culture=neutral, PublicKeyToken=null">
<type fullname="Mono.Linker.Tests.Cases.Substitutions.StubBodyWithStaticCtorImpl">
<method signature="System.Int32 TestMethod()" body="stub" value="2">
</method>
</type>
<type fullname="Mono.Linker.Tests.Cases.Substitutions.WrappingClass/Settings">
<method signature="System.Boolean TestValue()" body="stub" value="false">
</method>
</type>
</assembly>
</linker>

0 comments on commit 55c9a27

Please sign in to comment.