[Webkit-unassigned] [Bug 84598] Eliminate unnecessary SetLocal/GetLocals in DFG

[bugzilla-daemon at webkit.org]

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





--- Comment #5 from Yuqiang Xian <yuqiang.xian at intel.com>  2012-04-25 07:05:59 PST ---
(In reply to comment #3)
> 
> It seems like these changes to AbstractState are not enough.
> 
> You need to make sure that each basic block has its own copy of all variables it uses, from the standpoint of AbstractState.  Otherwise you will get unsound behavior.  Example:
> 
> 1: var o = ...;
> 2: if (p)
> 3:     ... = o.f;
> 4: ... = o.f;
> 
> AbstractState will prove in (3) that o is a cell and that it has a particular structure. But we need to be careful and make sure that we remember that there exists a path (from (2) to (4) directly, skipping (3)) where this proof has not been introduced. Hence, at (4) we actually don't know anything about o yet.
> 
> With your patch it seems that you will end up propagating the proof from (3) to (4), which is wrong.
> 

Thanks for pointing out this. Yes it's a bug, and the new patch is supposed to fix this, by merging the predictions of o from both (2) and (3) to (4), and making the prediction of o block local. Thankfully the valuesForOSR aids.

> One of the reasons why I think that SetLocal/GetLocal are useful, and why I haven't been able to convince myself to go directly to SSA, is that the SetLocal/GetLocal situation allows us to easily reason about the set of variables that are live at block boundaries, and to easy access them with a unified indexing scheme (operand numbers).  We could build similar functionality for SSA, but it will take some care.

I agree that SetLocal/GetLocal are useful. But I think for the variable having exactly one definition, we should be able to get rid of SetLocal/GetLocal without much pain, and that's exactly what this patch tries to do. And it should provide a better situation for more high level optimizations (e.g., we're not required to place SetLocal/GetLocals and possibly allocate new operands when we move some nodes across blocks, which also should not impact OSR as SetLocal is still used for OSR exit hints and we don't move the SetLocals). Long term if we'll have liveness analysis and global reg alloc, that would be more sound.

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