Check that constant is an integer before getting its value in HGraphBuilder::MatchRotateRight.

Check that constant is an integer before getting its value in HGraphBuilder::MatchRotateRight.

BUG=351263
LOG=N
TEST=mjsunit/regress/regress-351263
R=svenpanne@chromium.org

Committed: https://code.google.com/p/v8/source/detail?r=19890

[ulan, 2014-03-13 08:55:15]

PTAL

[Sven Panne, 2014-03-13 09:32:20]

On 2014/03/13 08:55:15, ulan wrote:
> PTAL

The test itself is missing in this CL...

[Sven Panne, 2014-03-13 09:36:39]

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

https://codereview.chromium.org/197803005/diff/1/src/hydrogen.cc#newcode8939
src/hydrogen.cc:8939: HConstant::cast(const32)->Integer32Value() != 32) {
Simplify this brain teaser to

return sub->left()->EqualsInteger32Constant(32)) && sub->right() == sa;

[ulan, 2014-03-13 11:03:54]

On 2014/03/13 09:36:39, Sven Panne wrote:
> https://codereview.chromium.org/197803005/diff/1/src/hydrogen.cc
> File src/hydrogen.cc (right):
> 
> https://codereview.chromium.org/197803005/diff/1/src/hydrogen.cc#newcode8939
> src/hydrogen.cc:8939: HConstant::cast(const32)->Integer32Value() != 32) {
> Simplify this brain teaser to
> 
> return sub->left()->EqualsInteger32Constant(32)) && sub->right() == sa;

Nice one, thanks! Uploaded the new patch set with the test.

[Sven Panne, 2014-03-13 11:23:16]

LGTM with a nit.

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

https://codereview.chromium.org/197803005/diff/20001/src/hydrogen.cc#newcode8935
src/hydrogen.cc:8935: if (sa != sub->right()) return false;
I think this line can be removed, it is redundant. Slowly things get readable
here. :-)

[ulan, 2014-03-13 11:50:15]

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

https://codereview.chromium.org/197803005/diff/20001/src/hydrogen.cc#newcode8935
src/hydrogen.cc:8935: if (sa != sub->right()) return false;
On 2014/03/13 11:23:17, Sven Panne wrote:
> I think this line can be removed, it is redundant. Slowly things get readable
> here. :-)

Done.

[ulan, 2014-03-13 11:50:57]

Committed patchset #3 manually as r19890 (presubmit successful).