{"action":"created","issue":{"url":"https://api.github.com/repos/jerryscript-project/jerryscript/issues/2392","repository_url":"https://api.github.com/repos/jerryscript-project/jerryscript","labels_url":"https://api.github.com/repos/jerryscript-project/jerryscript/issues/2392/labels{/name}","comments_url":"https://api.github.com/repos/jerryscript-project/jerryscript/issues/2392/comments","events_url":"https://api.github.com/repos/jerryscript-project/jerryscript/issues/2392/events","html_url":"https://github.com/jerryscript-project/jerryscript/pull/2392","id":330747922,"node_id":"MDExOlB1bGxSZXF1ZXN0MTkzNjgxNzQ2","number":2392,"title":" Rework 128 bit arithmetics of ecma_utf8_string_to_number","user":{"login":"akosthekiss","id":9976482,"node_id":"MDQ6VXNlcjk5NzY0ODI=","avatar_url":"https://avatars0.githubusercontent.com/u/9976482?v=4","gravatar_id":"","url":"https://api.github.com/users/akosthekiss","html_url":"https://github.com/akosthekiss","followers_url":"https://api.github.com/users/akosthekiss/followers","following_url":"https://api.github.com/users/akosthekiss/following{/other_user}","gists_url":"https://api.github.com/users/akosthekiss/gists{/gist_id}","starred_url":"https://api.github.com/users/akosthekiss/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/akosthekiss/subscriptions","organizations_url":"https://api.github.com/users/akosthekiss/orgs","repos_url":"https://api.github.com/users/akosthekiss/repos","events_url":"https://api.github.com/users/akosthekiss/events{/privacy}","received_events_url":"https://api.github.com/users/akosthekiss/received_events","type":"User","site_admin":false},"labels":[{"id":211039489,"node_id":"MDU6TGFiZWwyMTEwMzk0ODk=","url":"https://api.github.com/repos/jerryscript-project/jerryscript/labels/binary%20size","name":"binary size","color":"C0C0C0","default":false},{"id":211040980,"node_id":"MDU6TGFiZWwyMTEwNDA5ODA=","url":"https://api.github.com/repos/jerryscript-project/jerryscript/labels/ecma%20core","name":"ecma core","color":"bfd4f2","default":false},{"id":195265506,"node_id":"MDU6TGFiZWwxOTUyNjU1MDY=","url":"https://api.github.com/repos/jerryscript-project/jerryscript/labels/enhancement","name":"enhancement","color":"009800","default":true},{"id":211039376,"node_id":"MDU6TGFiZWwyMTEwMzkzNzY=","url":"https://api.github.com/repos/jerryscript-project/jerryscript/labels/memory%20consumption","name":"memory consumption","color":"C0C0C0","default":false}],"state":"open","locked":false,"assignee":null,"assignees":[],"milestone":null,"comments":8,"created_at":"2018-06-08T17:49:40Z","updated_at":"2018-06-11T17:43:18Z","closed_at":null,"author_association":"MEMBER","pull_request":{"url":"https://api.github.com/repos/jerryscript-project/jerryscript/pulls/2392","html_url":"https://github.com/jerryscript-project/jerryscript/pull/2392","diff_url":"https://github.com/jerryscript-project/jerryscript/pull/2392.diff","patch_url":"https://github.com/jerryscript-project/jerryscript/pull/2392.patch"},"body":"* Rewritten 128-bit integer \"type\" to use two 64-bit ints (they are enough), and made it a proper struct instead of an array.\r\n* Rewritten all single-bit shift loops to multi-bit shifts with the help of CLZ, and used `__builtin_clzll` where available.\r\n* Simplified (and documented) 128-bit DIV10 operation.\r\n* Renamed 128-bit integer handling macros to use simpler names. (And removed unused macros that were laying around.)\r\n\r\nThis is the result of a longer experiment and rewrites part of `ecma_utf8_string_to_number` quite heavily. Thus, for now, the commits are not squashed, allowing reviewers to take a closer look at each rewriting step separately. Should the PR be accepted, all this will be squashed, of course.\r\n"},"comment":{"url":"https://api.github.com/repos/jerryscript-project/jerryscript/issues/comments/396326187","html_url":"https://github.com/jerryscript-project/jerryscript/pull/2392#issuecomment-396326187","issue_url":"https://api.github.com/repos/jerryscript-project/jerryscript/issues/2392","id":396326187,"node_id":"MDEyOklzc3VlQ29tbWVudDM5NjMyNjE4Nw==","user":{"login":"akosthekiss","id":9976482,"node_id":"MDQ6VXNlcjk5NzY0ODI=","avatar_url":"https://avatars0.githubusercontent.com/u/9976482?v=4","gravatar_id":"","url":"https://api.github.com/users/akosthekiss","html_url":"https://github.com/akosthekiss","followers_url":"https://api.github.com/users/akosthekiss/followers","following_url":"https://api.github.com/users/akosthekiss/following{/other_user}","gists_url":"https://api.github.com/users/akosthekiss/gists{/gist_id}","starred_url":"https://api.github.com/users/akosthekiss/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/akosthekiss/subscriptions","organizations_url":"https://api.github.com/users/akosthekiss/orgs","repos_url":"https://api.github.com/users/akosthekiss/repos","events_url":"https://api.github.com/users/akosthekiss/events{/privacy}","received_events_url":"https://api.github.com/users/akosthekiss/received_events","type":"User","site_admin":false},"created_at":"2018-06-11T17:43:18Z","updated_at":"2018-06-11T17:43:18Z","author_association":"MEMBER","body":"Indeed, the second snippet is buggy. And that's not even the only error there, I just realize. I was typing it from a phone, if that's an excuse. But it shows the problem quite well in my opinion. With no concept of carry, multi-part addition in C is much of a trouble / unnecessary boilerplate.\r\n\r\nAs for the question about the result of an \"overflowing\" expression, I think the C standard has the answer: [C99 6.2.5 Types, paragraph 9](http://www.open-std.org/jtc1/sc22/WG14/www/docs/n1256.pdf)\r\n\r\n> A computation involving unsigned operands can never overflow, because a result that cannot be represented by the resulting unsigned integer type is reduced modulo the number that is one greater than the largest value that can be represented by the resulting type\r\n\r\nThat is, it is very much possible, `a0 + b0` can wrap around nicely and `a0 + b0` can be less than `b0`, which signals the carry.\r\n\r\nAs for the JS standard on the conversion, I haven't looked it up first, I have to admit. My primary goal was to improve the code while keeping the functionality unchanged. But the comment before `ecma_utf8_string_to_number` refers to [ES5.1 9.3.1](https://www.ecma-international.org/ecma-262/5.1/#sec-9.3.1), which I have looked up now and it expects 20 decimal digits of precision while `double` is only precise to ~15 decimal digits. This may explain why the implementation is not using the 53 bit mantissa of `double` but a longer one. Also the comment of the float32 code path (\"Less precise conversion\") suggests that the trivial algorithm that works with `ecma_number_t` may not be precise enough (i.e., it may be OK for float32 where we wave our hands when it comes to precision anyway but not OK for the float64 case, which has to be standard-conforming).\r\n"}}
{"actor":{"display_login":"akosthekiss"}}