Remove unused source notes
Categories
(Core :: JavaScript Engine, task, P2)
Tracking
()
Tracking | Status | |
---|---|---|
firefox72 | --- | fixed |
People
(Reporter: jandem, Assigned: jandem)
References
Details
Attachments
(5 files)
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.
Updated•5 years ago
|
Assignee | ||
Comment 1•5 years ago
|
||
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.
Assignee | ||
Comment 2•5 years ago
|
||
Depends on D54087
Assignee | ||
Comment 3•5 years ago
|
||
Unfortunately SRC_TABLESWITCH is still used by code coverage.
Depends on D54088
Assignee | ||
Comment 4•5 years ago
|
||
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•5 years ago
|
||
(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.
Assignee | ||
Updated•5 years ago
|
Pushed by jdemooij@mozilla.com: 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•5 years ago
|
||
bugherder |
Assignee | ||
Comment 8•5 years ago
|
||
Assignee | ||
Comment 9•5 years ago
|
||
Comment 10•5 years ago
|
||
Pushed by jdemooij@mozilla.com: 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
Assignee | ||
Comment 11•5 years ago
|
||
I hope we can remove the loop source notes as part of bug 1598548, but this is good enough for now.
Assignee | ||
Updated•5 years ago
|
Comment 12•5 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/2f229ecb29b1
https://hg.mozilla.org/mozilla-central/rev/c51b6708183f
Description
•