Skip to content

JIT: consider optimizing constructor pattern for struct creation #9237

Open

Description

@AndyAyersMS

Currently if you newobj a struct via a constructor call the jit assumes the struct's old value may be read during the construction of the new value. So the pattern emitted is:

;; zero temp obj storage
;; call constructor on temp obj
;; copy temp obj to the destination

When the final destination is register promoted all this tends to get cleaned up, but when the final destination can't be register promoted the code the jit produces can be surprisingly large and slow.

For instance, given

ValueTask<int> Work() => new ValueTask<int>(42);
ValueTaskAwaiter<int> vt = Work().GetAwaiter();
  ... vt.GetResult();

The jit may produce code like:

       33C9                 xor      rcx, rcx
       B82A000000           mov      eax, 42
       488D542420           lea      rdx, bword ptr [rsp+20H]
       C4E17957C0           vxorpd   xmm0, xmm0
       C4E17A7F02           vmovdqu  qword ptr [rdx], xmm0
       488D542420           lea      rdx, bword ptr [rsp+20H]
       48890A               mov      gword ptr [rdx], rcx
       894208               mov      dword ptr [rdx+8], eax
       C4E17A6F442420       vmovdqu  xmm0, qword ptr [rsp+20H]
       C4E17A7F442430       vmovdqu  qword ptr [rsp+30H], xmm0

In most cases the aliasing the jit is worried about does not happen, and the new value could be constructed directly into the destination. And also in most cases this construction will fully write the fields of the struct so the zero initialization is not necessary.

category:cq
theme:structs
skill-level:expert
cost:large
impact:large

Activity

  1. benaadams commented on Nov 8, 2017

    @benaadams
    Member

    And also in most cases this construction will fully write the fields of the struct so the zero initialization is not necessary.

    Language-wise; C# for example, forces complete initialization of all fields in the .ctor

    I assume the object initialization pattern would be a wrinkle

    var v = new Vertex { uv = new Vector2(0, 0) };

    And require zero init + assignment; rather than just assignment? (Though shouldn't block operating directly on destination)

  2. ygc369 commented on Nov 8, 2017

    @ygc369

    @AndyAyersMS
    I think C# should allow to use C-style struct constructor to tell the compiler explicitly to initialize a struct just in place.
    For example:

    struct A 
    {
        public int field;
        A(int v) { this.field = v; }
    };
    
    static void test()
    {
        A a(20);  //C-style constructor
        ......
    }
  3. benaadams commented on Nov 8, 2017

    @benaadams
    Member

    @ygc369 you'd need to file that in the https://github.com/dotnet/csharplang repository as its language syntax rather than runtime

    For same effect can currently do

    var a = new A(20);

    The issue I was seeing with object initialization is

    var a = new A { field = 20 };

    Doesn't need to fully initialize all the fields; so if it was a struct of 20 fields, can init 5 for example. However its probably faster to zero init all of them contiguously and then set the 5 rather than intermingling zeros and values for a patterned initialize.

  4. AndyAyersMS commented on Nov 8, 2017

    @AndyAyersMS
    MemberAuthor

    Didn't mean to imply that the zero init is never necessary, just that frequently the subsequent code overwrites all the zeros and so (modulo some assumptions about GC safe points) those zero init stores are dead.

    For zero init there is also the question of whether you can zero field by field or some kind of whole struct block init, and whether or not you should zero any padding voids.

  5. benaadams commented on Nov 8, 2017

    @benaadams
    Member

    Was just highlighting maybe a useful corner case example.

    I'd love to see this optimization as its a pretty common pattern when using a struct .ctor (with params)

  6. ygc369 commented on Nov 16, 2017

    @ygc369

    @AndyAyersMS
    Very happy to see this optimization, structs should be initialized just in place, no copying needed.
    When would it be done?

  7. AndyAyersMS commented on Nov 16, 2017

    @AndyAyersMS
    MemberAuthor

    Hard to say for sure when we might get to this. I am hoping maybe we can address some subset of cases relatively soon. The general solution likely requires first-class structs and new code to perform liveness on entire structs.

    The challenge is that to ensure correct semantics we need to show that for struct-typed temp and dest

    temp = 0
    ctor(&temp, ...)
    dest = temp
    

    gives the same result as

    ctor(&dest, ...)
    

    and that crucially depends on what has happened to dest in the past, what is in the ... and what ctor does. The worry is that somehow the values in dest might be looked at when computing the values for temp, and so the optimized version, which effectively interleaves construction and the copy could change the values that end up in dest.

    Because of the need to reason about past behavior it is not easy to fix this as the IL is imported. Instead the jit must produce the general pattern with the temp and then sometime later go back and clean it up if possible. However there might be some cases in the importer where we can say for sure that the old values can't be read and produce the optimized pattern directly.

  8. AndyAyersMS commented on May 7, 2018

    @AndyAyersMS
    MemberAuthor

    #8891 (reverse copy prop of structs) has similar ambitions -- this issue is more about the timing and placement.

    It would be preferable to catch (at least some cases of) this early and never introduce the temp, instead of introducing the temp and then optimizing away its use.

    Among other things this pattern often appears in initializer type methods and so causes extremely bloated codegen in debug/minopts.

  9. transferred this issue fromdotnet/coreclron Jan 31, 2020
  10. added this to the Future milestone on Jan 31, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    area-CodeGen-coreclrCLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI

    Type

    No type

    Projects

    No projects

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions