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

[win/x86] (uint)double.MaxValue produces different value with optimizations on vs. off #13651

Closed
jkotas opened this issue Oct 26, 2019 · 9 comments
Assignees
Labels
area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI bug
Milestone

Comments

@jkotas
Copy link
Member

jkotas commented Oct 26, 2019

Repro:

using System;
using System.Threading;
using System.Runtime.CompilerServices;

class Program
{
    [MethodImpl(MethodImplOptions.NoInlining)]
    static void foo()
    {
        double x = double.MaxValue;
        Console.WriteLine((uint)x);
    }

    static void Main(string[] args)
    {
        for (; ; ) { foo(); Thread.Sleep(1); }
    }
}

Result

  • Prints 0s first and then changes to 4294967295 once tier 1 code kicks in

category:correctness
theme:optimization
skill-level:beginner
cost:small

@jkotas
Copy link
Member Author

jkotas commented Oct 26, 2019

cc @dotnet/jit-contrib

This is fallout from VS2017->VS2019 update for official builds. The bug is likely in gtFoldExprConst.

@jkotas
Copy link
Member Author

jkotas commented Oct 26, 2019

Disabling CoreFX tests against this issue: dotnet/corefx@f9ececd

@mikedn
Copy link
Contributor

mikedn commented Oct 26, 2019

Tier 0 code:

       C5FB1005883E7907 vmovsd   xmm0, qword ptr [@RWD00]
       83EC08       sub      esp, 8
       C5FB110424   vmovsd   qword ptr [esp], xmm0
       E8F97DA44D   call     CORINFO_HELP_DBL2UINT
       8BC8         mov      ecx, eax
       E8F6F9FFFF   call     System.Console:WriteLine(int)

Tier 1 code:

       B9FFFFFFFF   mov      ecx, -1
       E8DB90FFFF   call     System.Console:WriteLine(int)

The gtFoldExprConst code generated by VC++:

5415D6E6 F2 0F 10 45 F0       movsd       xmm0,mmword ptr [d1]  
5415D6EB E8 B5 7D 0F 00       call        _dtoui3 (542554A5h)  
5415D6F0 89 45 D8             mov         dword ptr [ebp-28h],eax  

The JIT helper and the VC++ helper behave differently. Tiering aside, this is also an inconsistency between x64 (which always generates 0) and x86 (which generates when optimizing 4294967295).

I wonder if the JIT could simply call CORINFO_HELP_DBL2UINT from gtFoldExprConst somehow. Otherwise we'll probably need to copy that to the JIT code.

@jkotas
Copy link
Member Author

jkotas commented Oct 26, 2019

JIT could simply call CORINFO_HELP_DBL2UINT from gtFoldExprConst somehow

This does not work for cross-compilation. The JIT designs should be compatible with cross-compilation.

@mikedn
Copy link
Contributor

mikedn commented Oct 26, 2019

The JIT designs should be compatible with cross-compilation.

Yes, so that leaves the JIT with the following choices:

  • stop constant folding when the result is undefined
  • try to emulate whatever the helper provided by the VM is doing

The first is probably going to be safer, the second may just move the problem somewhere else - VM's own helpers aren't all written in ASM so they too may sometimes be affected by C++ compiler changes. And even if the JIT copies the VM helper implementation, due to cross-compiling it is not guaranteed that the behavior will be the same. Hopefully such cases are rare.

@sandreenko
Copy link
Contributor

stop constant folding when the result is undefined

That looks like a more stable solution.

Sounds similar to #10905

@msftgits msftgits transferred this issue from dotnet/coreclr Jan 31, 2020
@msftgits msftgits added this to the 5.0 milestone Jan 31, 2020
@briansull briansull self-assigned this Jun 29, 2020
@briansull
Copy link
Contributor

This no longer repros, the code consistently prints 0:

Tier 0 code:

; Assembly listing for method Program:foo()
; Emitting BLENDED_CODE for X64 CPU with AVX - Windows
; Tier-0 compilation
; MinOpts code
; rbp based frame
; partially interruptible
; Final local variable assignments
;
;  V00 OutArgs      [V00    ] (  1,  1   )  lclBlk (32) [rsp+0x00]   "OutgoingArgSpace"
;
; Lcl frame size = 32

G_M65459_IG01:
       55                   push     rbp
       4883EC20             sub      rsp, 32
       488D6C2420           lea      rbp, [rsp+20H]
						;; bbWeight=1    PerfScore 1.75
G_M65459_IG02:
       C4E1FB2C0D15000000   vcvttsd2si rcx, qword ptr [reloc @RWD08]
       E838FDFFFF           call     System.Console:WriteLine(int)
       90                   nop      
						;; bbWeight=1    PerfScore 8.25
G_M65459_IG03:
       488D6500             lea      rsp, [rbp]
       5D                   pop      rbp
       C3                   ret      
						;; bbWeight=1    PerfScore 2.00
RWD00  dq	0000000000000000h
RWD08  dq	7FEFFFFFFFFFFFFFh

Tier 1 code:

; Assembly listing for method Program:foo()
; Emitting BLENDED_CODE for X64 CPU with AVX - Windows
; Tier-1 compilation
; optimized code
; rsp based frame
; partially interruptible
; Final local variable assignments
;
;  V00 OutArgs      [V00    ] (  1,  1   )  lclBlk (32) [rsp+0x00]   "OutgoingArgSpace"
;
; Lcl frame size = 40

G_M65459_IG01:
       4883EC28             sub      rsp, 40
						;; bbWeight=1    PerfScore 0.25
G_M65459_IG02:
       33C9                 xor      ecx, ecx
       E8C583FFFF           call     System.Console:WriteLine(int)
       90                   nop      
						;; bbWeight=1    PerfScore 1.50
G_M65459_IG03:
       4883C428             add      rsp, 40
       C3                   ret      
						;; bbWeight=1    PerfScore 1.25

@briansull
Copy link
Contributor

briansull commented Jun 30, 2020

Possibly caused by dotnet/coreclr#27384

commit c322db1
Author: Vladimir Sadov vsadov@microsoft.com
Date: Fri Oct 25 10:14:45 2019 -0700

Use half-fences for volatile loads/stores on Windows ARM64 (dotnet/coreclr#27384)

And possibly fixed with commit: 75421f7

@briansull
Copy link
Contributor

This issue is fixed

@ghost ghost locked as resolved and limited conversation to collaborators Dec 11, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI bug
Projects
None yet
Development

No branches or pull requests

5 participants