Skip to content

Add symbolic constants for vm opcode flags #1062

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

Merged

Conversation

zherczeg
Copy link
Member

Should fix #1057.

@LaszloLango LaszloLango added enhancement An improvement style Related to coding style labels May 17, 2016
@@ -29,12 +29,25 @@
* @{
*/

/**
* Shift for extracting the type of getting input data.
Copy link
Member

Choose a reason for hiding this comment

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

The description of these four macros is a bit hard to grasp. Would be nice to have them improved. Otherwise, the newly introduced macros look good.

Copy link
Contributor

Choose a reason for hiding this comment

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

I think we usually write "position" instead of "shift".

@LaszloLango
Copy link
Contributor

LGTM

@zherczeg zherczeg force-pushed the pre_post_increment_fixes branch from a70d230 to cfb2a40 Compare May 18, 2016 08:44
@zherczeg
Copy link
Member Author

Reworked the patch. Binary is reduced by 220 bytes.

/**
* Each CBC opcode is transformed to three vm opcodes:
*
* - first opcode is a "get arguments" opcode which specify
Copy link
Member

Choose a reason for hiding this comment

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

typo: specify -> specifies (three occurences)

Copy link
Member

Choose a reason for hiding this comment

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

was this comment ignored intentionally?

Copy link
Member Author

Choose a reason for hiding this comment

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

Forgot to fix it.

@zherczeg zherczeg force-pushed the pre_post_increment_fixes branch from cfb2a40 to ba53d04 Compare May 18, 2016 09:31
@zherczeg
Copy link
Member Author

Patch refocused a bit, and the description is aligned for the new focus.

@zherczeg zherczeg force-pushed the pre_post_increment_fixes branch from ba53d04 to 9532380 Compare May 18, 2016 09:53
@LaszloLango
Copy link
Contributor

Still LGTM

@akosthekiss
Copy link
Member

LGTM

and comments. Space consumed by opcode triplets are reduced to 16 bits
down from 32 bits. This reduces the opcode triplet tables by 220 bytes.
New symbolic constants and defines were also added to describe common
operations.

JerryScript-DCO-1.0-Signed-off-by: Zoltan Herczeg zherczeg.u-szeged@partner.samsung.com
@zherczeg zherczeg force-pushed the pre_post_increment_fixes branch from 9532380 to 12a58e6 Compare May 18, 2016 12:05
@zherczeg zherczeg merged commit 12a58e6 into jerryscript-project:master May 18, 2016
@zherczeg zherczeg deleted the pre_post_increment_fixes branch May 18, 2016 12:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement An improvement style Related to coding style
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add symbolic constants for performing increment in vm.cpp
3 participants