fix: JS_NewFloat64 -O3 not support Infinity NaN by zeromake · Pull Request #401 · saghul/txiki.js

zeromake · 2023-09-28T01:51:24Z

close #399

saghul · 2023-09-28T04:52:18Z

Please don't change the CMake version, it's not related to your change AFAICT.

zeromake · 2023-09-28T05:51:02Z

@saghul CMake version change is reset

zeromake · 2023-09-28T05:54:41Z

@saghul Incidentally cmake latest version is 3.27, in deps/quickjs/CMakeLists.txt cmake_minimum_required(VERSION 3.4) what's going on

zeromake · 2023-09-28T06:01:38Z

@saghul
https://github.com/saghul/txiki.js/actions/runs/6333568907/job/17204665047

// (25).toExponential(0) on mingw is 2e+1
assert.eq((25).toExponential(0), "3e+1");

I'll find a Windows system debugging later

deps/quickjs/include/quickjs.h

zeromake · 2023-09-29T06:43:01Z

rounding snprintf on windows llvm-mingw unlike Linux

/* fesetround example */
#include <stdio.h>    /* printf */
#include <fenv.h>    /* fesetround, FE_* */
#include <math.h>    /* rint */

int main()
{
    float arr[6] = {28, -28, 25, -25, 24, -24};
    char buf[128] = {0};
    for (int i = 0; i < 6; i++) {
        float v = arr[i];
        printf("--------------------rounding %f:\n", v);
        fesetround(FE_DOWNWARD);
        
        snprintf(buf, 128, "%+.*e", 0, v);
        printf("FE_DOWNWARD: %s\n", buf);
        fesetround(FE_TONEAREST);
        snprintf(buf, 128, "%+.*e", 0, v);
        printf("FE_TONEAREST: %s\n", buf);
        fesetround(FE_TOWARDZERO);
        snprintf(buf, 128, "%+.*e", 0, v);
        printf("FE_TOWARDZERO: %s\n", buf);
        fesetround(FE_UPWARD);
        snprintf(buf, 128, "%+.*e", 0, v);
        printf("FE_UPWARD: %s\n\n", buf);
    }
    return 0;
}

on llvm-mingw

# -O0
--------------------rounding 28.000000:
FE_DOWNWARD: +3e+01
FE_TONEAREST: +3e+01
FE_TOWARDZERO: +3e+01
FE_UPWARD: +3e+01

--------------------rounding -28.000000:
FE_DOWNWARD: -3e+01
FE_TONEAREST: -3e+01
FE_TOWARDZERO: -3e+01
FE_UPWARD: -3e+01

--------------------rounding 25.000000:
FE_DOWNWARD: +2e+01
FE_TONEAREST: +2e+01
FE_TOWARDZERO: +2e+01
FE_UPWARD: +2e+01

--------------------rounding -25.000000:
FE_DOWNWARD: -2e+01
FE_TONEAREST: -2e+01
FE_TOWARDZERO: -2e+01
FE_UPWARD: -2e+01

--------------------rounding 24.000000:
FE_DOWNWARD: +2e+01
FE_TONEAREST: +2e+01
FE_TOWARDZERO: +2e+01
FE_UPWARD: +2e+01

--------------------rounding -24.000000:
FE_DOWNWARD: -2e+01
FE_TONEAREST: -2e+01
FE_TOWARDZERO: -2e+01
FE_UPWARD: -2e+01

# -O3
--------------------rounding 28.000000:
FE_DOWNWARD: +3e+01
FE_TONEAREST: +3e+01
FE_TOWARDZERO: +3e+01
FE_UPWARD: +3e+01

--------------------rounding -28.000000:
FE_DOWNWARD: -3e+01
FE_TONEAREST: -3e+01
FE_TOWARDZERO: -3e+01
FE_UPWARD: -3e+01

--------------------rounding 25.000000:
FE_DOWNWARD: +2e+01
FE_TONEAREST: +2e+01
FE_TOWARDZERO: +2e+01
FE_UPWARD: +2e+01

--------------------rounding -25.000000:
FE_DOWNWARD: -2e+01
FE_TONEAREST: -2e+01
FE_TOWARDZERO: -2e+01
FE_UPWARD: -2e+01

--------------------rounding 24.000000:
FE_DOWNWARD: +2e+01
FE_TONEAREST: +2e+01
FE_TOWARDZERO: +2e+01
FE_UPWARD: +2e+01

--------------------rounding -24.000000:
FE_DOWNWARD: -2e+01
FE_TONEAREST: -2e+01
FE_TOWARDZERO: -2e+01
FE_UPWARD: -2e+01

on linux gcc

# -O0
--------------------rounding 28.000000:
FE_DOWNWARD: +2e+01
FE_TONEAREST: +3e+01
FE_TOWARDZERO: +2e+01
FE_UPWARD: +3e+01

--------------------rounding -28.000000:
FE_DOWNWARD: -3e+01
FE_TONEAREST: -3e+01
FE_TOWARDZERO: -2e+01
FE_UPWARD: -2e+01

--------------------rounding 25.000000:
FE_DOWNWARD: +2e+01
FE_TONEAREST: +2e+01
FE_TOWARDZERO: +2e+01
FE_UPWARD: +3e+01

--------------------rounding -25.000000:
FE_DOWNWARD: -3e+01
FE_TONEAREST: -2e+01
FE_TOWARDZERO: -2e+01
FE_UPWARD: -2e+01

--------------------rounding 24.000000:
FE_DOWNWARD: +2e+01
FE_TONEAREST: +2e+01
FE_TOWARDZERO: +2e+01
FE_UPWARD: +3e+01

--------------------rounding -24.000000:
FE_DOWNWARD: -3e+01
FE_TONEAREST: -2e+01
FE_TOWARDZERO: -2e+01
FE_UPWARD: -2e+01

# -O3
--------------------rounding 28.000000:
FE_DOWNWARD: +2e+01
FE_TONEAREST: +3e+01
FE_TOWARDZERO: +2e+01
FE_UPWARD: +3e+01

--------------------rounding -28.000000:
FE_DOWNWARD: -3e+01
FE_TONEAREST: -3e+01
FE_TOWARDZERO: -2e+01
FE_UPWARD: -2e+01

--------------------rounding 25.000000:
FE_DOWNWARD: +2e+01
FE_TONEAREST: +2e+01
FE_TOWARDZERO: +2e+01
FE_UPWARD: +3e+01

--------------------rounding -25.000000:
FE_DOWNWARD: -3e+01
FE_TONEAREST: -2e+01
FE_TOWARDZERO: -2e+01
FE_UPWARD: -2e+01

--------------------rounding 24.000000:
FE_DOWNWARD: +2e+01
FE_TONEAREST: +2e+01
FE_TOWARDZERO: +2e+01
FE_UPWARD: +3e+01

--------------------rounding -24.000000:
FE_DOWNWARD: -3e+01
FE_TONEAREST: -2e+01
FE_TOWARDZERO: -2e+01
FE_UPWARD: -2e+01

saghul · 2023-09-29T07:11:07Z

Wow we are going deep here :-) Do you reckon this is a bug in the compiler? Making that particular test dependent on the platform, with a comment and a link to this issue sounds like an acceptable compromise IMHO.

zeromake · 2023-09-29T07:20:40Z

Wow we are going deep here :-) Do you reckon this is a bug in the compiler? Making that particular test dependent on the platform, with a comment and a link to this issue sounds like an acceptable compromise IMHO.

this pr only for Infinity NaN, del toExponential toFixed test, If you have a plan, let's mention another PR

fix: JS_NewFloat64 -O3 not support Infinity NaN

db3c30b

zeromake force-pushed the master branch from c6384c4 to db3c30b Compare September 28, 2023 05:50

saghul reviewed Sep 28, 2023

View reviewed changes

deps/quickjs/include/quickjs.h Outdated Show resolved Hide resolved

fix: add unlikely use

6a625da

saghul reviewed Sep 28, 2023

View reviewed changes

deps/quickjs/include/quickjs.h Outdated Show resolved Hide resolved

Update deps/quickjs/include/quickjs.h

df6877e

zeromake force-pushed the master branch from c56f92e to df6877e Compare September 29, 2023 06:08

feat: use CONFIG_PRINTF_RNDN

3d587da

fix: del toExponential toFixed test

cfb05f9

saghul merged commit ea59335 into saghul:master Sep 29, 2023

saghul mentioned this pull request Nov 7, 2023

Rounding bug on Windows (MinGW) quickjs-ng/quickjs#27

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: JS_NewFloat64 -O3 not support Infinity NaN#401

fix: JS_NewFloat64 -O3 not support Infinity NaN#401
saghul merged 5 commits intosaghul:masterfrom
zeromake:master

zeromake commented Sep 28, 2023 •

edited

Loading

Uh oh!

saghul commented Sep 28, 2023

Uh oh!

zeromake commented Sep 28, 2023

Uh oh!

zeromake commented Sep 28, 2023

Uh oh!

zeromake commented Sep 28, 2023

Uh oh!

Uh oh!

Uh oh!

zeromake commented Sep 29, 2023

Uh oh!

saghul commented Sep 29, 2023

Uh oh!

zeromake commented Sep 29, 2023 •

edited

Loading

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

zeromake commented Sep 28, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

saghul commented Sep 28, 2023

Uh oh!

zeromake commented Sep 28, 2023

Uh oh!

zeromake commented Sep 28, 2023

Uh oh!

zeromake commented Sep 28, 2023

Uh oh!

Uh oh!

Uh oh!

zeromake commented Sep 29, 2023

Uh oh!

saghul commented Sep 29, 2023

Uh oh!

zeromake commented Sep 29, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

zeromake commented Sep 28, 2023 •

edited

Loading

zeromake commented Sep 29, 2023 •

edited

Loading