1597943 - Remove unused source notes

Status: RESOLVED FIXED

(Core :: JavaScript Engine, task, P2)

Description

[Jan de Mooij [:jandem]]

After bug 1595476 lands (and hopefully sticks!), we can remove source notes we kept for Ion but will no longer use. This will save some memory, simplify the emitter and make it easier to do follow-up bytecode cleanup.

Comment 1

[Jan de Mooij [:jandem]]

Attachment: Bug 1597943 part 1 - Remove some now-unused source notes. r?arai!

Removes:

• SRC_IF
• SRC_IF_ELSE
• SRC_COND
• SRC_CONTINUE
• SRC_BREAK
• SRC_BREAK2LABEL
• SRC_SWITCHBREAK
• SRC_NEXTCASE

IonBuilder no longer needs those source notes since bug 1595476.

Comment 2

[Jan de Mooij [:jandem]]

Attachment: Bug 1597943 part 2 - Remove For::UpdateOffset and DoWhile::CondOffset source notes. r?arai!

Depends on D54087

Comment 3

[Jan de Mooij [:jandem]]

Attachment: Bug 1597943 part 3 - Remove JSOP_CONDSWITCH and CondSwitch source notes. r?arai!

Unfortunately SRC_TABLESWITCH is still used by code coverage.

Depends on D54088

Comment 4

[Jan de Mooij [:jandem]]

I'd like to remove SRC_TABLESWITCH too while we're at it, but it's still used by code coverage.

I think it's using this to check whether the table switch has a "default:" or if it's missing it. Nicolas, any thoughts on what we can do there? Maybe we can change the results slightly so we don't depend on this source note?

It would be really nice to remove these CFG-related source notes because after that we can easily optimize the line/column data.

Comment 5

[Nicolas B. Pierron [:nbp]]

(In reply to Jan de Mooij [:jandem] from comment #4)

I'd like to remove SRC_TABLESWITCH too while we're at it, but it's still used by code coverage.

I think it's using this to check whether the table switch has a "default:" or if it's missing it. Nicolas, any thoughts on what we can do there? Maybe we can change the results slightly so we don't depend on this source note?

It would be really nice to remove these CFG-related source notes because after that we can easily optimize the line/column data.

Code coverage uses this bit of information to detect whether there is a default: clause. One of the problem is that if the default clause is the last one, then it is indistinguishable from the end of the switch body. We have no way of knowing whether there is a break statement which jumps at the end of the switch body (without looping over the bytecode), and we cannot rely on counters value to discriminate one case over the other as Ion does not collect counters.

I guess the current code could be rewritten such that, when used in last position, the default branch is being assigned the difference of the table switch opcode hits minus the hits of all other cases. And this branch could be produced in all cases, even if the default clause is missing.

A-part from that this code sounds too complex and can probably be simplified by including the handling of the default case within the loop which is already handling all other numerically indexed cases.

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/283966504e96
part 1 - Remove some now-unused source notes. r=arai
https://hg.mozilla.org/integration/autoland/rev/f10441de0a30
part 2 - Remove For::UpdateOffset and DoWhile::CondOffset source notes. r=arai
https://hg.mozilla.org/integration/autoland/rev/00eaf53014b2
part 3 - Remove JSOP_CONDSWITCH and CondSwitch source notes. r=arai

Comment 7

[Andreea Pavel [:apavel]]

https://hg.mozilla.org/mozilla-central/rev/283966504e96
https://hg.mozilla.org/mozilla-central/rev/f10441de0a30
https://hg.mozilla.org/mozilla-central/rev/00eaf53014b2

Comment 8

[Jan de Mooij [:jandem]]

Attachment: Bug 1597943 part 4 - Stop using SRC_TABLESWITCH source note in code coverage. r?nbp!

Comment 9

[Jan de Mooij [:jandem]]

Attachment: Bug 1597943 part 5 - Remove SRC_TABLESWITCH source note. r?arai!

Comment 10

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/2f229ecb29b1
part 4 - Stop using SRC_TABLESWITCH source note in code coverage. r=nbp
https://hg.mozilla.org/integration/autoland/rev/c51b6708183f
part 5 - Remove SRC_TABLESWITCH source note. r=arai

Comment 11

[Jan de Mooij [:jandem]]

I hope we can remove the loop source notes as part of bug 1598548, but this is good enough for now.

Comment 12

[Arthur Iakab [arthur_iakab]]

https://hg.mozilla.org/mozilla-central/rev/2f229ecb29b1
https://hg.mozilla.org/mozilla-central/rev/c51b6708183f