Constant-fold strings in HGraphBuilder::BuildBinaryOperation.

Constant-fold strings in HGraphBuilder::BuildBinaryOperation.

R=vegorov@chromium.org
BUG=v8:3325
LOG=N

Committed: https://crrev.com/40ebcea1138c0c7af4329ea309c1085238d73409
Cr-Commit-Position: refs/heads/master@{#26782}

[Vyacheslav Egorov (Google), 2015-02-19 15:14:10]

vegorov@google.com changed reviewers:
+ vegorov@google.com

[Vyacheslav Egorov (Google), 2015-02-19 15:14:10]

LGTM

if it compiles and runs :)

https://codereview.chromium.org/935393002/diff/1/src/hydrogen.cc
File src/hydrogen.cc (right):

https://codereview.chromium.org/935393002/diff/1/src/hydrogen.cc#newcode10502
src/hydrogen.cc:10502: if (left->IsConstant() &&
HConstant::cast(left)->HasStringValue() &&
Maybe have 

Handle<String> left_string = left->IsConstant() &&
HConstant::cast(left)->HasStringValue() ? HConstant::cast(right)->StringValue()
: Handle<String>();
Handle<String> right_string = left->IsConstant() &&
HConstant::cast(left)->HasStringValue() ? HConstant::cast(right)->StringValue()
: Handle<String>();

then all three ifs can use it:

if (!left_string.is_null() && left_string->length() == 0) {
  ...
}

if (!right_string.is_null() && right_string->length() == 0) {
  ...
}

if (!left_string.is_null() && !right_string.is_null()) {
  ..
}

[Yang, 2015-02-20 13:49:03]

New patchsets have been uploaded after l-g-t-m from vegorov@google.com

[Yang, 2015-02-20 13:49:07]

The CQ bit was checked by yangguo@chromium.org

[commit-bot: I haz the power, 2015-02-20 13:50:03]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/935393002/20001

[commit-bot: I haz the power, 2015-02-20 14:10:20]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2015-02-20 14:10:21]

Try jobs failed on following builders:
  v8_linux_nodcheck_rel on tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux_nodcheck_rel/build...)

[Vyacheslav Egorov (Google), 2015-02-20 14:54:05]

I think when I was copy pasting code for my comment I did it super wrong :)

https://codereview.chromium.org/935393002/diff/20001/src/hydrogen.cc
File src/hydrogen.cc (right):

https://codereview.chromium.org/935393002/diff/20001/src/hydrogen.cc#newcode1...
src/hydrogen.cc:10494: ? HConstant::cast(right)->StringValue()
should be left here.

https://codereview.chromium.org/935393002/diff/20001/src/hydrogen.cc#newcode1...
src/hydrogen.cc:10497: left->IsConstant() &&
HConstant::cast(left)->HasStringValue()
should be right-> here.

[Yang, 2015-02-20 14:57:04]

On 2015/02/20 14:54:05, Vyacheslav Egorov (Google) wrote:
> I think when I was copy pasting code for my comment I did it super wrong :)
> 
> https://codereview.chromium.org/935393002/diff/20001/src/hydrogen.cc
> File src/hydrogen.cc (right):
> 
>
https://codereview.chromium.org/935393002/diff/20001/src/hydrogen.cc#newcode1...
> src/hydrogen.cc:10494: ? HConstant::cast(right)->StringValue()
> should be left here.
> 
>
https://codereview.chromium.org/935393002/diff/20001/src/hydrogen.cc#newcode1...
> src/hydrogen.cc:10497: left->IsConstant() &&
> HConstant::cast(left)->HasStringValue()
> should be right-> here.

I should have double checked :)

[Yang, 2015-02-20 14:57:32]

The CQ bit was checked by yangguo@chromium.org

[commit-bot: I haz the power, 2015-02-20 14:58:24]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/935393002/40001

[commit-bot: I haz the power, 2015-02-20 15:09:15]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2015-02-20 15:09:16]

Try jobs failed on following builders:
  v8_linux_nodcheck_rel on tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux_nodcheck_rel/build...)

[commit-bot: I haz the power, 2015-02-20 15:09:17]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2015-02-20 15:09:18]

Try jobs failed on following builders:
  v8_linux_nodcheck_rel on tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux_nodcheck_rel/build...)

[Vyacheslav Egorov (Google), 2015-02-20 15:10:17]

https://codereview.chromium.org/935393002/diff/40001/src/hydrogen.cc
File src/hydrogen.cc (right):

https://codereview.chromium.org/935393002/diff/40001/src/hydrogen.cc#newcode1...
src/hydrogen.cc:10497: left->IsConstant() &&
HConstant::cast(right)->HasStringValue()
one left is still here :)

[Yang, 2015-02-20 15:20:32]

The CQ bit was checked by yangguo@chromium.org

[Yang, 2015-02-20 15:20:47]

On 2015/02/20 15:10:17, Vyacheslav Egorov (Google) wrote:
> https://codereview.chromium.org/935393002/diff/40001/src/hydrogen.cc
> File src/hydrogen.cc (right):
> 
>
https://codereview.chromium.org/935393002/diff/40001/src/hydrogen.cc#newcode1...
> src/hydrogen.cc:10497: left->IsConstant() &&
> HConstant::cast(right)->HasStringValue()
> one left is still here :)

argh. copy paste is evil.

[commit-bot: I haz the power, 2015-02-20 15:21:34]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/935393002/60001

[commit-bot: I haz the power, 2015-02-20 16:17:04]

Committed patchset #4 (id:60001)

[commit-bot: I haz the power, 2015-02-20 16:17:05]

Committed patchset #4 (id:60001)

[commit-bot: I haz the power, 2015-02-20 16:17:16]

Patchset 4 (id:??) landed as
https://crrev.com/40ebcea1138c0c7af4329ea309c1085238d73409
Cr-Commit-Position: refs/heads/master@{#26782}