Split TypeGuard and BailoutTarget.

Split TypeGuard and BailoutTarget.

By splitting the guard and the bailout-target we can merge more targets. Take for instance the following code:
x = a[i];
t = x << 4;

We can merge the bailout-targets for both guards to jump before the 'x'. This means that we will do more work (when we bail out), but it will reduce the code-size.

It also allows us to do the following optimization:
x = a[i];  // knowing that 'a' is a JS array.
if (x < 10) ...
=>
if (typeof i != 'number') bailout-to-1;
x = a[i];
if (typeof x != 'number') bailout-to-1;
if (x < 10) ...

Note that we don't do an integer check on 'i', and that we don't do an out-of-bounds check. Indeed, if i is out-of-bounds (or not an integer) then we will get 'undefined' back which is not a number. Since the bailout-target is before the array-access we will execute it again in bailout-code and correctly throw an exception there.



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

[ricow1, 2012-07-23 12:52:26]

LGTM with a few comments

https://chromiumcodereview.appspot.com/10807069/diff/1/lib/compiler/implement...
File lib/compiler/implementation/ssa/bailout.dart (right):

https://chromiumcodereview.appspot.com/10807069/diff/1/lib/compiler/implement...
lib/compiler/implementation/ssa/bailout.dart:354: assert(guard.inputs.length ==
2);
Why is the length of the inputs always 2

https://chromiumcodereview.appspot.com/10807069/diff/1/lib/compiler/implement...
File lib/compiler/implementation/ssa/codegen.dart (right):

https://chromiumcodereview.appspot.com/10807069/diff/1/lib/compiler/implement...
lib/compiler/implementation/ssa/codegen.dart:3101: List<HBailoutTarget> guards =
node.thenBlock.bailoutTargets;
rename guards to targets

https://chromiumcodereview.appspot.com/10807069/diff/1/lib/compiler/implement...
File lib/compiler/implementation/ssa/nodes.dart (left):

https://chromiumcodereview.appspot.com/10807069/diff/1/lib/compiler/implement...
lib/compiler/implementation/ssa/nodes.dart:1044: int checkedInputIndex = 0;
why remove this?

https://chromiumcodereview.appspot.com/10807069/diff/1/lib/compiler/implement...
File lib/compiler/implementation/ssa/tracer.dart (right):

https://chromiumcodereview.appspot.com/10807069/diff/1/lib/compiler/implement...
lib/compiler/implementation/ssa/tracer.dart:499: skippedFirstGuarded = true;
you should add a comment explaining why we can safely skip this

[floitsch, 2012-07-23 13:27:48]

https://chromiumcodereview.appspot.com/10807069/diff/1/lib/compiler/implement...
File lib/compiler/implementation/ssa/bailout.dart (right):

https://chromiumcodereview.appspot.com/10807069/diff/1/lib/compiler/implement...
lib/compiler/implementation/ssa/bailout.dart:354: assert(guard.inputs.length ==
2);
On 2012/07/23 12:52:26, ricow1 wrote:
> Why is the length of the inputs always 2

Added comment:
// A type-guard initially only has two inputs: the guarded instruction
// and the bailout-target. Only after adding the environment is it
// allowed to have more inputs.

https://chromiumcodereview.appspot.com/10807069/diff/1/lib/compiler/implement...
File lib/compiler/implementation/ssa/codegen.dart (right):

https://chromiumcodereview.appspot.com/10807069/diff/1/lib/compiler/implement...
lib/compiler/implementation/ssa/codegen.dart:3101: List<HBailoutTarget> guards =
node.thenBlock.bailoutTargets;
On 2012/07/23 12:52:26, ricow1 wrote:
> rename guards to targets

Done.

https://chromiumcodereview.appspot.com/10807069/diff/1/lib/compiler/implement...
File lib/compiler/implementation/ssa/nodes.dart (left):

https://chromiumcodereview.appspot.com/10807069/diff/1/lib/compiler/implement...
lib/compiler/implementation/ssa/nodes.dart:1044: int checkedInputIndex = 0;
On 2012/07/23 12:52:26, ricow1 wrote:
> why remove this?

It looked like an unnecessary complexity to me.
Before this CL the input was the environment that was used for the bailout call,
and therefore the order mattered. Now that's not true anymore. We have the
bailout-target which defines how we should send the environment variables to the
bailout-function.

https://chromiumcodereview.appspot.com/10807069/diff/1/lib/compiler/implement...
File lib/compiler/implementation/ssa/tracer.dart (right):

https://chromiumcodereview.appspot.com/10807069/diff/1/lib/compiler/implement...
lib/compiler/implementation/ssa/tracer.dart:499: skippedFirstGuarded = true;
On 2012/07/23 12:52:26, ricow1 wrote:
> you should add a comment explaining why we can safely skip this

refactored code.

[ricow1, 2012-07-23 13:35:12]

LGTM

[ngeoffray, 2012-08-16 12:34:46]

Just for the record: why making the split?

[floitsch, 2012-08-16 14:19:07]

On 2012/08/16 12:34:46, ngeoffray wrote:
> Just for the record: why making the split?

Updated the description.

[ngeoffray, 2012-08-16 14:26:00]

On 2012/08/16 14:19:07, floitsch wrote:
> On 2012/08/16 12:34:46, ngeoffray wrote:
> > Just for the record: why making the split?
> 
> Updated the description.

Thanks!