Reserve a state variable for the bailout function.

Reserve a state variable for the bailout function.

BUG=http://dartbug.com/5807

Committed: https://code.google.com/p/dart/source/detail?r=13620

[kasperl, 2012-10-12 12:33:52]

LGTM!

https://codereview.chromium.org/11092104/diff/2001/lib/compiler/implementatio...
File lib/compiler/implementation/ssa/codegen.dart (right):

https://codereview.chromium.org/11092104/diff/2001/lib/compiler/implementatio...
lib/compiler/implementation/ssa/codegen.dart:2793: pushExpressionAsStatement(new
js.Assignment(new js.VariableUse(stateName),
Add a helper for using the state? 

   new js.VariableUse(variableNames.stateName)

[karlklose, 2012-10-12 12:45:13]

LGTM.

[floitsch, 2012-10-12 13:16:44]

https://codereview.chromium.org/11092104/diff/2001/lib/compiler/implementatio...
File lib/compiler/implementation/ssa/codegen.dart (right):

https://codereview.chromium.org/11092104/diff/2001/lib/compiler/implementatio...
lib/compiler/implementation/ssa/codegen.dart:2793: pushExpressionAsStatement(new
js.Assignment(new js.VariableUse(stateName),
On 2012/10/12 12:33:52, kasperl wrote:
> Add a helper for using the state? 
> 
>    new js.VariableUse(variableNames.stateName)

Done.

[ngeoffray, 2012-10-15 14:12:29]

https://codereview.chromium.org/11092104/diff/4004/lib/compiler/implementatio...
File lib/compiler/implementation/ssa/codegen.dart (right):

https://codereview.chromium.org/11092104/diff/4004/lib/compiler/implementatio...
lib/compiler/implementation/ssa/codegen.dart:2715: newParameters.add(new
js.Parameter(name));
Something doesn't look right here: why do you need to do it for state but not
for env$i? Is it the declaredVariables.add call that makes it safe? Should you
do that for the state as well instead of this CL?

https://codereview.chromium.org/11092104/diff/4004/lib/compiler/implementatio...
File lib/compiler/implementation/ssa/variable_allocator.dart (right):

https://codereview.chromium.org/11092104/diff/4004/lib/compiler/implementatio...
lib/compiler/implementation/ssa/variable_allocator.dart:436: // throughout the
function. Therefore we make sure no one uses it at any
I'd prefer keeping the comment that was there before, and add an explicit
comment on why we need stateName too.

[floitsch, 2012-10-18 14:58:47]

https://codereview.chromium.org/11092104/diff/4004/lib/compiler/implementatio...
File lib/compiler/implementation/ssa/codegen.dart (right):

https://codereview.chromium.org/11092104/diff/4004/lib/compiler/implementatio...
lib/compiler/implementation/ssa/codegen.dart:2715: newParameters.add(new
js.Parameter(name));
On 2012/10/15 14:12:29, ngeoffray wrote:
> Something doesn't look right here: why do you need to do it for state but not
> for env$i? Is it the declaredVariables.add call that makes it safe? Should you
> do that for the state as well instead of this CL?

There was still a bug in the handling of environment variables. But they are
easier to handle since they only live in the setup. The state variable is more
annoying since there are "state=0" statements sprinkled all over the function.
Even if state doesn't conflict with any argument it might still conflict with a
local. Therefore we have to reserve 'state' for the whole function.

Fix for env0... in https://codereview.chromium.org/11186048

https://codereview.chromium.org/11092104/diff/4004/lib/compiler/implementatio...
File lib/compiler/implementation/ssa/variable_allocator.dart (right):

https://codereview.chromium.org/11092104/diff/4004/lib/compiler/implementatio...
lib/compiler/implementation/ssa/variable_allocator.dart:436: // throughout the
function. Therefore we make sure no one uses it at any
On 2012/10/15 14:12:29, ngeoffray wrote:
> I'd prefer keeping the comment that was there before, and add an explicit
> comment on why we need stateName too.

https://codereview.chromium.org/11186048