977371 - Allow more than 2^20 blockids

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Luke Wagner [:luke]]

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...

Comment 1

[Luke Wagner [:luke]]

Attachment: pack-pnd-flags

Simple patch.

Comment 2

[Jason Orendorff [: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.

Comment 3

[Luke Wagner [:luke]]

Agreed on JS_BIT; maybe we should just remove it.

https://hg.mozilla.org/integration/mozilla-inbound/rev/efa6f63f06b9

Comment 4

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/efa6f63f06b9