Initial implementation of top-level compilation of expressions in test...

Initial implementation of top-level compilation of expressions in test
context.  Test contexts are used for the left subexpressions of
short-circuited boolean operators.  The right subexpressions inherit
their expression context from the binary op expression.

Compilation of short-circuited operations in effect and test context
is straightforward:

effect(e0 || e1) =
 test(e0, L0, L1)
 L1: effect(e1)
 L0:

test(e0 || e1, L0, L1) =
 test(e0, L0, L2)
 L2: test(e1, L0, L1)

Because the value of the first subexpression may be needed as the
value of the whole expression in a value context, we introduce a
hybrid value/test contest (the value is needed if true, but not if
false).

value(e0 || e1) =
 value/test(e0, L0, L1)
 L1: value(e1)
 L0:

The compilation of value/test and test/value (introduced by boolean
AND) is:

value/test(e0 || e1, L0, L1) =
 value/test(e0, L0, L2)
 L2: value/test(e1, L0, L1)

test/value(e0 || e1, L0, L1) =
 test(e0, L0, L2)
 L2: test/value(e1, L0, L1)

Boolean AND is the dual.  The AST nodes themselves (not their parents)
are responsible for producing the proper result (effect, value, or
control flow) depending on their context.

[Kevin Millikin (Chromium), 2009-10-30 10:19:12]

I want to get this change in so we can build off it.  There is clearly some
duplicated code here, I will refactor it later.  I have not looked too closely
at the generated code for optimization opportunities.

[William Hesse, 2009-10-30 12:09:39]

LGTM.
Comments apply to all platforms.

http://codereview.chromium.org/339082/diff/1/4
File src/ast.h (right):

http://codereview.chromium.org/339082/diff/1/4#newcode169
Line 169: kValueTest,
Bigendian nomenclature: effect(true) ## effect(false).  Comment to that effect.

http://codereview.chromium.org/339082/diff/1/7
File src/fast-codegen.cc (right):

http://codereview.chromium.org/339082/diff/1/7#newcode247
Line 247: // use the end of the expression, otherwise inherit the same true
label.
Typo: inherit the same false label

http://codereview.chromium.org/339082/diff/1/3
File src/ia32/fast-codegen-ia32.cc (right):

http://codereview.chromium.org/339082/diff/1/3#newcode126
Line 126: TestAndBranch(source, true_label_, &discard);
Why not
Label save;
TestAndBranch(source, &save, false_label_);
__ bind(&save);
__ push(source);
__ jmp(true_label_);
?

http://codereview.chromium.org/339082/diff/1/3#newcode156
Line 156: __ mov(eax, Operand(ebp, SlotOffset(source)));
This could be optimized if TestAndBranch took an operand as the test value. 
Then we could also push the operand directly if needed.

http://codereview.chromium.org/339082/diff/1/3#newcode670
Line 670: Label discard;
Same comment as above, on optimization.

http://codereview.chromium.org/339082/diff/1/3#newcode673
Line 673: TestAndBranch(eax, true_label_, &discard);
Can we give TestAndBranch a fallthrough label, so that the fallthrough from the
last test in TestAndBranch can be used?
That seems like a worthwhile optimization, even if we don't have a global
fallthrough label.

http://codereview.chromium.org/339082/diff/1/6
File src/x64/fast-codegen-x64.cc (right):

http://codereview.chromium.org/339082/diff/1/6#newcode680
Line 680: TestAndBranch(rax, true_label_, false_label_);
Maybe we should have an operation "TestDropAndBranch" which handles kTest,
kValueTest, and kTestValue appropriately.

[Kevin Millikin (Chromium), 2009-10-30 13:45:53]

http://codereview.chromium.org/339082/diff/1/4
File src/ast.h (right):

http://codereview.chromium.org/339082/diff/1/4#newcode169
Line 169: kValueTest,
On 2009/10/30 12:09:39, William Hesse wrote:
> Bigendian nomenclature: effect(true) ## effect(false).  Comment to that
effect.

There'll be a big comment here explaining the whole thing, and another in the
implementation (currently sharing the code gen selector, but not necessarily
forever).

While things are evolving, I'll try to keep it simple.

http://codereview.chromium.org/339082/diff/1/7
File src/fast-codegen.cc (right):

http://codereview.chromium.org/339082/diff/1/7#newcode247
Line 247: // use the end of the expression, otherwise inherit the same true
label.
On 2009/10/30 12:09:39, William Hesse wrote:
> Typo: inherit the same false label

Thanks.

http://codereview.chromium.org/339082/diff/1/3
File src/ia32/fast-codegen-ia32.cc (right):

http://codereview.chromium.org/339082/diff/1/3#newcode126
Line 126: TestAndBranch(source, true_label_, &discard);
On 2009/10/30 12:09:39, William Hesse wrote:
> Why not
> Label save;
> TestAndBranch(source, &save, false_label_);
> __ bind(&save);
> __ push(source);
> __ jmp(true_label_);
> ?

TestAndBranch destroys source.

http://codereview.chromium.org/339082/diff/1/3#newcode156
Line 156: __ mov(eax, Operand(ebp, SlotOffset(source)));
On 2009/10/30 12:09:39, William Hesse wrote:
> This could be optimized if TestAndBranch took an operand as the test value. 
> Then we could also push the operand directly if needed.

TestAndBranch needs the value in a register to compare to immediates right away,
so we might as well do it here.

http://codereview.chromium.org/339082/diff/1/3#newcode673
Line 673: TestAndBranch(eax, true_label_, &discard);
On 2009/10/30 12:09:39, William Hesse wrote:
> Can we give TestAndBranch a fallthrough label, so that the fallthrough from
the
> last test in TestAndBranch can be used?
> That seems like a worthwhile optimization, even if we don't have a global
> fallthrough label.

We'd need a global fallthrough to get the whole benefit, because TestAndBranch
is frequently called on the code generator's label pair.

http://codereview.chromium.org/339082/diff/1/6
File src/x64/fast-codegen-x64.cc (right):

http://codereview.chromium.org/339082/diff/1/6#newcode680
Line 680: TestAndBranch(rax, true_label_, false_label_);
On 2009/10/30 12:09:39, William Hesse wrote:
> Maybe we should have an operation "TestDropAndBranch" which handles kTest,
> kValueTest, and kTestValue appropriately.

Possibly.  This whole chunk of platform-specific code in the assignments needs
to be refactored.