-
Notifications
You must be signed in to change notification settings - Fork 684
Remove trivial ecma_number arithmetic functions #2123
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.
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
Remove trivial ecma_number arithmetic functions #2123
Conversation
I don't see any improvement here. This is just a noise. I think these inlining are done by compiler automatically, because the binary size did not increase. |
@LaszloLango I've updated the result with the run time standard deviation and highlighted the tests and results where the gain is magnitude greater then the std deviation. That's why I think it's more then noise. |
Also I've compared the two binaries (with patch and without patch) and there is difference between them. It's means inlining these functions has real effect. |
@@ -687,7 +687,7 @@ ecma_number_calc_remainder (ecma_number_t left_num, /**< left operand */ | |||
* | |||
* @return number - result of addition. | |||
*/ | |||
ecma_number_t | |||
inline ecma_number_t __attr_always_inline___ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please just remove these functions.
e8153f2
to
b4bcc77
Compare
@zherczeg Your suggestion have been applied! |
b4bcc77
to
4d3bbef
Compare
jerry-core/vm/vm.c
Outdated
ecma_number_t new_value = ecma_number_add (ecma_get_float_from_value (left_value), | ||
ecma_get_number_from_value (right_value)); | ||
ecma_number_t new_value = ecma_get_float_from_value (left_value) + | ||
ecma_get_number_from_value (right_value); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is not nice alignment. What is the difference between float value and number value?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Number value not specifies that the stored value is float or integer. Unfortunately it is not a nice alignment, but this is what vera++ requires.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think you can use ()
brackets to help aligning
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It worked! Thanks for your advice!
The affected function calls have been replaced with the appropriate arithmetic operands. JerryScript-DCO-1.0-Signed-off-by: Robert Fancsik [email protected]
4d3bbef
to
f1fcbf8
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
The affected function calls have been replaced with the appropriate arithmetic operands.
JerryScript-DCO-1.0-Signed-off-by: Robert Fancsik [email protected]