Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Linker substitutions don't optimize as much as they can #1106

Closed
rolfbjarne opened this issue Apr 16, 2020 · 3 comments
Closed

Linker substitutions don't optimize as much as they can #1106

rolfbjarne opened this issue Apr 16, 2020 · 3 comments

Comments

@rolfbjarne
Copy link
Member

Sample code:

static void Optimizable ()
{
	if (IntPtr.Size == 8)
		Console.WriteLine ("64bit");
	else
		Console.WriteLine ("32bit");
}

Substitution xml:

<linker>
  <assembly fullname="System.Private.CoreLib">
    <type fullname="System.IntPtr">
      <method signature="System.Int32 get_Size()" body="stub" value="8" />
    </type>
  </assembly>
</linker>

Initial IL:

default void Optimizable ()  cil managed 
    {
	.maxstack 2
	.locals init (
		bool	V_0)
	IL_0000:  nop 
	IL_0001:  call int32 native int::get_Size()
	IL_0006:  ldc.i4.8 
	IL_0007:  ceq 
	IL_0009:  stloc.0 
	IL_000a:  ldloc.0 
	IL_000b:  brfalse.s IL_001a

	IL_000d:  ldstr "64bit"
	IL_0012:  call void class [mscorlib]System.Console::WriteLine(string)
	IL_0017:  nop 
	IL_0018:  br.s IL_0025

	IL_001a:  ldstr "32bit"
	IL_001f:  call void class [mscorlib]System.Console::WriteLine(string)
	IL_0024:  nop 
	IL_0025:  ret 
    } // end of method Application::Optimizable

Post-linked IL:

default void Optimizable ()  cil managed 
    {
	.maxstack 2
	.locals init (
		bool	V_0)
	IL_0000:  nop 
	IL_0001:  call int32 class [System.Private.CoreLib]System.IntPtr::get_Size()
	IL_0006:  ldc.i4.8 
	IL_0007:  ceq 
	IL_0009:  stloc.0 
	IL_000a:  ldloc.0 
	IL_000b:  pop 
	IL_000c:  ldstr "64bit"
	IL_0011:  call void class [mscorlib]System.Console::WriteLine(string)
	IL_0016:  nop 
	IL_0017:  br.s IL_0019

	IL_0019:  ret 
    } // end of method Application::Optimizable

it seems the optimized C# would be:

static void Optimizable ()
{
	if (IntPtr.Size == 8)
		Console.WriteLine ("64bit");
}

But we can do better, I want the optimized C# to look like this:

static void Optimizable ()
{
	Console.WriteLine ("64bit");
}

Desired IL:

default void Optimized ()  cil managed 
    {
	.maxstack 8
	IL_0000:  ldstr "64bit"
	IL_0005:  call void class [mscorlib]System.Console::WriteLine(string)
	IL_000c:  ret 
    } // end of method Application::Optimized
@marek-safar
Copy link
Contributor

This is not the purpose of this feature/setting. It uses constant propagation to remove unreachable code to reduce the size, it does not do inlining or any other IL optimizations. There are a few reasons for that.

  • It has no impact on perf as the AOT/JIT will remove the code when it's really possible (the rules are not simple).
  • The pass can be used when the output is not the final self-contained container (e.g. you are building lib with ref assemblies)
  • Developers should still have a way to debug the substituted code.
  • There is consistent behaviour for substitution of methods and fields.

If you use it with release not debug csc mode the code after linking will look like this

    call       int32 [System.Private.CoreLib]System.IntPtr::get_Size()
    pop
    ldstr      "64bit"
    call       void [System.Console]System.Console::WriteLine(string)
    ret

We might add an additional pass if this turns in noticeable size saving but right now there are bigger fish to fry.

@tannergooding
Copy link
Member

tannergooding commented Feb 5, 2021

@marek-safar, this ends up leaving a number of otherwise "dead" types in the IL as well as the supporting IL for calling those methods (a minimum of 6 bytes of IL per call):
image

I think it would be good to actually remove these calls when we know they only return a constant value so we can do further downstream trimming.

@marek-safar
Copy link
Contributor

This has been implemented in net7

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants