[Interpreter] Adds an optimization to remove redundant Ldar/Star.

[Interpreter] Adds an optimization to remove redundant Ldar/Star.

Adds an optimization to omit generating Ldar/Star if the same register
is loaded or stored from the accumulator in the earlier instruction.

BUG=v8:4280
LOG=N

Committed: https://crrev.com/f665c823dd303e690ac136608d4e16dbfc39e2e8
Cr-Commit-Position: refs/heads/master@{#31984}

[mythria, 2015-11-12 12:45:44]

mythria@chromium.org changed reviewers:
+ oth@chromium.org, rmcilroy@chromium.org

[mythria, 2015-11-12 12:45:44]

Could you please review my changes

Thanks,
Mythri

[rmcilroy, 2015-11-12 14:24:26]

Nice! lgtm, but please wait for Orion to review too.

https://codereview.chromium.org/1435283002/diff/1/test/cctest/interpreter/tes...
File test/cctest/interpreter/test-bytecode-generator.cc (right):

https://codereview.chromium.org/1435283002/diff/1/test/cctest/interpreter/tes...
test/cctest/interpreter/test-bytecode-generator.cc:4749: B(LdaSmi8), U8(1),     
   // strange shuffling.
Update the comment - "hence the store to another local register"

https://codereview.chromium.org/1435283002/diff/1/test/cctest/interpreter/tes...
test/cctest/interpreter/test-bytecode-generator.cc:5178: ExpectedSnippet<int>
snippets[] = {
nit - add a comment that you are actually checking that the redundant Ldar's
don't get removed if on a different basic block.

https://codereview.chromium.org/1435283002/diff/1/test/unittests/interpreter/...
File test/unittests/interpreter/bytecode-array-builder-unittest.cc (right):

https://codereview.chromium.org/1435283002/diff/1/test/unittests/interpreter/...
test/unittests/interpreter/bytecode-array-builder-unittest.cc:45: .LoadNull()
nit - comment on why this is required.

[mythria, 2015-11-13 10:14:37]

Fixed comments.

https://codereview.chromium.org/1435283002/diff/1/test/cctest/interpreter/tes...
File test/cctest/interpreter/test-bytecode-generator.cc (right):

https://codereview.chromium.org/1435283002/diff/1/test/cctest/interpreter/tes...
test/cctest/interpreter/test-bytecode-generator.cc:4749: B(LdaSmi8), U8(1),     
   // strange shuffling.
On 2015/11/12 14:24:26, rmcilroy wrote:
> Update the comment - "hence the store to another local register"

Done.

https://codereview.chromium.org/1435283002/diff/1/test/cctest/interpreter/tes...
test/cctest/interpreter/test-bytecode-generator.cc:5178: ExpectedSnippet<int>
snippets[] = {
On 2015/11/12 14:24:26, rmcilroy wrote:
> nit - add a comment that you are actually checking that the redundant Ldar's
> don't get removed if on a different basic block.

Done.

https://codereview.chromium.org/1435283002/diff/1/test/unittests/interpreter/...
File test/unittests/interpreter/bytecode-array-builder-unittest.cc (right):

https://codereview.chromium.org/1435283002/diff/1/test/unittests/interpreter/...
test/unittests/interpreter/bytecode-array-builder-unittest.cc:45: .LoadNull()
On 2015/11/12 14:24:26, rmcilroy wrote:
> nit - comment on why this is required.

Done.

[oth, 2015-11-13 10:51:32]

lgtm. There's a tiny typo, but great. Thank you for doing this, appreciate
there's a lot of work updating all the tests.

https://codereview.chromium.org/1435283002/diff/20001/test/cctest/interpreter...
File test/cctest/interpreter/test-bytecode-generator.cc (right):

https://codereview.chromium.org/1435283002/diff/20001/test/cctest/interpreter...
test/cctest/interpreter/test-bytecode-generator.cc:5179: {"var ld_a = 1;\n"     
       // This test is to check Ldar doesnot
s/doesnot/does not/

[mythria, 2015-11-13 12:36:45]

Thanks for the reviews.

https://codereview.chromium.org/1435283002/diff/20001/test/cctest/interpreter...
File test/cctest/interpreter/test-bytecode-generator.cc (right):

https://codereview.chromium.org/1435283002/diff/20001/test/cctest/interpreter...
test/cctest/interpreter/test-bytecode-generator.cc:5179: {"var ld_a = 1;\n"     
       // This test is to check Ldar doesnot
On 2015/11/13 10:51:32, oth wrote:
> s/doesnot/does not/

Done.

[mythria, 2015-11-13 14:13:41]

The CQ bit was checked by mythria@chromium.org

[mythria, 2015-11-13 14:13:42]

The patchset sent to the CQ was uploaded after l-g-t-m from
rmcilroy@chromium.org, oth@chromium.org
Link to the patchset: https://codereview.chromium.org/1435283002/#ps60001
(title: "rebased the patch")

[commit-bot: I haz the power, 2015-11-13 14:13:50]

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

[commit-bot: I haz the power, 2015-11-13 14:15:02]

Committed patchset #4 (id:60001)

[commit-bot: I haz the power, 2015-11-14 23:21:34]

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