[Webkit-unassigned] [Bug 80415] Eliminate redundant Phis in DFG

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=80415


Filip Pizlo <fpizlo at apple.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
 Attachment #130619|review?                     |review+
               Flag|                            |




--- Comment #17 from Filip Pizlo <fpizlo at apple.com>  2012-03-07 11:59:18 PST ---
(From update of attachment 130619)
View in context: https://bugs.webkit.org/attachment.cgi?id=130619&action=review

R=me as it is, but it might be worth considering doing away with the Phi-to-block links, if possible.

> Source/JavaScriptCore/dfg/DFGRedundantPhiEliminationPhase.cpp:83
> +                if (replacement != NoNode) {
> +                    // Current Phi node will be skipped, we need to update
> +                    // the variable information if it references this node.
> +                    BasicBlock* basicBlock = m_graph.m_blocks[node.blockIndex()].get();
> +                    VirtualRegister operand = node.local();
> +                    if (operandIsArgument(operand)) {
> +                        unsigned arg = operandToArgument(operand);
> +                        if (basicBlock->variablesAtHead.argument(arg) == index) {
> +                            // This argument must be unused in this block.
> +                            ASSERT(basicBlock->variablesAtTail.argument(arg) == index);
> +                            basicBlock->variablesAtHead.argument(arg) = replacement;
> +                            basicBlock->variablesAtTail.argument(arg) = replacement;
> +                        }
> +                    } else {
> +                        if (basicBlock->variablesAtHead.local(operand) == index) {
> +                            // This local variable must be unused in this block.
> +                            ASSERT(basicBlock->variablesAtTail.local(operand) == index);
> +                            basicBlock->variablesAtHead.local(operand) = replacement;
> +                            basicBlock->variablesAtTail.local(operand) = replacement;
> +                        }
> +                    }

I think this is fine, but why didn't you just walk all blocks' variablesAtHead|Tail lists after the Phi fixup fixpoint?  That way, you wouldn't have had to save the block index in the Phi nodes' OpInfos.

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.