roslyn: Multiple `+ sizeof()` not being compiled down to a single add instruction in conjunction with an argument

Version Used: should be the latest

Steps to Reproduce:

https://sharplab.io/#v2:EYLgtghgzgLgpgJwDQxASwDYB8ACAGAAhwEYBuAWACgcBmIgJgIGECBvKgzoutAOxmYQMAYwAUfAbwCUbDl3k4A7AV4EA1AShoAXnAD2AM1HAAnvBkatuw6ICuUABZ6EMC5p36jEqRUryAvlT+QA

  1. Write some code
    public int Calc(int n) {
        return n + sizeof(byte) + sizeof(ushort) + sizeof(int);
    }
  1. ildasm it
    // Methods
    .method public hidebysig 
        instance int32 Calc (
            int32 n
        ) cil managed 
    {
        // Method begins at RVA 0x2050
        // Code size 8 (0x8)
        .maxstack 8

        IL_0000: ldarg.1
        IL_0001: ldc.i4.1
        IL_0002: add
        IL_0003: ldc.i4.2
        IL_0004: add
        IL_0005: ldc.i4.4
        IL_0006: add
        IL_0007: ret
    } // end of method C::Calc
  1. see the multiple adds and be sad

Expected Behavior:

    // Methods
    .method public hidebysig 
        instance int32 Calc (
            int32 n
        ) cil managed 
    {
        // Method begins at RVA 0x2050
        // Code size 8 (0x8)
        .maxstack 8

        IL_0000: ldarg.1
        IL_0001: ldc.i4.7
        IL_0002: add
        IL_0007: ret
    } // end of method C::Calc

Actual Behavior:

    // Methods
    .method public hidebysig 
        instance int32 Calc (
            int32 n
        ) cil managed 
    {
        // Method begins at RVA 0x2050
        // Code size 8 (0x8)
        .maxstack 8

        IL_0000: ldarg.1
        IL_0001: ldc.i4.1
        IL_0002: add
        IL_0003: ldc.i4.2
        IL_0004: add
        IL_0005: ldc.i4.4
        IL_0006: add
        IL_0007: ret
    } // end of method C::Calc

thankfully the JIT ASM is properly adding 0x7

C.Calc(Int32)
    L0000: mov eax, edx
    L0002: add eax, 0x7
    L0005: ret

About this issue

  • Original URL
  • State: closed
  • Created 6 years ago
  • Comments: 34 (34 by maintainers)

Most upvoted comments

The operator is left-associative, so the correct expression binding here is ((((n + sizeof(byte)) + sizeof(ushort)) + sizeof(int)).

Ah, I see; and indeed, if we change it to return sizeof(byte) + sizeof(ushort) + sizeof(int) + n;, the compiler folds the expression to 7 + n.

Maybe it would be worth updating the analyzers/code-fixes to take this into account…

The JIT has to do everything at runtime

This is not true.

Yes it is. The JIT only happens at runtime. There may be other intermediate processes that can run before (like ngen, crossgen, aot, etc) and which can pass additional information to the JIT; but when it comes to the JIT itself, it has to take exactly what it was given and work with it at runtime.

There is no reason there can’t be an IL compiler that does things like this.

For some optimizations, yes. I completely agree that having an intermediate and dedicated ilopt.exe would be great. It should take the IL and do analysis on it. It should do all the simple optimizations (like constant folding) and it should try to do more expensive analysis and leave hints/breadcrumbs to the JIT where possible. However, we have no such team to do that today and not all optimizations are possible to do at this level.

  • A valid F# optimization is not necessarily a valid C# optimization (and vice versa). F#, for example, supports method inlining at compile time. C# does not.
  • Sometimes, the best optimizations are made when you have the full context of the original source file

Why does the compiler need to do anything here given that it won’t have any affect on the final code that is executed? Any effort here would be time not spent on things that would have a more positive effect on an application?

@CyrusNajmabadi, because it can impact multiple things downstream in the runtime/JIT. The JIT, due to time constraints/etc has to take a very narrow view of things when determining inlining and certain other optimizations (unlike an AOT compiler, it doesn’t have the ability to do all the analysis itself).

If the C# compiler has a case where it can produce better/smaller IL and remove the need for the JIT to do the additional analysis/etc, then it can have a positive impact downstream (the method has a higher chance of being inlined, the JIT doesn’t need to produce as many nodes, it doesn’t have to fold the values together, itself etc). This also means that, since the JIT doesn’t have to do that work itself, it can instead spend that time on optimizations that the C# compiler can’t or won’t make.

This seems like a farily trivial case and something that the compiler already should be handling (and it indeed does in some cases).

@SirJosh3917 Constant calculation can be forced via:

    public int Calc(int n) {
        const int size = sizeof(byte) + sizeof(short) + sizeof(int);
        return n + size;
    }