1537908 - Store numICEntries in SharedScriptData

Status: RESOLVED FIXED

(Core :: JavaScript Engine: JIT, enhancement, P2)

Description

[Jan de Mooij [:jandem]]

This will let us remove the Vector in ICScript::Create. We can allocate the ICScript upfront and write to that directly.

Comment 1

[Jan de Mooij [:jandem]]

We should probably move the JOF_TYPESET uint16_t from JSScript to SharedScriptData at the same time.

Comment 2

[Jan de Mooij [:jandem]]

Attachment: Bug 1537908 part 1 - Clean up BytecodeEmitter's TypeSet count a bit. r?tcampbell!

This makes the naming more consistent. Also, instead of checking against
JSScript::MaxBytecodeTypeSets at every JOF_TYPESET op, we can move this
condition to SharedScriptData creation if we use uint32_t instead of uint16_t.
This is more efficient and also nicer because MaxBytecodeTypeSets seems to be
more of a JSScript implementation detail the bytecode emitter doessn't need to
worry about.

Comment 3

[Jan de Mooij [:jandem]]

Attachment: Bug 1537908 part 2 - Store numICEntries in SharedScriptData. r?tcampbell!

This also tidies up overflow checks to ensure numICEntries never overflows
UINT32_MAX in the emitter.

ICScript currently stores numICEntries as uint32_t, but it's not an
issue due to BaselineMaxScriptLength.

Depends on D28449

Comment 4

[Jan de Mooij [:jandem]]

Attachment: Bug 1537908 part 3 - Use SharedScriptData::numICEntries to allocate and initialize ICScript directly. r?tcampbell!

Because SharedScriptData now contains numICEntries, we can allocate the ICScript
up front and fill it in directly. Advantages are: no Vector malloc/free overhead,
no ICEntry copying, no more LifoAlloc::steal, better cache locality, fewer moving
parts. This will also make it easier to merge ICScript and TypeScript in the future.

Depends on D28450

Comment 5

[Jan de Mooij [:jandem]]

leave-open because after this lands I want to post a part 4 to replace the exception in part 2 with a static_assert.

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/f35cf62b01a1
part 1 - Clean up BytecodeEmitter's TypeSet count a bit. r=tcampbell
https://hg.mozilla.org/integration/autoland/rev/39133aa95fce
part 2 - Store numICEntries in SharedScriptData. r=tcampbell
https://hg.mozilla.org/integration/autoland/rev/50967daabdf4
part 3 - Use SharedScriptData::numICEntries to allocate and initialize ICScript directly. r=tcampbell

Comment 7

[Razvan Maries]

https://hg.mozilla.org/mozilla-central/rev/f35cf62b01a1
https://hg.mozilla.org/mozilla-central/rev/39133aa95fce
https://hg.mozilla.org/mozilla-central/rev/50967daabdf4

Comment 8

[Jan de Mooij [:jandem]]

Attachment: Bug 1537908 part 4 - Replace overflow check with a static_assert. r?tcampbell!

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/3ce407a09308
part 4 - Replace overflow check with a static_assert. r=tcampbell

Comment 10

[Cristian Brindusan [:cbrindusan]]

https://hg.mozilla.org/mozilla-central/rev/3ce407a09308