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

fix Issue 15316 - different NaN value in struct initializer #6163

Closed
wants to merge 2 commits into from

Conversation

MartinNowak
Copy link
Member

@MartinNowak MartinNowak commented Sep 30, 2016

  • the fix to restore signaling NaNs after real->float/double conversions
    was only applied to e2ir but not in todt, leading to different NaN
    init values for floats fields in default initalized structs
  • move precision conversion functions to CTFloat and handle SNAN case
  • optimized gdc builts of dmd still loose SNANs way earlier when
    initializing RealExp with TargetReal.snan
    fixed in 0bbe753

@dlang-bot
Copy link
Contributor

dlang-bot commented Sep 30, 2016

Fix Bugzilla Description
15316 struct float fields not correctly initialised on x64

@WalterBright
Copy link
Member

Auto-merge toggled on

- the fix to restore signaling NaNs after real->float/double conversions
  was only applied to e2ir but not in todt, leading to different NaN
  init values for floats fields in default initalized structs
- move precision conversion functions to CTFloat and handle SNAN case
- optimized gdc builts of dmd still loose SNANs way earlier when
  initializing RealExp with TargetReal.snan
- some structs (e.g. struct S { float f; double d; }) get initialized
  using x87 loads/stores from the __initZ initializer, this
  initialization through the FPU still converts SNaNs to QNaNs
- fix SNaN init values for optimized gdc builds of dmd
  simple assignments might convert them to quiet NaNs
@MartinNowak
Copy link
Member Author

MartinNowak commented Sep 30, 2016

Changed to only a partial fix of Issue 15316. While now the object file only contains SNaNs, initialization assignment of the __initZ struct literal for some structs (e.g. struct S { float f; double d; }( involves x87 loads/stores that will convert SNaNs to QNaNs. Changing the backend to always use movss/movsd for those is out of scope for this PR.

Note that it mostly works for S s = S.init; b/c that gets converted to field-wise assignment in the frontend, while S s; (S s = _D3mod1S6__initZ;) gets converted to struct assignment in the backend.

@WalterBright
Copy link
Member

WalterBright commented Oct 1, 2016

Auto-merge toggled off until it passes so that it doesn't jug up the autotester.

@MartinNowak
Copy link
Member Author

This seems like a hopeless endeavor, we have little control over the init code for the other backends (though they are unlikely to use x87, but other architectures might also silence SNaNs).
As noone seems to rely on this much, I'd say we should switch over to QNaNs by default (Walter brought up that SNaNs didn't work out well).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants