Closed
Bug 818798
Opened 12 years ago
Closed 8 years ago
Dead code confuses the heck out of Ion alias analysis, effectively disables LICM
Categories
(Core :: JavaScript Engine, defect)
Tracking
()
RESOLVED
FIXED
People
(Reporter: bzbarsky, Unassigned)
References
(Blocks 1 open bug)
Details
Attachments
(2 files)
In the attached testcase, we end up unable to hoist the img.data get in the if condition because it thinks it's aliased by some setter in the loop. And the reason it thinks that is that we have no type info for the if body, so the img.data[0] get there produces MCallGetElement which aliases the world.
Comment 1•10 years ago
|
||
Here's a version of the testcase which can be run in a standalone JS shell and which is slightly more complex to prevent Ion from finding ways around the problem. The "t += img.data[0]" never executes, so we don't get a type specialization for it, so MBinaryArithInstruction::getAliasSet calls it AliasSet::Store(AliasSet::Any), and this blocks LICM of all loads in the loop.
Comment 2•10 years ago
|
||
I think the main problem here is that we often add MIR instructions that we know will always bail when they execute (empty type barriers, say) and then go on to add even more instructions to those blocks. We can potentially waste a lot of time optimizing dead code.
Reporter | ||
Updated•10 years ago
|
Attachment #8458686 -
Attachment mime type: application/javascript → text/plain
Assignee | ||
Updated•10 years ago
|
Assignee: general → nobody
Reporter | ||
Comment 3•9 years ago
|
||
This seems to be fixed now. Did we start ignoring branches with no type info for LICM purposes at some point?
Flags: needinfo?(jdemooij)
Comment 4•9 years ago
|
||
(In reply to Boris Zbarsky [:bz] from comment #3) > This seems to be fixed now. Did we start ignoring branches with no type > info for LICM purposes at some point? This is the loop body: if (img.data[0] == 1) { t += img.data[0]; } TI knows (and we insert a TypeBarrier) that the first img.data[0] is |undefined|, so the if-expression becomes: if (undefined == 1) GVN knows this is always false, so it's replaced with an MGoto. Then it notices the if-body is now unreachable so we remove it. After that, LICM is trivial. So no, we didn't fix the real issue here, but nbp is working on pruning cold branches based on Baseline feedback, this should also get rid of the if-body before we do LICM (and it should work in more cases).
Flags: needinfo?(jdemooij)
Comment 5•8 years ago
|
||
Fixed by bug 1229813?
Reporter | ||
Comment 6•8 years ago
|
||
Should be, yes.
You need to log in
before you can comment on or make changes to this bug.
Description
•