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.

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

Change the way the JIT renumbers byte code temps #6232

Merged
merged 1 commit into from
Sep 12, 2019

Conversation

pleath
Copy link
Contributor

@pleath pleath commented Aug 5, 2019

The JIT currently does not renumber byte code temps in boolean-expression-like cases that have the form:
Brm $L1
Rm = op1 ...
Br $L2
$L1:
Rm = op2 ...
$L2:
... = op3 Rm
This is hard to maintain, because it relies on conscientious use of the Unused op in byte code where a def of a register is not followed by a use, but we still want to renumber the register at the next use. Failure to emit Unused results in silent failure to renumber, which in turn results in suboptimal jitted code. This change deletes Unused, adds several byte codes of the form op_ReuseLoc, and causes the JIT to renumber lifetimes at each def unless the op_ReuseLoc is emitted. Failure to emit op_ReuseLoc will produce incorrect code that is detectable by the JIT (i.e., temps not defined on all paths). The biggest change is to the family of byte code patterns we emit to handle dynamic binding cases. These are restructured to limit the kinds of op_ReuseLoc ops we need to define.

@@ -45,7 +45,7 @@ EXDEF2 (BRLONG, BrLong, OP_Br)
#endif
DEF3 (CUSTOM, StartCall, OP_StartCall, StartCall)
DEF2 (NOP, Nop, Empty)
DEF2_WMS(NOP, Unused, Reg1)
// DEF2_WMS(NOP, Unused, Reg1)
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we delete this ?

Copy link
Contributor

@boingoing boingoing left a comment

Choose a reason for hiding this comment

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

Looks great. Big change here, especially in those prop store / load methods. Everything looks right to me.

chakrabot pushed a commit that referenced this pull request Sep 12, 2019
Merge pull request #6232 from pleath:no-unused

The JIT currently does not renumber byte code temps in boolean-expression-like cases that have the form:
Brm $L1
Rm = op1 ...
Br $L2
$L1:
Rm = op2 ...
$L2:
... = op3 Rm
This is hard to maintain, because it relies on conscientious use of the Unused op in byte code where a def of a register is not followed by a use, but we still want to renumber the register at the next use. Failure to emit Unused results in silent failure to renumber, which in turn results in suboptimal jitted code. This change deletes Unused, adds several byte codes of the form op_ReuseLoc, and causes the JIT to renumber lifetimes at each def unless the op_ReuseLoc is emitted. Failure to emit op_ReuseLoc will produce incorrect code that is detectable by the JIT (i.e., temps not defined on all paths). The biggest change is to the family of byte code patterns we emit to handle dynamic binding cases. These are restructured to limit the kinds of op_ReuseLoc ops we need to define.
@chakrabot chakrabot merged commit fc907f2 into chakra-core:master Sep 12, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants