[turbofan] Initial version of the new LoadElimination.

[turbofan] Initial version of the new LoadElimination.

This adds a new optimization phase to the TurboFan pipeline, which walks
over the effect chain and tries to eliminate redundant loads (and even
some stores) of object fields. We currently ignore element access, but
that will probably need to be handled as well at some point. We also
don't have any special treatment to properly track object maps, which is
also on the list of things that will happen afterwards.

The implementation is pretty simple currently, and probably way to
inefficient. It's meant to be a proof-of-concept to iterate on.

R=jarin@chromium.org
BUG=v8:4930, v8:5141
Committed: https://crrev.com/d70dc1ace4e2a27ccdbdc6550f16e1499087ff48
Cr-Commit-Position: refs/heads/master@{#37528}

[Jarin, 2016-07-04 13:18:07]

https://codereview.chromium.org/2120253002/diff/1/src/compiler/load-eliminati...
File src/compiler/load-elimination.cc (right):

https://codereview.chromium.org/2120253002/diff/1/src/compiler/load-eliminati...
src/compiler/load-elimination.cc:69: bool MayAlias(Node* a, Node* b) { return
QueryAlias(a, b) == kMayAlias; }
How about QueryAlias(a, b) == kMayAlias || QueryAlias(a, b) == kMustAlias (or
QueryAlias(a, b) != kNoAlias)?

https://codereview.chromium.org/2120253002/diff/1/src/compiler/load-eliminati...
src/compiler/load-elimination.cc:73: class AbstractField final : public
ZoneObject {
Please explain in a comment what this is (+ the rest of the code).

https://codereview.chromium.org/2120253002/diff/1/src/compiler/load-eliminati...
src/compiler/load-elimination.cc:234: DCHECK_LE(0, field_index);
It is a shame that we cannot learn from control flow. If we could, this would
get rid of (at least some) map transitions.

https://codereview.chromium.org/2120253002/diff/1/src/compiler/load-eliminati...
src/compiler/load-elimination.cc:310: int field_index = FieldIndexOf(access);
Should not we also check the size? Either we only handle kPointerSize (and
ignore smaller sizes when loading + dcheck the store size is not bigger) or we
record the access size in the state (and handle the different size
appropriately).

Similarly in the store case.

https://codereview.chromium.org/2120253002/diff/20001/test/unittests/compiler...
File test/unittests/compiler/load-elimination-unittest.cc (left):

https://codereview.chromium.org/2120253002/diff/20001/test/unittests/compiler...
test/unittests/compiler/load-elimination-unittest.cc:3: // found in the LICENSE
file.
Actually, this is unit-testable, so we should test.

[Benedikt Meurer, 2016-07-04 17:33:54]

The CQ bit was checked by bmeurer@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-07-04 17:34:04]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[Benedikt Meurer, 2016-07-04 17:34:18]

Patchset #3 (id:40001) has been deleted

[commit-bot: I haz the power, 2016-07-04 17:38:15]

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

[commit-bot: I haz the power, 2016-07-04 17:38:15]

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

[Benedikt Meurer, 2016-07-05 11:29:10]

https://codereview.chromium.org/2120253002/diff/1/src/compiler/load-eliminati...
File src/compiler/load-elimination.cc (right):

https://codereview.chromium.org/2120253002/diff/1/src/compiler/load-eliminati...
src/compiler/load-elimination.cc:69: bool MayAlias(Node* a, Node* b) { return
QueryAlias(a, b) == kMayAlias; }
On 2016/07/04 13:18:07, Jarin wrote:
> How about QueryAlias(a, b) == kMayAlias || QueryAlias(a, b) == kMustAlias (or
> QueryAlias(a, b) != kNoAlias)?

Done.

https://codereview.chromium.org/2120253002/diff/1/src/compiler/load-eliminati...
src/compiler/load-elimination.cc:73: class AbstractField final : public
ZoneObject {
On 2016/07/04 13:18:07, Jarin wrote:
> Please explain in a comment what this is (+ the rest of the code).

Done.

https://codereview.chromium.org/2120253002/diff/1/src/compiler/load-eliminati...
src/compiler/load-elimination.cc:234: DCHECK_LE(0, field_index);
Yeah... compiler hall of shame.

https://codereview.chromium.org/2120253002/diff/1/src/compiler/load-eliminati...
src/compiler/load-elimination.cc:310: int field_index = FieldIndexOf(access);
On 2016/07/04 13:18:07, Jarin wrote:
> Should not we also check the size? Either we only handle kPointerSize (and
> ignore smaller sizes when loading + dcheck the store size is not bigger) or we
> record the access size in the state (and handle the different size
> appropriately).
> 
> Similarly in the store case.

Done.

https://codereview.chromium.org/2120253002/diff/20001/test/unittests/compiler...
File test/unittests/compiler/load-elimination-unittest.cc (left):

https://codereview.chromium.org/2120253002/diff/20001/test/unittests/compiler...
test/unittests/compiler/load-elimination-unittest.cc:3: // found in the LICENSE
file.
On 2016/07/04 13:18:07, Jarin wrote:
> Actually, this is unit-testable, so we should test.

Done.

[Benedikt Meurer, 2016-07-05 11:29:30]

Patchset #3 (id:60001) has been deleted

[Benedikt Meurer, 2016-07-05 11:29:36]

The CQ bit was checked by bmeurer@chromium.org

[commit-bot: I haz the power, 2016-07-05 11:29:46]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-07-05 11:29:47]

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

[commit-bot: I haz the power, 2016-07-05 11:29:48]

No L-G-T-M from a valid reviewer yet. 
CQ run can only be started by full committers or once the patch has
received an L-G-T-M from a full committer.
Even if an L-G-T-M may have been provided, it was from a non-committer,
_not_ a full super star committer.
Committers are members of the group "project-v8-committers".
Note that this has nothing to do with OWNERS files.

[Benedikt Meurer, 2016-07-05 11:30:02]

The CQ bit was checked by bmeurer@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-07-05 11:30:06]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-07-05 11:31:52]

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

[commit-bot: I haz the power, 2016-07-05 11:31:52]

Dry run: Try jobs failed on following builders:
  v8_mac_rel_ng on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_mac_rel_ng/builds/4456)

[Jarin, 2016-07-05 11:54:34]

lgtm. Thanks!

[Benedikt Meurer, 2016-07-05 11:54:54]

The CQ bit was checked by bmeurer@chromium.org

[commit-bot: I haz the power, 2016-07-05 11:55:02]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-07-05 12:19:54]

Committed patchset #4 (id:100001)

[commit-bot: I haz the power, 2016-07-05 12:19:55]

CQ bit was unchecked.

[commit-bot: I haz the power, 2016-07-05 12:20:28]

Description was changed from

==========
[turbofan] Initial version of the new LoadElimination.

This adds a new optimization phase to the TurboFan pipeline, which walks
over the effect chain and tries to eliminate redundant loads (and even
some stores) of object fields. We currently ignore element access, but
that will probably need to be handled as well at some point. We also
don't have any special treatment to properly track object maps, which is
also on the list of things that will happen afterwards.

The implementation is pretty simple currently, and probably way to
inefficient. It's meant to be a proof-of-concept to iterate on.

R=jarin@chromium.org
BUG=v8:4930,v8:5141
==========

to

==========
[turbofan] Initial version of the new LoadElimination.

This adds a new optimization phase to the TurboFan pipeline, which walks
over the effect chain and tries to eliminate redundant loads (and even
some stores) of object fields. We currently ignore element access, but
that will probably need to be handled as well at some point. We also
don't have any special treatment to properly track object maps, which is
also on the list of things that will happen afterwards.

The implementation is pretty simple currently, and probably way to
inefficient. It's meant to be a proof-of-concept to iterate on.

R=jarin@chromium.org
BUG=v8:4930,v8:5141

Committed: https://crrev.com/d70dc1ace4e2a27ccdbdc6550f16e1499087ff48
Cr-Commit-Position: refs/heads/master@{#37528}
==========

[commit-bot: I haz the power, 2016-07-05 12:20:29]

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