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.

Sign up for GitHub

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

Jump to bottom

refactor: use constant packages, FI_F for addon and style fixes in math/base/special/ldexpf #2868

Open
wants to merge 5 commits into
base: develop
Choose a base branch
from
Open

refactor: use constant packages, FI_F for addon and style fixes in math/base/special/ldexpf #2868

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
4aa48f4
refactor: use constant packages, FI_F for addon, style fixes
gunjjoshi Sep 6, 2024
5805f17
Merge branch 'stdlib-js:develop' into ldexpf
gunjjoshi Sep 7, 2024
ad9556b
Merge branch 'stdlib-js:develop' into ldexpf
gunjjoshi Sep 7, 2024
4a04069
revert main.c, add correspondig test, update main.js
gunjjoshi Sep 7, 2024
359290b
test: add JS test, update main.js
gunjjoshi Sep 7, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 4 additions & 5 deletions lib/node_modules/@stdlib/math/base/special/ldexpf/README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ var ldexpf = require( '@stdlib/math/base/special/ldexpf' );

#### ldexpf( frac, exp )

Multiplies a [single-precision floating-point number][ieee754] by an `integer` power of two (i.e., `x = frac * 2^exp`).
Multiplies a [single-precision floating-point number][ieee754] by an integer power of two (i.e., `x = frac * 2^exp`).

```javascript
var x = ldexpf( 0.5, 3 ); // => 0.5 * 2^3 = 0.5 * 8
Expand Down Expand Up @@ -67,9 +67,7 @@ x = ldexpf( -Infinity, -118 );

## Notes

// TODO: update this once we have `frexpf`.

- This function is the inverse of [`frexp`][@stdlib/math/base/special/frexp].
- This function is the inverse of [`frexpf`][@stdlib/math/base/special/frexpf].

</section>

Expand Down Expand Up @@ -201,7 +199,8 @@ int main( void ) {
<section class="links">

[ieee754]: https://en.wikipedia.org/wiki/IEEE_754-1985
[@stdlib/math/base/special/frexp]: https://github.com/stdlib-js/stdlib/tree/develop/lib/node_modules/%40stdlib/math/base/special/frexp

[@stdlib/math/base/special/frexpf]: https://github.com/stdlib-js/stdlib/tree/develop/lib/node_modules/%40stdlib/math/base/special/frexpf

<!-- <related-links> -->

Expand Down
2 changes: 1 addition & 1 deletion lib/node_modules/@stdlib/math/base/special/ldexpf/docs/repl.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@

{{alias}}( frac, exp )
Multiplies a single-precision floating-point number by an integer power of
two; i.e., `x = frac * 2^exp`.
two (i.e., `x = frac * 2^exp`).

If `frac` equals positive or negative `zero`, `NaN`, or positive or negative
infinity, the function returns a value equal to `frac`.
Expand Down
10 changes: 5 additions & 5 deletions lib/node_modules/@stdlib/math/base/special/ldexpf/examples/c/example.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,13 +22,13 @@

int main( void ) {
float y;
int i;
int i;

const float frac[] = { 0.5f, 5.0f, 0.0f, 3.5f, 7.9f };
const int32_t exp[] = { 3, -2, 20, 39, 14 };

for ( i = 0; i < 5; i++ ) {
y = stdlib_base_ldexpf( frac[ i ], exp[ i ] );
printf( "ldexpf(%f, %d) = %f\n", frac[ i ], exp[ i ], y );
}
for ( i = 0; i < 5; i++ ) {
y = stdlib_base_ldexpf( frac[ i ], exp[ i ] );
printf( "ldexpf(%f, %d) = %f\n", frac[ i ], exp[ i ], y );
}
}
41 changes: 23 additions & 18 deletions lib/node_modules/@stdlib/math/base/special/ldexpf/lib/main.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,12 +39,17 @@
var toWord = require( '@stdlib/number/float32/base/to-word' );
var fromWord = require( '@stdlib/number/float32/base/from-word' );
var float64ToFloat32 = require( '@stdlib/number/float64/base/to-float32' );
var FLOAT32_EXPONENT_MASK = require( '@stdlib/constants/float32/exponent-mask' );
var FLOAT32_PRECISION = require( '@stdlib/constants/float32/precision' );
var FLOAT32_ABS_MASK = require( '@stdlib/constants/float32/abs-mask' );


// VARIABLES //

var TWO25 = 33554432.0; // 0x4c000000
var TWOM25 = 2.9802322387695312e-8; // 0x33000000
var FLOAT32_SIGNIFICAND_MASK_WITH_SIGN = 0x807fffff; // 1 00000000 11111111111111111111111

Check warning on line 51 in lib/node_modules/@stdlib/math/base/special/ldexpf/lib/main.js

View workflow job for this annotation

GitHub Actions / Lint Changed Files 

Identifier name 'FLOAT32_SIGNIFICAND_MASK_WITH_SIGN' is too long (> 25)
var ALL_ONES = 0xff; // 0xff = 255 => 11111111


// MAIN //
Expand Down Expand Up @@ -91,51 +96,51 @@
frac = float64ToFloat32( frac );
ix = toWord( frac );

// Extract exponent
k = ( ix & 0x7f800000 ) >> 23;
// Extract exponent:
k = ( ix & FLOAT32_EXPONENT_MASK ) >> ( FLOAT32_PRECISION - 1 );

// 0 or subnormal frac
// 0 or subnormal frac:
if ( k === 0 ) {
if ( ( ix & 0x7fffffff ) === 0 ) {
// +-0
if ( ( ix & FLOAT32_ABS_MASK ) === 0 ) {
// +-0:
return frac;
}
frac = float64ToFloat32( frac * TWO25 );
ix = toWord( frac );
k = ( ( ix & 0x7f800000 ) >> 23 ) - 25;
k = ( ( ix & FLOAT32_EXPONENT_MASK ) >> ( FLOAT32_PRECISION - 1 ) ) - 25;

Check warning on line 110 in lib/node_modules/@stdlib/math/base/special/ldexpf/lib/main.js

View workflow job for this annotation

GitHub Actions / Lint Changed Files 

This line has a length of 81. Maximum allowed is 80
if ( exp < -50000 ) {
// Underflow
// Underflow:
return 0.0;
}
}

// NaN or Inf
if ( k === 0xff ) {
// NaN or Inf:
if ( k === ALL_ONES ) {
return float64ToFloat32( frac + frac );
}
k += exp;
if ( k > 0xfe ) {
// Overflow
k = ( k + exp ) | 0;
if ( k > ALL_ONES - 1 ) {
// Overflow:
return copysignf( PINF, frac );
}
if ( k > 0 ) {
// Normal result
frac = fromWord( ( ix & 0x807fffff ) | ( k << 23 ) );
// Normal result:
frac = fromWord( ( ix & FLOAT32_SIGNIFICAND_MASK_WITH_SIGN ) | ( k << ( FLOAT32_PRECISION - 1 ) ) );

Check warning on line 128 in lib/node_modules/@stdlib/math/base/special/ldexpf/lib/main.js

View workflow job for this annotation

GitHub Actions / Lint Changed Files 

This line has a length of 108. Maximum allowed is 80
return frac;
}
if ( k <= -25 ) {
if ( exp > 50000 ) {
Copy link
Member

Choose a reason for hiding this comment

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

@gunjjoshi I am a bit confused. Why is this branch unreachable? If you can exercise L133, then you should also be able to exercise exp > 50000 by providing a exp value greater than 50000.

Copy link
Member Author

@gunjjoshi gunjjoshi Sep 7, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

@kgryte This is because, suppose we take exp as 60000. Then, on L116, k will become greater than or equal to exp, i.e., k>=60000. Then, in the very next line, i.e., on L117, the if condition will evaluate to true, as the condition checks k>0xfe, where 0xfe=254. So, the execution will go inside that block, and the result would be returned from L119.

Copy link
Member

Choose a reason for hiding this comment

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

@gunjjoshi Okay. We actually need to update L116 to emulate 32-bit integer arithmetic. It should be k = (k + exp)|0;. Then you can replicate the scenario where exp is very large and k becomes negative.

Copy link
Member Author

Choose a reason for hiding this comment

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

After updating this to according to 32-bit integer arithmetic, we do not need to add the removed block again here, right? I have added the corresponding test to only test.native.js, as we're not having that block here.

Copy link
Member

Choose a reason for hiding this comment

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

No, you do need to re-add the block as now it is possible to overflow to a negative number.

Copy link
Member Author

Choose a reason for hiding this comment

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

For instance, for exp = 3e9, k = -1294967296 in JS, but in C, k = 1.

Copy link
Member

Choose a reason for hiding this comment

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

@gunjjoshi Can you try recompiling the addon, but setting the fwrapv compiler flag? https://www.gnu.org/software/c-intro-and-ref/manual/html_node/Signed-Overflow.html

Copy link
Member

Choose a reason for hiding this comment

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

To compile with the flag set, you can use the same approach as we used when checking for fused operations previously.

Copy link
Member Author

Choose a reason for hiding this comment

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

I tried:

CFLAGS="-ffp-contract=off -fwrapv" make install-node-addons NODE_ADDONS_PATTERN="math/base/special/ldexpf"

But even with this, it doesn't work. What happens is, when I pass a large value such as 3e9 for exp, and then try to print the value of exp from inside the function, it comes out to be 1. So, the value of exp is being taken as 1 in these cases.

Copy link
Member

Choose a reason for hiding this comment

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

Hmm...odd. I wonder if I get the same behavior on my machine. I'll try and check locally sometime in the next day or so.

// In case of integer overflow in n + k
// In case of integer overflow in n + k:
return copysignf( PINF, frac );
}

// Underflow
// Underflow:
return copysignf( 0.0, frac );
}

// Subnormal result
// Subnormal result:
k += 25;
frac = fromWord( ( ix & 0x807fffff ) | ( k << 23 ) );
frac = fromWord( ( ix & FLOAT32_SIGNIFICAND_MASK_WITH_SIGN ) | ( k << ( FLOAT32_PRECISION - 1 ) ) );

Check warning on line 143 in lib/node_modules/@stdlib/math/base/special/ldexpf/lib/main.js

View workflow job for this annotation

GitHub Actions / Lint Changed Files 

This line has a length of 104. Maximum allowed is 80
return float64ToFloat32( frac * TWOM25 );
}

Expand Down
16 changes: 13 additions & 3 deletions lib/node_modules/@stdlib/math/base/special/ldexpf/manifest.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,10 +36,14 @@
"libraries": [],
"libpath": [],
"dependencies": [
"@stdlib/math/base/napi/binary",
"@stdlib/number/float32/base/from-word",
"@stdlib/number/float32/base/to-word",
"@stdlib/math/base/special/copysignf",
"@stdlib/constants/float32/pinf"
"@stdlib/constants/float32/pinf",
"@stdlib/constants/float32/exponent-mask",
"@stdlib/constants/float32/precision",
"@stdlib/constants/float32/abs-mask"
]
},
{
Expand All @@ -58,7 +62,10 @@
"@stdlib/number/float32/base/from-word",
"@stdlib/number/float32/base/to-word",
"@stdlib/math/base/special/copysignf",
"@stdlib/constants/float32/pinf"
"@stdlib/constants/float32/pinf",
"@stdlib/constants/float32/exponent-mask",
"@stdlib/constants/float32/precision",
"@stdlib/constants/float32/abs-mask"
]
},
{
Expand All @@ -75,7 +82,10 @@
"@stdlib/number/float32/base/from-word",
"@stdlib/number/float32/base/to-word",
"@stdlib/math/base/special/copysignf",
"@stdlib/constants/float32/pinf"
"@stdlib/constants/float32/pinf",
"@stdlib/constants/float32/exponent-mask",
"@stdlib/constants/float32/precision",
"@stdlib/constants/float32/abs-mask"
]
}
]
Expand Down
82 changes: 3 additions & 79 deletions lib/node_modules/@stdlib/math/base/special/ldexpf/src/addon.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,83 +17,7 @@
*/

#include "stdlib/math/base/special/ldexpf.h"
#include <node_api.h>
#include <stdint.h>
#include <assert.h>
#include "stdlib/math/base/napi/binary.h"

/**
* Receives JavaScript callback invocation data.
*
* @param env environment under which the function is invoked
* @param info callback data
* @return Node-API value
*/
static napi_value addon( napi_env env, napi_callback_info info ) {
napi_status status;

// Get callback arguments:
size_t argc = 2;
napi_value argv[ 2 ];
status = napi_get_cb_info( env, info, &argc, argv, NULL, NULL );
assert( status == napi_ok );

// Check whether we were provided the correct number of arguments:
if ( argc < 2 ) {
status = napi_throw_error( env, NULL, "invalid invocation. Insufficient arguments." );
assert( status == napi_ok );
return NULL;
}
if ( argc > 2 ) {
status = napi_throw_error( env, NULL, "invalid invocation. Too many arguments." );
assert( status == napi_ok );
return NULL;
}

napi_valuetype vtype0;
status = napi_typeof( env, argv[ 0 ], &vtype0 );
assert( status == napi_ok );
if ( vtype0 != napi_number ) {
status = napi_throw_type_error( env, NULL, "invalid argument. First argument must be a number." );
assert( status == napi_ok );
return NULL;
}

napi_valuetype vtype1;
status = napi_typeof( env, argv[ 1 ], &vtype1 );
assert( status == napi_ok );
if ( vtype1 != napi_number ) {
status = napi_throw_type_error( env, NULL, "invalid argument. Second argument must be a number." );
assert( status == napi_ok );
return NULL;
}

double frac;
status = napi_get_value_double( env, argv[ 0 ], &frac );
assert( status == napi_ok );

int32_t exp;
status = napi_get_value_int32( env, argv[ 1 ], &exp );
assert( status == napi_ok );

napi_value v;
status = napi_create_double( env, (double)stdlib_base_ldexpf( (float)frac, exp ), &v );
assert( status == napi_ok );

return v;
}

/**
* Initializes a Node-API module.
*
* @param env environment under which the function is invoked
* @param exports exports object
* @return main export
*/
static napi_value init( napi_env env, napi_value exports ) {
napi_value fcn;
napi_status status = napi_create_function( env, "exports", NAPI_AUTO_LENGTH, addon, NULL, &fcn );
assert( status == napi_ok );
return fcn;
}

NAPI_MODULE( NODE_GYP_MODULE_NAME, init )
// cppcheck-suppress shadowFunction
STDLIB_MATH_BASE_NAPI_MODULE_FI_F( stdlib_base_ldexpf )
39 changes: 22 additions & 17 deletions lib/node_modules/@stdlib/math/base/special/ldexpf/src/main.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,10 +35,15 @@
#include "stdlib/number/float32/base/from_word.h"
#include "stdlib/math/base/special/copysignf.h"
#include "stdlib/constants/float32/pinf.h"
#include "stdlib/constants/float32/exponent_mask.h"
#include "stdlib/constants/float32/precision.h"
#include "stdlib/constants/float32/abs_mask.h"
#include <stdint.h>

static const float TWO25 = 33554432.0f; // 0x4c000000
static const float TWOM25 = 2.9802322387695312e-8f; // 0x33000000
static const int32_t FLOAT32_SIGNIFICAND_MASK_WITH_SIGN = 0x807fffff; // 1 00000000 11111111111111111111111
static const int32_t ALL_ONES = 0xff; // 0xff = 255 => 11111111

/**
* Multiplies a single-precision floating-point number by an integer power of two.
Expand All @@ -60,52 +65,52 @@ float stdlib_base_ldexpf( const float frac, const int32_t exp ) {
stdlib_base_float32_to_word( frac, &uix );
ix = (int32_t)uix;

// Extract exponent
k = ( ix & 0x7f800000 ) >> 23;
// Extract exponent:
k = ( ix & STDLIB_CONSTANT_FLOAT32_EXPONENT_MASK ) >> ( STDLIB_CONSTANT_FLOAT32_PRECISION - 1 );

// 0 or subnormal frac
// 0 or subnormal frac:
fracc = frac;
if ( k == 0 ) {
if ( ( ix & 0x7fffffff ) == 0 ) {
// +-0
if ( ( ix & STDLIB_CONSTANT_FLOAT32_ABS_MASK ) == 0 ) {
// +-0:
return frac;
}
fracc = frac * TWO25;
stdlib_base_float32_to_word( fracc, &uix );
ix = (int32_t)uix;
k = ( ( ix & 0x7f800000 ) >> 23 ) - 25;
k = ( ( ix & STDLIB_CONSTANT_FLOAT32_EXPONENT_MASK ) >> ( STDLIB_CONSTANT_FLOAT32_PRECISION - 1 ) ) - 25;
if ( exp < -50000 ) {
// Underflow
// Underflow:
return 0.0;
}
}

// NaN or Inf
if ( k == 0xff ) {
// NaN or Inf:
if ( k == ALL_ONES ) {
return fracc + fracc;
}
k += exp;
if ( k > 0xfe ) {
// Overflow
if ( k > ALL_ONES - 1 ) {
// Overflow:
return stdlib_base_copysignf( STDLIB_CONSTANT_FLOAT32_PINF, fracc );
}
if ( k > 0 ) {
// Normal result
stdlib_base_float32_from_word( (uint32_t)( ( ix & 0x807fffff ) | ( k << 23 ) ), &fracc );
// Normal result:
stdlib_base_float32_from_word( (uint32_t)( ( ix & FLOAT32_SIGNIFICAND_MASK_WITH_SIGN ) | ( k << ( STDLIB_CONSTANT_FLOAT32_PRECISION - 1 ) ) ), &fracc );
return fracc;
}
if ( k <= -25 ) {
if ( exp > 50000 ) {
Copy link
Member

Choose a reason for hiding this comment

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

@gunjjoshi Same comment as for the JS version.

Copy link
Member

@kgryte kgryte Sep 7, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Okay. I see why this was in scalbn. The issue is when exp is so large that k integer overflows. Recall that k is int32_t. When k exceeds the maximum positive signed 32-bit integer, the value wraps around. In which case, L92 can generate a negative k when exp is very large. You need to revert this change.

Copy link
Member

Choose a reason for hiding this comment

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

...and add a respective test.

// In case of integer overflow in n + k
// In case of integer overflow in n + k:
return stdlib_base_copysignf( STDLIB_CONSTANT_FLOAT32_PINF, fracc );
}

// Underflow
// Underflow:
return stdlib_base_copysignf( 0.0f, fracc );
}

// Subnormal result
// Subnormal result:
k += 25;
stdlib_base_float32_from_word( (uint32_t)( ( ix & 0x807fffff ) | ( k << 23 ) ), &fracc );
stdlib_base_float32_from_word( (uint32_t)( ( ix & FLOAT32_SIGNIFICAND_MASK_WITH_SIGN ) | ( k << ( STDLIB_CONSTANT_FLOAT32_PRECISION - 1 ) ) ), &fracc );
return fracc * TWOM25;
}
6 changes: 6 additions & 0 deletions lib/node_modules/@stdlib/math/base/special/ldexpf/test/test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -225,3 +225,9 @@ tape( 'if provided a fraction and a very large negative exponent, the function r
t.equal( v, 0.0, 'returns expected value' );
t.end();
});

tape( 'if provided a small fraction and a very large positive exponent, the function returns positive infinity due to integer overflow', function test( t ) {
var v = ldexpf( 1.17549435e-38, 3e10 );
t.equal( v, PINF, 'returns expected value' );
t.end();
});
6 changes: 6 additions & 0 deletions lib/node_modules/@stdlib/math/base/special/ldexpf/test/test.native.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -234,3 +234,9 @@ tape( 'if provided a fraction and a very large negative exponent, the function r
t.equal( v, 0.0, 'returns expected value' );
t.end();
});

tape( 'if provided a small fraction and a very large positive exponent, the function returns positive infinity due to integer overflow', opts, function test( t ) {
var v = ldexpf( 1.17549435e-38, 60000 );
t.equal( v, PINF, 'returns expected value' );
t.end();
});
Loading