Implement BigInt asIntN and asUintN methods #4736

batizdaniel · 2021-08-16T07:10:52Z

The following methods were implemented:

BigInt.asIntN
BigInt.asUintN

Custom dispatcher also added to builtin_bigint.

JerryScript-DCO-1.0-Signed-off-by: Daniel Batiz [email protected]

zherczeg

The code is coming from the standard?

jerry-core/ecma/builtin-objects/ecma-builtin-bigint.c

rerobika · 2021-08-30T08:10:31Z

jerry-core/ecma/builtin-objects/ecma-builtin-bigint.c

+                                     ecma_value_t bigint, /**< bigint number */
+                                     bool is_signed) /**< The operation is signed */
+{
+  JERRY_UNUSED (this_arg);


due to custom dispatcher passing this_args in unnecessary.

Passing this as argument is unnecessary.

jerry-core/ecma/builtin-objects/ecma-builtin-bigint.c

zherczeg

I would like to get a bit more info about the high level concept of the algorith, so I could suggest optimizations.

zherczeg · 2021-08-31T08:23:36Z

jerry-core/ecma/builtin-objects/ecma-builtin-bigint.c

+  const uint32_t size_of_divisor_in_bits = (sizeof (uint32_t) * sizeof (uint64_t));
+
+  uint64_t whole_part = (uint64_t) floor (input_bits / size_of_divisor_in_bits);
+  uint32_t remainder = (uint32_t) fmod (input_bits, size_of_divisor_in_bits);


The input_bits should be an integer number. Furthermore JerryScript has a maximum length for bigints, any number greater than that does not need any clamping (just increase its refcount and return with it).

The head of the code is coming from the standard. So the input_bits should be a number according to the ToIndex method.
(https://tc39.es/ecma262/#sec-bigint.asintn)

Nice but my comment stiill stands, if the number is greater than the maximum bits allowed, just return with the original value, otherwise you can use an integer div and mod. If uint conversion is needed, and the original value is negative, throw an allocation error.

jerry-core/ecma/builtin-objects/ecma-builtin-bigint.c

zherczeg · 2021-08-31T08:54:32Z

I would also like to have some tests on JerryScript, not just the test262 ones. These tests should check corner cases of the algorithm, not covered by test262

rerobika · 2021-10-20T13:58:59Z

jerry-core/ecma/builtin-objects/ecma-builtin-bigint.c

+                                     ecma_value_t bigint, /**< bigint number */
+                                     bool is_signed) /**< The operation is signed */
+{
+  JERRY_UNUSED (this_arg);


Passing this as argument is unnecessary.

jerry-core/ecma/builtin-objects/ecma-builtin-bigint.c

rerobika · 2021-10-20T14:00:10Z

jerry-core/ecma/builtin-objects/ecma-builtin-bigint.c

+    uint32_t check_sign_mask = (uint32_t) 1 << (mask_bit - 1);
+
+    uint32_t mask = ((uint32_t) 1 << mask_bit) - 1;
+
+    uint32_t last_cell = (exact_size >= sizeof (uint32_t)) ? (uint32_t) (min_size / sizeof (uint32_t)) - 1 : 0;
+
+    bool is_positive = false;
+
+    bool is_representation_positive = false;


jerry-core/ecma/builtin-objects/ecma-builtin-bigint.c

The following methods were implemented: - BigInt.asIntN - BigInt.asUintN Custom dispatcher also added to builtin_bigint. JerryScript-DCO-1.0-Signed-off-by: Daniel Batiz [email protected]

zherczeg · 2021-10-27T09:20:21Z

jerry-core/ecma/builtin-objects/ecma-builtin-bigint.c

+enum
+{
+  ECMA_BUILTIN_BIGINT_START = 0,
+  ECMA_BUILTIN_BIGINT_AS_INT_N,


Missing comments.

zherczeg · 2021-10-27T09:23:04Z

jerry-core/ecma/builtin-objects/ecma-builtin-bigint.c

+  const uint32_t size_of_divisor_in_bits = (sizeof (uint32_t) * sizeof (uint64_t));
+
+  uint64_t whole_part = (uint64_t) floor (input_bits / size_of_divisor_in_bits);
+  uint32_t remainder = (uint32_t) fmod (input_bits, size_of_divisor_in_bits);


Nice but my comment stiill stands, if the number is greater than the maximum bits allowed, just return with the original value, otherwise you can use an integer div and mod. If uint conversion is needed, and the original value is negative, throw an allocation error.

zherczeg · 2021-10-27T09:29:01Z

jerry-core/ecma/builtin-objects/ecma-builtin-bigint.c

+        is_zero_values = true;
+      }
+    }
+    if (is_zero_values)


there is no else here so add a newline here

zherczeg · 2021-10-27T09:29:25Z

jerry-core/ecma/builtin-objects/ecma-builtin-bigint.c

+    }
+    while (digits_p < digits_end_p);
+
+    int16_t equal_bit_s = 0;


The following methods were implemented: - BigInt.asIntN - BigInt.asUintN Custom dispatcher also added to builtin_bigint. The implementation is based on PR jerryscript-project#4736, only applied the requested changes. Signed-off-by: Gergo Csizi [email protected]

The following methods were implemented: - BigInt.asIntN - BigInt.asUintN Custom dispatcher also added to builtin_bigint. The implementation is based on PR jerryscript-project#4736, only applied the requested changes. JerryScript-DCO-1.0-Signed-off-by: Gergo Csizi [email protected]

The implementation is based on PR jerryscript-project#4736, only resolved the conflicts. JerryScript-DCO-1.0-Signed-off-by: Gergo Csizi [email protected]

The following methods were implemented: - BigInt.asIntN - BigInt.asUintN Custom dispatcher also added to builtin_bigint. The implementation is based on PR jerryscript-project#4736, only applied the requested changes. JerryScript-DCO-1.0-Signed-off-by: Gergo Csizi [email protected] Co-authored-by: Daniel Batiz [email protected]

The following methods were implemented: - BigInt.asIntN - BigInt.asUintN Custom dispatcher also added to builtin_bigint. The implementation is based on PR jerryscript-project#4736, only applied the requested changes. Co-authored-by: Daniel Batiz [email protected] JerryScript-DCO-1.0-Signed-off-by: Gergo Csizi [email protected]

ossy-szeged · 2024-11-15T08:30:38Z

closing, continue in PR #5165

The following methods were implemented: - BigInt.asIntN - BigInt.asUintN Custom dispatcher also added to builtin_bigint. The implementation is based on PR jerryscript-project#4736, only applied the requested changes. Co-authored-by: Daniel Batiz [email protected] JerryScript-DCO-1.0-Signed-off-by: Gergo Csizi [email protected]

The following methods were implemented: - BigInt.asIntN - BigInt.asUintN Custom dispatcher also added to builtin_bigint. The implementation is based on PR #4736, only applied the requested changes. Co-authored-by: Daniel Batiz [email protected] JerryScript-DCO-1.0-Signed-off-by: Gergo Csizi [email protected]

batizdaniel force-pushed the bigint_asint branch 4 times, most recently from 7095fd8 to d2dede2 Compare August 18, 2021 09:36

rerobika requested a review from zherczeg August 30, 2021 07:58

zherczeg requested changes Aug 30, 2021

View reviewed changes

rerobika requested changes Aug 30, 2021

View reviewed changes

batizdaniel force-pushed the bigint_asint branch 2 times, most recently from 4b77e3a to 23cd30f Compare August 30, 2021 14:00

zherczeg requested changes Aug 31, 2021

View reviewed changes

batizdaniel force-pushed the bigint_asint branch 2 times, most recently from cfb13fc to ce1ba9f Compare September 1, 2021 12:45

rerobika requested changes Oct 20, 2021

View reviewed changes

Implement BigInt asIntN and asUintN methods

23c771c

The following methods were implemented: - BigInt.asIntN - BigInt.asUintN Custom dispatcher also added to builtin_bigint. JerryScript-DCO-1.0-Signed-off-by: Daniel Batiz [email protected]

batizdaniel force-pushed the bigint_asint branch from ce1ba9f to 23c771c Compare October 26, 2021 12:38

zherczeg requested changes Oct 27, 2021

View reviewed changes

LaszloLango added this to the Release 3.0.0 milestone Nov 13, 2024

gergocs mentioned this pull request Nov 15, 2024

Implement BigInt asIntN and asUintN methods #5165

Merged

gergocs added a commit to gergocs/jerryscript that referenced this pull request Nov 15, 2024

Implement operations of atomics

9594095

The implementation is based on PR jerryscript-project#4736, only resolved the conflicts. JerryScript-DCO-1.0-Signed-off-by: Gergo Csizi [email protected]

ossy-szeged removed this from the Release 3.0.0 milestone Nov 15, 2024

ossy-szeged closed this Nov 15, 2024

Implement BigInt asIntN and asUintN methods #4736

Implement BigInt asIntN and asUintN methods #4736

Conversation

batizdaniel commented Aug 16, 2021

Uh oh!

zherczeg left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

zherczeg left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

zherczeg commented Aug 31, 2021

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ossy-szeged commented Nov 15, 2024

Uh oh!

Uh oh!