678630 - IonMonkey LICM doesn't hoist instructions due only to phi nodes

Status: RESOLVED INVALID

(Core :: JavaScript Engine, defect)

Description

[Ryan Pearl [:rpearl]]

In the attached test-case, the add instructions get hoisted above the phi, and into the first block, but we leave boxes in the loop for no reason (other than the phi nodes). However, it is possible hoist the boxes, which makes the phi unnecessary.

When checking whether an instruction can be hoisted, phi nodes need to be taken into account in a more sophisticated way.

Comment 1

[Andrew Scheff]

Why are boxes not hoisted?  If the input to a box is hoisted than the box should be as well.  Boxing is idempotent.. right?

Comment 2

[Ryan Pearl [:rpearl]]

Attachment: Test case

Sorry, forgot the test case.

Comment 3

[Ryan Pearl [:rpearl]]

Oh, i see what happened--forgot to re-build after a patch. Sorry!

Note that there is a separate issue that we have phis merging nodes that have been hoisted out of the block and are now unnecessary...

Comment 4

[Andrew Scheff]

LICM currently does not try to solve the problem of "what if an entire block is loop invariant and can be hoisted?" This is the issue you're referring to, yes?

Let me know if you have any ideas of easy ways to implement this, it would be cool to have blocks disappear

Comment 5

[Ryan Pearl [:rpearl]]

Sort of? Consider the test case in this bug, when compiled with Ionmonkey tip (f7076b6bc9c9 or later). The loop block (block 3) becomes empty, and the phi nodes in block 1 are now unnecessary, since they no longer correspond to a join-point--we just want the last-defined box. The expense of just leaving them (unless it confuses lowering, since it is not one value per predecessor) is unnecessary moves.

Comment 6

[Andrew Scheff]

I see.  Maybe another pass after LICM that looks for empty blocks and acts accordingly would be the right way to fix this.  Does GVN encounter any similar problems with removing all instructions from a block? How did you handle it?