Closed Bug 1394749 Opened 8 years ago Closed 8 years ago

Remove redundant Phi creation from IonBuilder when possible.

Categories

(Core :: JavaScript Engine: JIT, enhancement)

enhancement
Not set
normal

Tracking

()

RESOLVED INVALID

People

(Reporter: nbp, Assigned: nbp)

References

(Blocks 2 open bugs)

Details

When we create a new MBasicBlock, we currently create all the Phi, even if we do not yet know all the predecessors. Except for loop edges, we are likely (guaranteed?) to know all the predecessors before generating any code. More over, we are likely (guaranteed?) to have visited all the predecessor of a block before generating any code for it. Knowing these facts, we could add another mode for adding Phi nodes in the graph. Instead of generating Phi at the creation of the block, we could only generate Phi nodes for each non-redundant Phi before adding the first instruction. Thus we would only have redundant Phi nodes for loop edges. With this approach, most branches would generate a few Phi, instead of one Phi per local. This should definitely help with the test case from Bug 1377710, which currently generates 1 Phi per local for each branch, and spend a lot of time removing redundant Phis. This would also help Bug 1388002, by reducing the amount of memory allocated for Phi nodes, and reducing the re-allocations of Phi operands. This would also help by reducing the number of changes in Resume Point operands, thus making it possible to potentially share more sub-parts of the resume point operands vector.
Note that originally Ion had "lazy phis". This was removed in bug 732862 because it was very complicated. If we can find a good way to do this I agree it would be great.
Ok, I will open another bug, as we already prevent the creation of redundant Phi, the problem I was seeing was not redundant Phi as I thought, but the creation of Phi nodes such as: block 2: 32 MFilterTypeSet Constant_12 block 3: 45 MPhi Constant_12 FilterTypeSet_32 I will open another bug to deal with this particular issue, as the current description does not fit the problem, nor the solution.
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → INVALID
You need to log in before you can comment on or make changes to this bug.