Skip to content
This repository was archived by the owner on Dec 20, 2019. It is now read-only.

Conversation

@lateralusX
Copy link
Member

No description provided.

@lateralusX lateralusX requested a review from alexanderkyte March 7, 2019 10:51
Copy link
Collaborator

@alexanderkyte alexanderkyte left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM as long as it builds on non-Windows platforms correctly.

@lateralusX
Copy link
Member Author

Run it using a PR build and the LLVM builds looked OK after this commit, without it we have build errors on other platforms.

@lateralusX lateralusX merged commit 286f431 into mono:release_60 Mar 8, 2019
lateralusX pushed a commit to lateralusX/llvm that referenced this pull request Mar 8, 2019
Mono's LLVM backend was always emitting code like this:

> 000d7234 <double_ToString>:
>    d7234:       e92d4800        push    {fp, lr}
>    d7238:       ed2d8b02        vpush   {d8}
>    d723c:       ed908b00        vldr    d8, [r0]
>    d7240:       eb12e4e2        bl      5905d0 <plt_System_Globalization_NumberFormatInfo_get_CurrentInfo>
>    d7244:       e1a03000        mov     r3, r0
>    d7248:       ec510b18        vmov    r0, r1, d8
>    d724c:       e3a02000        mov     r2, #0
>    d7250:       eb12e91a        bl      5916c0 <plt_System_Number_FormatDouble_double_string_System_Globalization_NumberFormatInfo>
>    d7254:       ecbd8b02        vpop    {d8}
>    d7258:       e8bd8800        pop     {fp, pc}

despite the correct setting passed to `llc`. Note that passing the floating
point value (`d8`) in the integer registers `r0` and `r1` is wrong.

With this change we get the following:

> 000d9934 <double_ToString>:
>    d9934:       e92d4800        push    {fp, lr}
>    d9938:       ed2d8b02        vpush   {d8}
>    d993c:       ed908b00        vldr    d8, [r0]
>    d9940:       eb137c2e        bl      5b8a00 <plt_System_Globalization_NumberFormatInfo_get_CurrentInfo>
>    d9944:       eeb00b48        vmov.f64        d0, d8
>    d9948:       e1a01000        mov     r1, r0
>    d994c:       e3a00000        mov     r0, #0
>    d9950:       eb138066        bl      5b9af0 <plt_System_Number_FormatDouble_double_string_System_Globalization_NumberFormatInfo>
>    d9954:       ecbd8b02        vpop    {d8}
>    d9958:       e8bd4800        pop     {fp, lr}
>    d995c:       e1a0f00e        mov     pc, lr

Which matches with what the Mono JIT emits:

>   0:   e92d4100        push    {r8, lr}
>   4:   e24dd028        sub     sp, sp, mono#40     ; 0x28
>   8:   e58d0018        str     r0, [sp, mono#24]
>   c:   e59d0018        ldr     r0, [sp, mono#24]
>  10:   ed900b00        vldr    d0, [r0]
>  14:   ed8d0b08        vstr    d0, [sp, mono#32]
>  18:   eb00000e        bl      0x58
>  1c:   e1a01000        mov     r1, r0
>  20:   ed9d0b08        vldr    d0, [sp, mono#32]
>  24:   e3a00000        mov     r0, #0
>  28:   eb000007        bl      0x4c
>  2c:   e28dd028        add     sp, sp, mono#40     ; 0x28
>  30:   e8bd8100        pop     {r8, pc}

Both are passing the floating point argument correctly via `d0`.

Fixes mono/mono#11095
vargaz pushed a commit that referenced this pull request Oct 1, 2019
Mono's LLVM backend was always emitting code like this:

> 000d7234 <double_ToString>:
>    d7234:       e92d4800        push    {fp, lr}
>    d7238:       ed2d8b02        vpush   {d8}
>    d723c:       ed908b00        vldr    d8, [r0]
>    d7240:       eb12e4e2        bl      5905d0 <plt_System_Globalization_NumberFormatInfo_get_CurrentInfo>
>    d7244:       e1a03000        mov     r3, r0
>    d7248:       ec510b18        vmov    r0, r1, d8
>    d724c:       e3a02000        mov     r2, #0
>    d7250:       eb12e91a        bl      5916c0 <plt_System_Number_FormatDouble_double_string_System_Globalization_NumberFormatInfo>
>    d7254:       ecbd8b02        vpop    {d8}
>    d7258:       e8bd8800        pop     {fp, pc}

despite the correct setting passed to `llc`. Note that passing the floating
point value (`d8`) in the integer registers `r0` and `r1` is wrong.

With this change we get the following:

> 000d9934 <double_ToString>:
>    d9934:       e92d4800        push    {fp, lr}
>    d9938:       ed2d8b02        vpush   {d8}
>    d993c:       ed908b00        vldr    d8, [r0]
>    d9940:       eb137c2e        bl      5b8a00 <plt_System_Globalization_NumberFormatInfo_get_CurrentInfo>
>    d9944:       eeb00b48        vmov.f64        d0, d8
>    d9948:       e1a01000        mov     r1, r0
>    d994c:       e3a00000        mov     r0, #0
>    d9950:       eb138066        bl      5b9af0 <plt_System_Number_FormatDouble_double_string_System_Globalization_NumberFormatInfo>
>    d9954:       ecbd8b02        vpop    {d8}
>    d9958:       e8bd4800        pop     {fp, lr}
>    d995c:       e1a0f00e        mov     pc, lr

Which matches with what the Mono JIT emits:

>   0:   e92d4100        push    {r8, lr}
>   4:   e24dd028        sub     sp, sp, #40     ; 0x28
>   8:   e58d0018        str     r0, [sp, #24]
>   c:   e59d0018        ldr     r0, [sp, #24]
>  10:   ed900b00        vldr    d0, [r0]
>  14:   ed8d0b08        vstr    d0, [sp, #32]
>  18:   eb00000e        bl      0x58
>  1c:   e1a01000        mov     r1, r0
>  20:   ed9d0b08        vldr    d0, [sp, #32]
>  24:   e3a00000        mov     r0, #0
>  28:   eb000007        bl      0x4c
>  2c:   e28dd028        add     sp, sp, #40     ; 0x28
>  30:   e8bd8100        pop     {r8, pc}

Both are passing the floating point argument correctly via `d0`.

Fixes mono/mono#11095
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.

2 participants