Skip to content
This repository was archived by the owner on Jan 23, 2023. It is now read-only.

Spill side-effects in impAssignMultiRegTypeToVar.#11077

Merged
pgavlin merged 2 commits intodotnet:masterfrom
pgavlin:gh10940
Apr 19, 2017
Merged

Spill side-effects in impAssignMultiRegTypeToVar.#11077
pgavlin merged 2 commits intodotnet:masterfrom
pgavlin:gh10940

Conversation

@pgavlin
Copy link

@pgavlin pgavlin commented Apr 19, 2017

This importer function converts IR from the form

    tree

to

            /- tree
    STMT - =
            \- lclVar

in order to conform to the JIT's representation of multi-register values.
Today, it does not check whether or not the tree being spilled has any
side-effects that may interfere with the side-effects of trees that are
already on the evaluation stack, which can lead to bad codegen. This
change fixes this function to spill any side-effects on the stack
before generating the temp assign.

Fixes #10940.

This importer function converts IR from the form

    tree

to
            /- tree
    STMT - =
            \- lclVar

in order to conform to the JIT's representation of multi-register values.
Today, it does not check whether or not the tree being spilled has any
side-effects that may interfere with the side-effects of trees that are
already on the evaluation stack, which can lead to bad codegen. This
change fixes this function to spill any side-effects on the stack
before generating the temp assign.

Fixes #10940.
@pgavlin
Copy link
Author

pgavlin commented Apr 19, 2017

@briansull @CarolEidt @dotnet/jit-contrib PTAL

@AndyAyersMS
Copy link
Member

Would be a good idea to check that we're not breaking trees in such a way as to reintroduce cases of #9562.

You should be able to spot the broken opts, if any, from jit-diffs on corelib.

Also, please add a test case.

@pgavlin
Copy link
Author

pgavlin commented Apr 19, 2017

Also, please add a test case.

Yes, I'm working on one.

@briansull
Copy link

LGTM

@pgavlin
Copy link
Author

pgavlin commented Apr 19, 2017

@AndyAyersMS: there is one diff in S.P.CoreLib on Linux and OS X. It is in EventSource:CreateManifestAndDescriptors:

 G_M43523_IG10:
        488BFB               mov      rdi, rbx
-       E800000000           call     EventSource:GetGuid(ref):struct
-       48898550FFFFFF       mov      qword ptr [rbp-B0H], rax
-       48899558FFFFFF       mov      qword ptr [rbp-A8H], rdx
-       488BFB               mov      rdi, rbx
        8B75D4               mov      esi, dword ptr [rbp-2CH]
        E800000000           call     EventSource:GetName(ref,int):ref
        488985B0FEFFFF       mov      gword ptr [rbp-150H], rax
+       488BFB               mov      rdi, rbx
+       E800000000           call     EventSource:GetGuid(ref):struct
+       48898550FFFFFF       mov      qword ptr [rbp-B0H], rax
+       48899558FFFFFF       mov      qword ptr [rbp-A8H], rdx
        488D3D00000000       lea      rdi, [(reloc)]
        E800000000           call     CORINFO_HELP_NEWSFAST
        488B95B0FEFFFF       mov      rdx, gword ptr [rbp-150H]
        F30F6F8550FFFFFF     movdqu   xmm0, qword ptr [rbp-B0H]
        F30F7F8530FFFFFF     movdqu   qword ptr [rbp-D0H], xmm0
        8B7DD4               mov      edi, dword ptr [rbp-2CH]
        893C24               mov      dword ptr [rsp], edi
        488985A8FEFFFF       mov      gword ptr [rbp-158H], rax
        488BF8               mov      rdi, rax
        488BF2               mov      rsi, rdx
        488B9530FFFFFF       mov      rdx, qword ptr [rbp-D0H]
        488B8D38FFFFFF       mov      rcx, qword ptr [rbp-C8H]
        4D8BC7               mov      r8, r15
        4C8B8D08FFFFFF       mov      r9, gword ptr [rbp-F8H]
        E800000000           call     ManifestBuilder:.ctor(ref,struct,ref,ref,int):this

It corresponds to this source code:

                manifest = new ManifestBuilder(GetName(eventSourceType, flags), GetGuid(eventSourceType), eventSourceDllName,
                                               resources, flags);

So this diff is expected: the baseline JIT was incorrectly reordering the calls to GetName and GetGuid.

@AndyAyersMS
Copy link
Member

Thanks for checking.

Changes LGTM too.

@pgavlin pgavlin added this to the 2.0.0 milestone Apr 19, 2017
@pgavlin pgavlin merged commit 6d073e1 into dotnet:master Apr 19, 2017
@pgavlin pgavlin deleted the gh10940 branch April 19, 2017 22:16
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[osx.10.12-x64/release] Parameters are not evaluated from left to right on OSX in release mode.

4 participants