1595699 - Bytecode emitter cleanup after bug 1595476

Status: RESOLVED FIXED

(Core :: JavaScript Engine, task, P2)

Description

[Jan de Mooij [:jandem]]

After bug 1595476 lands we can simplify the frontend a bit:

• We can remove some of the source notes used by IonControlFlow that we no longer need.
• We can simplify how continues are handled. Bug 1595476 changes the bytecode for them in while/for-in/for-of loops to be more similar to other loops, but we can avoid the new JSOP_JUMPTARGET if there are no continues and clean up continue-handling a bit more.

Comment 1

[Ted Campbell [:tcampbell]]

Some things I'm interested in exploring here:

• Wrap uses of IonBuilder::current with helpful functions that are more descriptive. The notion of current==nullptr meaning deadcode isn't well explained to new readers.
• I find some names around PendingBlock to be a bit confusing. It doesn't convey that these well that these are source edges
• Factor out the giant #if DEBUG blocks in builder code to helper functions
• Remove the NOPs for loops and adjust the source notes. This should give more consistency
• Can loopDepth be replaced by just checking loopstack size?
• The IonBuilder::startLoop nextPC update should be unconditional after we remove NOP for loops
• Explore moving loop condition blocks to start of loop so that we only process bytecode forward
• Can we make it a strict invariant that LOOPHEAD is the only backedge?
• Do we need to still need JUMPTARGET for the fallthrough of a conditional branch?
• We need an SMDOC for the overall cfg builder. The visitJumpTarget code is such a key part, but it is undersold

Comment 2

[Jan de Mooij [:jandem]]

(In reply to Ted Campbell [:tcampbell] from comment #1)

Some things I'm interested in exploring here:

• Can loopDepth be replaced by just checking loopstack size?

I considered this but they're not equivalent when inlining: each builder then has its own loopStack_ and loopDepth_ is the sum of those...