Fix CI to make actual release builds on Windows #346

saghul · 2024-04-06T18:38:33Z

CMAKE_BUILD_TYPE only applies on single-configuration generators: https://cmake.org/cmake/help/latest/variable/CMAKE_BUILD_TYPE.html

For multi-configuration generators like Visual Studio (or Xcode) --config needs to be used in order to build that specific configuration.

saghul · 2024-04-06T18:45:48Z

Alright, now we can see the actual failures:

D:\a\quickjs\quickjs\quickjs.c(41035,35): error C2099: initializer is not a constant [D:\a\quickjs\quickjs\build\qjs.vcxproj]
D:\a\quickjs\quickjs\quickjs.c(41055,5): warning C4232: nonstandard extension used: 'f_f': address of dllimport 'trunc' is not static, identity not guaranteed [D:\a\quickjs\quickjs\build\qjs.vcxproj]
  C:\Program Files (x86)\Windows Kits\10\Include\10.0.20348.0\ucrt\corecrt_math.h(541,44):
  see declaration of 'trunc'
  
D:\a\quickjs\quickjs\quickjs.c(41060,5): warning C4232: nonstandard extension used: 'f_f': address of dllimport 'acosh' is not static, identity not guaranteed [D:\a\quickjs\quickjs\build\qjs.vcxproj]
  C:\Program Files (x86)\Windows Kits\10\Include\10.0.20348.0\ucrt\corecrt_math.h(493,47):
  see declaration of 'acosh'
  
D:\a\quickjs\quickjs\quickjs.c(41061,5): warning C4232: nonstandard extension used: 'f_f': address of dllimport 'asinh' is not static, identity not guaranteed [D:\a\quickjs\quickjs\build\qjs.vcxproj]
  C:\Program Files (x86)\Windows Kits\10\Include\10.0.20348.0\ucrt\corecrt_math.h(494,47):
  see declaration of 'asinh'
  
D:\a\quickjs\quickjs\quickjs.c(41062,5): warning C4232: nonstandard extension used: 'f_f': address of dllimport 'atanh' is not static, identity not guaranteed [D:\a\quickjs\quickjs\build\qjs.vcxproj]
  C:\Program Files (x86)\Windows Kits\10\Include\10.0.20348.0\ucrt\corecrt_math.h(495,47):
  see declaration of 'atanh'
  
D:\a\quickjs\quickjs\quickjs.c(41063,5): warning C4232: nonstandard extension used: 'f_f': address of dllimport 'expm1' is not static, identity not guaranteed [D:\a\quickjs\quickjs\build\qjs.vcxproj]
  C:\Program Files (x86)\Windows Kits\10\Include\10.0.20348.0\ucrt\corecrt_math.h(507,47):
  see declaration of 'expm1'
  
D:\a\quickjs\quickjs\quickjs.c(41064,5): warning C4232: nonstandard extension used: 'f_f': address of dllimport 'log1p' is not static, identity not guaranteed [D:\a\quickjs\quickjs\build\qjs.vcxproj]
  C:\Program Files (x86)\Windows Kits\10\Include\10.0.20348.0\ucrt\corecrt_math.h(521,47):
  see declaration of 'log1p'
  
D:\a\quickjs\quickjs\quickjs.c(41067,5): warning C4232: nonstandard extension used: 'f_f': address of dllimport 'cbrt' is not static, identity not guaranteed [D:\a\quickjs\quickjs\build\qjs.vcxproj]
  C:\Program Files (x86)\Windows Kits\10\Include\10.0.20348.0\ucrt\corecrt_math.h(499,47):
  see declaration of 'cbrt'

CMAKE_BUILD_TYPE only applies on single-configuration generators: https://cmake.org/cmake/help/latest/variable/CMAKE_BUILD_TYPE.html For multi-configuration generators like Visual Studio (or Xcode) --config needs to be used in order to build that specific configuration.

chqrlie · 2024-04-06T20:19:01Z

I propose a more generic approach:

we should avoid initializing data from dynamic entry points
using wrappers may actually improve performance as most of these functions are inlined by the compilers when used in expressions.

/* use local wrappers for math functions to
   - avoid initializing data with dynamic library entry points.
   - avoid some overhead if the call can be inlined at compile or link time.
 */
static double js_fabs(double d) { return fabs(d); }
static double js_floor(double d) { return floor(d); }
static double js_ceil(double d) { return ceil(d); }
static double js_sqrt(double d) { return sqrt(d); }
static double js_acos(double d) { return acos(d); }
static double js_asin(double d) { return asin(d); }
static double js_atan(double d) { return atan(d); }
static double js_atan2(double a, double b) { return atan2(a, b); }
static double js_cos(double d) { return cos(d); }
static double js_exp(double d) { return exp(d); }
static double js_log(double d) { return log(d); }
static double js_sin(double d) { return sin(d); }
static double js_tan(double d) { return tan(d); }
static double js_trunc(double d) { return trunc(d); }
static double js_cosh(double d) { return cosh(d); }
static double js_sinh(double d) { return sinh(d); }
static double js_tanh(double d) { return tanh(d); }
static double js_acosh(double d) { return acosh(d); }
static double js_asinh(double d) { return asinh(d); }
static double js_atanh(double d) { return atanh(d); }
static double js_expm1(double d) { return expm1(d); }
static double js_log1p(double d) { return log1p(d); }
static double js_log2(double d) { return log2(d); }
static double js_log10(double d) { return log10(d); }
static double js_cbrt(double d) { return cbrt(d); }

static const JSCFunctionListEntry js_math_funcs[] = {
    JS_CFUNC_MAGIC_DEF("min", 2, js_math_min_max, 0 ),
    JS_CFUNC_MAGIC_DEF("max", 2, js_math_min_max, 1 ),
    JS_CFUNC_SPECIAL_DEF("abs", 1, f_f, js_fabs ),
    JS_CFUNC_SPECIAL_DEF("floor", 1, f_f, js_floor ),
    JS_CFUNC_SPECIAL_DEF("ceil", 1, f_f, js_ceil ),
    JS_CFUNC_SPECIAL_DEF("round", 1, f_f, js_math_round ),
    JS_CFUNC_SPECIAL_DEF("sqrt", 1, f_f, js_sqrt ),

    JS_CFUNC_SPECIAL_DEF("acos", 1, f_f, js_acos ),
    JS_CFUNC_SPECIAL_DEF("asin", 1, f_f, js_asin ),
    JS_CFUNC_SPECIAL_DEF("asin", 1, f_f, js_asin ),
    JS_CFUNC_SPECIAL_DEF("atan2", 2, f_f_f, js_atan2 ),
    JS_CFUNC_SPECIAL_DEF("cos", 1, f_f, js_cos ),
    JS_CFUNC_SPECIAL_DEF("exp", 1, f_f, js_exp ),
    JS_CFUNC_SPECIAL_DEF("log", 1, f_f, js_log ),
    JS_CFUNC_SPECIAL_DEF("pow", 2, f_f_f, js_pow ),
    JS_CFUNC_SPECIAL_DEF("sin", 1, f_f, js_sin ),
    JS_CFUNC_SPECIAL_DEF("sin", 1, f_f, js_sin ),
    /* ES6 */
    JS_CFUNC_SPECIAL_DEF("trunc", 1, f_f, js_trunc ),
    JS_CFUNC_SPECIAL_DEF("sign", 1, f_f, js_math_sign ),
    JS_CFUNC_SPECIAL_DEF("cosh", 1, f_f, js_cosh ),
    JS_CFUNC_SPECIAL_DEF("sinh", 1, f_f, js_sinh ),
    JS_CFUNC_SPECIAL_DEF("sinh", 1, f_f, js_sinh ),
    JS_CFUNC_SPECIAL_DEF("acosh", 1, f_f, js_acosh ),
    JS_CFUNC_SPECIAL_DEF("asinh", 1, f_f, js_asinh ),
    JS_CFUNC_SPECIAL_DEF("asinh", 1, f_f, js_asinh ),
    JS_CFUNC_SPECIAL_DEF("expm1", 1, f_f, js_expm1 ),
    JS_CFUNC_SPECIAL_DEF("log1p", 1, f_f, js_log1p ),
    JS_CFUNC_SPECIAL_DEF("log2", 1, f_f, js_log2 ),
    JS_CFUNC_SPECIAL_DEF("log10", 1, f_f, js_log10 ),
    JS_CFUNC_SPECIAL_DEF("cbrt", 1, f_f, js_cbrt ),

saghul · 2024-04-06T20:30:42Z

I propose a more generic approach:

* we should avoid initializing data from dynamic entry points

* using wrappers may actually improve performance as most of these functions are inlined by the compilers when used in expressions.

/* use local wrappers for math functions to
   - avoid initializing data with dynamic library entry points.
   - avoid some overhead if the call can be inlined at compile or link time.
 */
static double js_fabs(double d) { return fabs(d); }
static double js_floor(double d) { return floor(d); }
static double js_ceil(double d) { return ceil(d); }
static double js_sqrt(double d) { return sqrt(d); }
static double js_acos(double d) { return acos(d); }
static double js_asin(double d) { return asin(d); }
static double js_atan(double d) { return atan(d); }
static double js_atan2(double a, double b) { return atan2(a, b); }
static double js_cos(double d) { return cos(d); }
static double js_exp(double d) { return exp(d); }
static double js_log(double d) { return log(d); }
static double js_sin(double d) { return sin(d); }
static double js_tan(double d) { return tan(d); }
static double js_trunc(double d) { return trunc(d); }
static double js_cosh(double d) { return cosh(d); }
static double js_sinh(double d) { return sinh(d); }
static double js_tanh(double d) { return tanh(d); }
static double js_acosh(double d) { return acosh(d); }
static double js_asinh(double d) { return asinh(d); }
static double js_atanh(double d) { return atanh(d); }
static double js_expm1(double d) { return expm1(d); }
static double js_log1p(double d) { return log1p(d); }
static double js_log2(double d) { return log2(d); }
static double js_log10(double d) { return log10(d); }
static double js_cbrt(double d) { return cbrt(d); }

static const JSCFunctionListEntry js_math_funcs[] = {
    JS_CFUNC_MAGIC_DEF("min", 2, js_math_min_max, 0 ),
    JS_CFUNC_MAGIC_DEF("max", 2, js_math_min_max, 1 ),
    JS_CFUNC_SPECIAL_DEF("abs", 1, f_f, js_fabs ),
    JS_CFUNC_SPECIAL_DEF("floor", 1, f_f, js_floor ),
    JS_CFUNC_SPECIAL_DEF("ceil", 1, f_f, js_ceil ),
    JS_CFUNC_SPECIAL_DEF("round", 1, f_f, js_math_round ),
    JS_CFUNC_SPECIAL_DEF("sqrt", 1, f_f, js_sqrt ),

    JS_CFUNC_SPECIAL_DEF("acos", 1, f_f, js_acos ),
    JS_CFUNC_SPECIAL_DEF("asin", 1, f_f, js_asin ),
    JS_CFUNC_SPECIAL_DEF("asin", 1, f_f, js_asin ),
    JS_CFUNC_SPECIAL_DEF("atan2", 2, f_f_f, js_atan2 ),
    JS_CFUNC_SPECIAL_DEF("cos", 1, f_f, js_cos ),
    JS_CFUNC_SPECIAL_DEF("exp", 1, f_f, js_exp ),
    JS_CFUNC_SPECIAL_DEF("log", 1, f_f, js_log ),
    JS_CFUNC_SPECIAL_DEF("pow", 2, f_f_f, js_pow ),
    JS_CFUNC_SPECIAL_DEF("sin", 1, f_f, js_sin ),
    JS_CFUNC_SPECIAL_DEF("sin", 1, f_f, js_sin ),
    /* ES6 */
    JS_CFUNC_SPECIAL_DEF("trunc", 1, f_f, js_trunc ),
    JS_CFUNC_SPECIAL_DEF("sign", 1, f_f, js_math_sign ),
    JS_CFUNC_SPECIAL_DEF("cosh", 1, f_f, js_cosh ),
    JS_CFUNC_SPECIAL_DEF("sinh", 1, f_f, js_sinh ),
    JS_CFUNC_SPECIAL_DEF("sinh", 1, f_f, js_sinh ),
    JS_CFUNC_SPECIAL_DEF("acosh", 1, f_f, js_acosh ),
    JS_CFUNC_SPECIAL_DEF("asinh", 1, f_f, js_asinh ),
    JS_CFUNC_SPECIAL_DEF("asinh", 1, f_f, js_asinh ),
    JS_CFUNC_SPECIAL_DEF("expm1", 1, f_f, js_expm1 ),
    JS_CFUNC_SPECIAL_DEF("log1p", 1, f_f, js_log1p ),
    JS_CFUNC_SPECIAL_DEF("log2", 1, f_f, js_log2 ),
    JS_CFUNC_SPECIAL_DEF("log10", 1, f_f, js_log10 ),
    JS_CFUNC_SPECIAL_DEF("cbrt", 1, f_f, js_cbrt ),

👍 I'll try that now!

Fixes: #309

saghul · 2024-04-06T20:55:35Z

@chqrlie Adopted your suggestion, thasnk for reviewing! I used js_math as the prefix since I saw there was precedent in other functions.

chqrlie · 2024-04-06T21:02:10Z

@chqrlie Adopted your suggestion, thasnk for reviewing! I used js_math as the prefix since I saw there was precedent in other functions.

Good point! you might then rename js_pow as js_math_pow for consistency.

saghul · 2024-04-06T21:14:54Z

@chqrlie Adopted your suggestion, thasnk for reviewing! I used js_math as the prefix since I saw there was precedent in other functions.

Good point! you might then rename js_pow as js_math_pow for consistency.

Did that too!

saghul mentioned this pull request Apr 6, 2024

MSVC 2022 build errors #309

Closed

saghul force-pushed the saghul-patch-4 branch from 668ef51 to 62574d9 Compare April 6, 2024 20:29

saghul force-pushed the saghul-patch-4 branch from 3986af7 to a849994 Compare April 6, 2024 20:42

saghul added 3 commits April 6, 2024 22:54

Fix compilation on MSVC 2022 in release mode

ecd17c8

Fixes: #309

Remove unnecessary shell change in CI

2433f22

Add Windows + Ninja to CI

8c04773

saghul force-pushed the saghul-patch-4 branch from a849994 to 8c04773 Compare April 6, 2024 20:54

saghul marked this pull request as ready for review April 6, 2024 20:54

chqrlie approved these changes Apr 6, 2024

View reviewed changes

saghul merged commit c33b8c9 into master Apr 6, 2024
46 checks passed

saghul deleted the saghul-patch-4 branch April 6, 2024 22:08

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix CI to make actual release builds on Windows #346

Fix CI to make actual release builds on Windows #346

saghul commented Apr 6, 2024

saghul commented Apr 6, 2024

chqrlie commented Apr 6, 2024

saghul commented Apr 6, 2024

saghul commented Apr 6, 2024

chqrlie commented Apr 6, 2024

saghul commented Apr 6, 2024

Fix CI to make actual release builds on Windows #346

Fix CI to make actual release builds on Windows #346

Conversation

saghul commented Apr 6, 2024

saghul commented Apr 6, 2024

chqrlie commented Apr 6, 2024

saghul commented Apr 6, 2024

saghul commented Apr 6, 2024

chqrlie commented Apr 6, 2024

saghul commented Apr 6, 2024