Closed Bug 977371 Opened 6 years ago Closed 6 years ago

Allow more than 2^20 blockids

Categories

(Core :: JavaScript Engine, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla30

People

(Reporter: luke, Assigned: luke)

Details

Attachments

(1 file)

Currently, blockid generation is capped at 2^20.  (Block-ids are created for basically any block with { } in the program and the block-id count is per-top-most function so it is easy to get a lot for Emscripten-generated codes.)  The (undocumented) reason for this cap is that the 'blockid' bitfield in the ParseNode is 20 bits (it shares the word with dflags/xflags).  The set of dflags/xflags have decreased over time so we could shrink these bitfields and inflate blockid to at least 2^22 which is will comfortably fit the biggest generated code we've seen so far.  The alternative is to inflate ParseNode a word but there are a lot of them...
Attached patch pack-pnd-flagsSplinter Review
Simple patch.
Attachment #8382637 - Flags: review?(jorendorff)
Comment on attachment 8382637 [details] [diff] [review]
pack-pnd-flags

Review of attachment 8382637 [details] [diff] [review]:
-----------------------------------------------------------------

::: js/src/frontend/ParseNode.h
@@ +663,5 @@
>                                             into a definition after the function
>                                             body has been parsed. */
> +#define PND_EMITTEDFUNCTION    0x200    /* hoisted function that was emitted */
> +
> +    static_assert(PND_EMITTEDFUNCTION <= (1 << NumDefinitionFlagBits), "Not enough bits");

Has to be <, I think, not <=, since the bits of dflags range from (1<<0) to (1<<(NumDefinitionFlagBits-1)).

@@ +685,5 @@
>                                             1. array initialiser has holes
>                                             2. array initializer has spread node */
> +#define PNX_NONCONST    0x40            /* initialiser has non-constants */
> +
> +    static_assert(PNX_NONCONST <= (1 << NumListFlagBits), "Not enough bits");

Same here.

::: js/src/frontend/Parser.cpp
@@ +64,5 @@
>              return null();                                                                  \
>          }                                                                                   \
>      JS_END_MACRO
>  
> +static const unsigned MaxBlockId = JS_BIT(ParseNode::NumBlockIdBits);

Call this BlockIdLimit please, since "Max" too strongly indicates a <= bound, but this is a < bound...

Optional: I'm not crazy about JS_BIT(); feel free to use (1 << ParseNode::NumBlockIdBits) if you agree.
Attachment #8382637 - Flags: review?(jorendorff) → review+
Agreed on JS_BIT; maybe we should just remove it.

https://hg.mozilla.org/integration/mozilla-inbound/rev/efa6f63f06b9
https://hg.mozilla.org/mozilla-central/rev/efa6f63f06b9
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla30
You need to log in before you can comment on or make changes to this bug.