Remove code that adjusts synced and copied flags in MergeTo.

Remove code that adjusts synced and copied flags in MergeTo.

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

[Kevin Millikin (Chromium), 2009-05-19 07:34:25]

The style is fishy.  It will look good if you fix the issues noted below.

http://codereview.chromium.org/113524/diff/1/3
File src/frame-element.h (right):

http://codereview.chromium.org/113524/diff/1/3#newcode155
Line 155: // If the elements agree exactly aside from the Copied bit, they are
equal.
'Copied' should not be capitalized.

http://codereview.chromium.org/113524/diff/1/3#newcode156
Line 156: if (!((value_ ^ other.value_) & ~CopiedField::mask()) ) return true;
There's an extra space between the last two parentheses on this line.

Don't forget to follow the style guide.  "Short conditional statements may be
written on one line if this enhances readability. You may use this only when the
line is brief and the statement does not use the else clause."

This whole function is missing line breaks and braces.

http://codereview.chromium.org/113524/diff/1/3#newcode157
Line 157: else if (is_constant() &&
If xor'ing and masking is a win in the first test (saving an instruction over
masking, masking, and comparing) then it's probably also a win to use it in the
second test (especially if the common subexpression is lifted out).

What you have might be more readable here, but I'm always suspicious about
if (...) {
  return true;
} else {
  return false;
}

http://codereview.chromium.org/113524/diff/1/4
File src/virtual-frame.cc (right):

http://codereview.chromium.org/113524/diff/1/4#newcode271
Line 271: // We don't need to recompute exact copied flags.
No need to comment the code that used to be here.

Just get rid of this comment.  After this change, we *never* compute exact
copied flags, so there's probably no need to note that we specifically don't do
it here.

[William Hesse, 2009-05-19 08:46:16]

All comments adressed.

http://codereview.chromium.org/113524/diff/1/3
File src/frame-element.h (right):

http://codereview.chromium.org/113524/diff/1/3#newcode155
Line 155: // If the elements agree exactly aside from the Copied bit, they are
equal.
On 2009/05/19 07:34:25, Kevin Millikin wrote:
> 'Copied' should not be capitalized.

Done.

http://codereview.chromium.org/113524/diff/1/3#newcode156
Line 156: if (!((value_ ^ other.value_) & ~CopiedField::mask()) ) return true;
On 2009/05/19 07:34:25, Kevin Millikin wrote:
> There's an extra space between the last two parentheses on this line.
> 
> Don't forget to follow the style guide.  "Short conditional statements may be
> written on one line if this enhances readability. You may use this only when
the
> line is brief and the statement does not use the else clause."
> 
> This whole function is missing line breaks and braces.

Done.

http://codereview.chromium.org/113524/diff/1/3#newcode157
Line 157: else if (is_constant() &&
On 2009/05/19 07:34:25, Kevin Millikin wrote:
> If xor'ing and masking is a win in the first test (saving an instruction over
> masking, masking, and comparing) then it's probably also a win to use it in
the
> second test (especially if the common subexpression is lifted out).
> 
> What you have might be more readable here, but I'm always suspicious about
> if (...) {
>   return true;
> } else {
>   return false;
> }

Done.

http://codereview.chromium.org/113524/diff/1/3#newcode173
Line 173: bool not_same_location =
This test is now factored out to a separate (inline) function.

http://codereview.chromium.org/113524/diff/1/4
File src/virtual-frame.cc (right):

http://codereview.chromium.org/113524/diff/1/4#newcode271
Line 271: // We don't need to recompute exact copied flags.
On 2009/05/19 07:34:25, Kevin Millikin wrote:
> No need to comment the code that used to be here.
> 
> Just get rid of this comment.  After this change, we *never* compute exact
> copied flags, so there's probably no need to note that we specifically don't
do
> it here.

Done.

[Kevin Millikin (Chromium), 2009-05-19 09:02:34]

LGTM now.