Refactor GenericBinaryOperation and its helper functions to always return a R...

Refactor GenericBinaryOperation and its helper functions to always return a Result.

This is a preparation step for including number type information
in the virtual frame. We need a common place where we can update
the number type information of the result of a binary operation
since we should not modify the state of the virtual frame elements
directly.


Committed: http://code.google.com/p/v8/source/detail?r=3661

[Kevin Millikin (Chromium), 2010-01-20 15:38:45]

LGTM, but I have some style suggestions.

http://codereview.chromium.org/551080/diff/1/2
File src/ia32/codegen-ia32.cc (right):

http://codereview.chromium.org/551080/diff/1/2#newcode917
src/ia32/codegen-ia32.cc:917: GenericBinaryFlags flags;
It looks like the flags aren't needed until later, and aren't always needed. 
You could consider moving this switch later in the function so the use isn't so
far from the definition.

We also have a function Token::IsBitOp that helps here:

if (loop_nesting() > 0 &&
    (Token::IsBitOp(op) || type->IsLikelySmi())) {
  flags = NO_SMI_CODE_IN_STUB;
}

http://codereview.chromium.org/551080/diff/1/2#newcode991
src/ia32/codegen-ia32.cc:991: type, false, overwrite_mode);
Indentation is now off here.

http://codereview.chromium.org/551080/diff/1/2#newcode993
src/ia32/codegen-ia32.cc:993: answer = ConstantSmiBinaryOperation(op, &right,
left.handle(),
And here.

http://codereview.chromium.org/551080/diff/1/2#newcode1664
src/ia32/codegen-ia32.cc:1664: return LikelySmiBinaryOperation(op,
&constant_operand, operand,
You could do 'answer =' instead of 'return' here for symmetry with the other
cases.

http://codereview.chromium.org/551080/diff/1/2#newcode1692
src/ia32/codegen-ia32.cc:1692: return LikelySmiBinaryOperation(op,
&constant_operand, operand,
Same comment.

http://codereview.chromium.org/551080/diff/1/2#newcode1874
src/ia32/codegen-ia32.cc:1874: return LikelySmiBinaryOperation(op,
&constant_operand, operand,
Maybe have 'answer =' here instead of 'return'.

http://codereview.chromium.org/551080/diff/1/2#newcode1877
src/ia32/codegen-ia32.cc:1877: return LikelySmiBinaryOperation(op, operand,
&constant_operand,
And here.

[fschneider, 2010-01-20 16:04:08]

Also changed the x64 version accordingly.

http://codereview.chromium.org/551080/diff/1/2
File src/ia32/codegen-ia32.cc (right):

http://codereview.chromium.org/551080/diff/1/2#newcode917
src/ia32/codegen-ia32.cc:917: GenericBinaryFlags flags;
On 2010/01/20 15:38:45, Kevin Millikin wrote:
> It looks like the flags aren't needed until later, and aren't always needed. 
> You could consider moving this switch later in the function so the use isn't
so
> far from the definition.
> 
> We also have a function Token::IsBitOp that helps here:
> 
> if (loop_nesting() > 0 &&
>     (Token::IsBitOp(op) || type->IsLikelySmi())) {
>   flags = NO_SMI_CODE_IN_STUB;
> }
> 

Done.

http://codereview.chromium.org/551080/diff/1/2#newcode991
src/ia32/codegen-ia32.cc:991: type, false, overwrite_mode);
On 2010/01/20 15:38:45, Kevin Millikin wrote:
> Indentation is now off here.

Done.

http://codereview.chromium.org/551080/diff/1/2#newcode993
src/ia32/codegen-ia32.cc:993: answer = ConstantSmiBinaryOperation(op, &right,
left.handle(),
On 2010/01/20 15:38:45, Kevin Millikin wrote:
> And here.

Done.

http://codereview.chromium.org/551080/diff/1/2#newcode1664
src/ia32/codegen-ia32.cc:1664: return LikelySmiBinaryOperation(op,
&constant_operand, operand,
On 2010/01/20 15:38:45, Kevin Millikin wrote:
> You could do 'answer =' instead of 'return' here for symmetry with the other
> cases.

Done.

http://codereview.chromium.org/551080/diff/1/2#newcode1692
src/ia32/codegen-ia32.cc:1692: return LikelySmiBinaryOperation(op,
&constant_operand, operand,
On 2010/01/20 15:38:45, Kevin Millikin wrote:
> Same comment.

Done.

http://codereview.chromium.org/551080/diff/1/2#newcode1874
src/ia32/codegen-ia32.cc:1874: return LikelySmiBinaryOperation(op,
&constant_operand, operand,
On 2010/01/20 15:38:45, Kevin Millikin wrote:
> Maybe have 'answer =' here instead of 'return'.

Done.

http://codereview.chromium.org/551080/diff/1/2#newcode1877
src/ia32/codegen-ia32.cc:1877: return LikelySmiBinaryOperation(op, operand,
&constant_operand,
On 2010/01/20 15:38:45, Kevin Millikin wrote:
> And here.

Done.