[turbofan] Restore basic write barrier elimination.

[turbofan] Restore basic write barrier elimination.

Restore the basic write barrier elimination that we used to run as part
of the simplified lowering phase (in ChangeLowering actually) before, by
moving the write barrier computation to SimplifiedLowering where we can
still look at types and consider the heap/isolate, and just update the
WriteBarrierKind in the FieldAccess/ElementAccess that we later use when
lowering to a machine Load/Store.

CQ_INCLUDE_TRYBOTS=tryserver.v8:v8_linux64_tsan_rel
R=mstarzinger@chromium.org
BUG=v8:4969, chromium:608636
LOG=n

Committed: https://crrev.com/7dcb6ad379fbacbc8bdc8e11a6e50d680ffa3f62
Cr-Commit-Position: refs/heads/master@{#35969}

[Benedikt Meurer, 2016-05-02 13:06:58]

Hey Michi,

Here's the CL to bring back the basic write barrier elimination that was
previously in ChangeLowering and was temporarily disabled because we made that
part concurrent. This essentially just adds the write barrier hint to the
Field/ElementAccess (not sure if we want to keep it there long-term, tho), and
puts the computation of that into SimplifiedLowering.
Please take a look.

Thanks,
Benedikt

[Benedikt Meurer, 2016-05-02 13:07:03]

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

[commit-bot: I haz the power, 2016-05-02 13:07:12]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1938993002/1
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1938993002/1

[commit-bot: I haz the power, 2016-05-02 13:54:57]

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

[commit-bot: I haz the power, 2016-05-02 13:54:58]

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

[Michael Starzinger, 2016-05-02 17:20:37]

https://codereview.chromium.org/1938993002/diff/1/src/compiler/access-builder.cc
File src/compiler/access-builder.cc (right):

https://codereview.chromium.org/1938993002/diff/1/src/compiler/access-builder...
src/compiler/access-builder.cc:117: kPointerWriteBarrier};
Not sure this is correct, the value is of type "untagged pointer" a normal write
barrier here would not do the right thing AFAICT. This should be kNoWriteBarrier
IMHO. Or am I missing something?

https://codereview.chromium.org/1938993002/diff/1/src/compiler/access-builder...
src/compiler/access-builder.cc:159: kPointerWriteBarrier};
Likewise, "untagged pointer" here. In this case it might even be a pointer to an
object outside the heap. This should be kNoWriteBarrier IMHO.

https://codereview.chromium.org/1938993002/diff/1/src/compiler/simplified-ope...
File src/compiler/simplified-operator.cc (right):

https://codereview.chromium.org/1938993002/diff/1/src/compiler/simplified-ope...
src/compiler/simplified-operator.cc:92: lhs.machine_type == rhs.machine_type;
nit: Can we play it safe and also compare and hash the write barrier kind here?

https://codereview.chromium.org/1938993002/diff/1/src/compiler/simplified-ope...
src/compiler/simplified-operator.cc:102: return
base::hash_combine(access.base_is_tagged, access.offset,
nit: Likewise.

https://codereview.chromium.org/1938993002/diff/1/src/compiler/simplified-ope...
src/compiler/simplified-operator.cc:125: lhs.machine_type == rhs.machine_type;
nit: Likewise.

https://codereview.chromium.org/1938993002/diff/1/src/compiler/simplified-ope...
src/compiler/simplified-operator.cc:135: return
base::hash_combine(access.base_is_tagged, access.header_size,
nit: Likewise.

https://codereview.chromium.org/1938993002/diff/1/src/globals.h
File src/globals.h (right):

https://codereview.chromium.org/1938993002/diff/1/src/globals.h#newcode456
src/globals.h:456: enum WriteBarrierKind : uint8_t {
nit: Does this have to be global or can we put it into simplified-operator.h
instead?

[Benedikt Meurer, 2016-05-02 17:59:09]

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

[commit-bot: I haz the power, 2016-05-02 17:59:17]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1938993002/1
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1938993002/1

[commit-bot: I haz the power, 2016-05-02 19:23:02]

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

[commit-bot: I haz the power, 2016-05-02 19:23:03]

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

[Benedikt Meurer, 2016-05-03 08:59:50]

Description was changed from

==========
[turbofan] Restore basic write barrier elimination.

Restore the basic write barrier elimination that we used to run as part
of the simplified lowering phase (in ChangeLowering actually) before, by
moving the write barrier computation to SimplifiedLowering where we can
still look at types and consider the heap/isolate, and just update the
WriteBarrierKind in the FieldAccess/ElementAccess that we later use when
lowering to a machine Load/Store.

CQ_INCLUDE_TRYBOTS=tryserver.v8:v8_linux64_tsan_rel,v8_linux_nosnap_dbg
R=mstarzinger@chromium.org
BUG=v8:4969
LOG=n
==========

to

==========
[turbofan] Restore basic write barrier elimination.

Restore the basic write barrier elimination that we used to run as part
of the simplified lowering phase (in ChangeLowering actually) before, by
moving the write barrier computation to SimplifiedLowering where we can
still look at types and consider the heap/isolate, and just update the
WriteBarrierKind in the FieldAccess/ElementAccess that we later use when
lowering to a machine Load/Store.

CQ_INCLUDE_TRYBOTS=tryserver.v8:v8_linux64_tsan_rel
R=mstarzinger@chromium.org
BUG=v8:4969, chromium: 608636
LOG=n
==========

[Benedikt Meurer, 2016-05-03 09:05:14]

Please take another look.

https://codereview.chromium.org/1938993002/diff/1/src/compiler/access-builder.cc
File src/compiler/access-builder.cc (right):

https://codereview.chromium.org/1938993002/diff/1/src/compiler/access-builder...
src/compiler/access-builder.cc:117: kPointerWriteBarrier};
On 2016/05/02 17:20:37, Michael Starzinger wrote:
> Not sure this is correct, the value is of type "untagged pointer" a normal
write
> barrier here would not do the right thing AFAICT. This should be
kNoWriteBarrier
> IMHO. Or am I missing something?

Done.

https://codereview.chromium.org/1938993002/diff/1/src/compiler/access-builder...
src/compiler/access-builder.cc:159: kPointerWriteBarrier};
On 2016/05/02 17:20:37, Michael Starzinger wrote:
> Likewise, "untagged pointer" here. In this case it might even be a pointer to
an
> object outside the heap. This should be kNoWriteBarrier IMHO.

Done.

https://codereview.chromium.org/1938993002/diff/1/src/compiler/simplified-ope...
File src/compiler/simplified-operator.cc (right):

https://codereview.chromium.org/1938993002/diff/1/src/compiler/simplified-ope...
src/compiler/simplified-operator.cc:92: lhs.machine_type == rhs.machine_type;
On 2016/05/02 17:20:37, Michael Starzinger wrote:
> nit: Can we play it safe and also compare and hash the write barrier kind
here?

Done.

https://codereview.chromium.org/1938993002/diff/1/src/compiler/simplified-ope...
src/compiler/simplified-operator.cc:102: return
base::hash_combine(access.base_is_tagged, access.offset,
On 2016/05/02 17:20:37, Michael Starzinger wrote:
> nit: Likewise.

Done.

https://codereview.chromium.org/1938993002/diff/1/src/compiler/simplified-ope...
src/compiler/simplified-operator.cc:125: lhs.machine_type == rhs.machine_type;
On 2016/05/02 17:20:37, Michael Starzinger wrote:
> nit: Likewise.

Done.

https://codereview.chromium.org/1938993002/diff/1/src/compiler/simplified-ope...
src/compiler/simplified-operator.cc:135: return
base::hash_combine(access.base_is_tagged, access.header_size,
On 2016/05/02 17:20:37, Michael Starzinger wrote:
> nit: Likewise.

Done.

https://codereview.chromium.org/1938993002/diff/1/src/globals.h
File src/globals.h (right):

https://codereview.chromium.org/1938993002/diff/1/src/globals.h#newcode456
src/globals.h:456: enum WriteBarrierKind : uint8_t {
We need it for both machine and simplified operators.

[Benedikt Meurer, 2016-05-03 09:10:34]

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

[commit-bot: I haz the power, 2016-05-03 09:10:45]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1938993002/20001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1938993002/20001

[Benedikt Meurer, 2016-05-03 09:12:36]

Description was changed from

==========
[turbofan] Restore basic write barrier elimination.

Restore the basic write barrier elimination that we used to run as part
of the simplified lowering phase (in ChangeLowering actually) before, by
moving the write barrier computation to SimplifiedLowering where we can
still look at types and consider the heap/isolate, and just update the
WriteBarrierKind in the FieldAccess/ElementAccess that we later use when
lowering to a machine Load/Store.

CQ_INCLUDE_TRYBOTS=tryserver.v8:v8_linux64_tsan_rel
R=mstarzinger@chromium.org
BUG=v8:4969, chromium: 608636
LOG=n
==========

to

==========
[turbofan] Restore basic write barrier elimination.

Restore the basic write barrier elimination that we used to run as part
of the simplified lowering phase (in ChangeLowering actually) before, by
moving the write barrier computation to SimplifiedLowering where we can
still look at types and consider the heap/isolate, and just update the
WriteBarrierKind in the FieldAccess/ElementAccess that we later use when
lowering to a machine Load/Store.

CQ_INCLUDE_TRYBOTS=tryserver.v8:v8_linux64_tsan_rel
R=mstarzinger@chromium.org
BUG=v8:4969,chromium: 608636
LOG=n
==========

[Benedikt Meurer, 2016-05-03 09:12:56]

Description was changed from

==========
[turbofan] Restore basic write barrier elimination.

Restore the basic write barrier elimination that we used to run as part
of the simplified lowering phase (in ChangeLowering actually) before, by
moving the write barrier computation to SimplifiedLowering where we can
still look at types and consider the heap/isolate, and just update the
WriteBarrierKind in the FieldAccess/ElementAccess that we later use when
lowering to a machine Load/Store.

CQ_INCLUDE_TRYBOTS=tryserver.v8:v8_linux64_tsan_rel
R=mstarzinger@chromium.org
BUG=v8:4969,chromium: 608636
LOG=n
==========

to

==========
[turbofan] Restore basic write barrier elimination.

Restore the basic write barrier elimination that we used to run as part
of the simplified lowering phase (in ChangeLowering actually) before, by
moving the write barrier computation to SimplifiedLowering where we can
still look at types and consider the heap/isolate, and just update the
WriteBarrierKind in the FieldAccess/ElementAccess that we later use when
lowering to a machine Load/Store.

CQ_INCLUDE_TRYBOTS=tryserver.v8:v8_linux64_tsan_rel
R=mstarzinger@chromium.org
BUG=v8:4969,chromium:608636
LOG=n
==========

[Michael Starzinger, 2016-05-03 09:14:11]

LGTM.

[Benedikt Meurer, 2016-05-03 09:14:29]

The CQ bit was unchecked by bmeurer@chromium.org

[Benedikt Meurer, 2016-05-03 09:14:58]

The CQ bit was checked by bmeurer@chromium.org

[commit-bot: I haz the power, 2016-05-03 09:15:05]

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

[commit-bot: I haz the power, 2016-05-03 09:42:47]

Description was changed from

==========
[turbofan] Restore basic write barrier elimination.

Restore the basic write barrier elimination that we used to run as part
of the simplified lowering phase (in ChangeLowering actually) before, by
moving the write barrier computation to SimplifiedLowering where we can
still look at types and consider the heap/isolate, and just update the
WriteBarrierKind in the FieldAccess/ElementAccess that we later use when
lowering to a machine Load/Store.

CQ_INCLUDE_TRYBOTS=tryserver.v8:v8_linux64_tsan_rel
R=mstarzinger@chromium.org
BUG=v8:4969,chromium:608636
LOG=n
==========

to

==========
[turbofan] Restore basic write barrier elimination.

Restore the basic write barrier elimination that we used to run as part
of the simplified lowering phase (in ChangeLowering actually) before, by
moving the write barrier computation to SimplifiedLowering where we can
still look at types and consider the heap/isolate, and just update the
WriteBarrierKind in the FieldAccess/ElementAccess that we later use when
lowering to a machine Load/Store.

CQ_INCLUDE_TRYBOTS=tryserver.v8:v8_linux64_tsan_rel
R=mstarzinger@chromium.org
BUG=v8:4969,chromium:608636
LOG=n
==========

[commit-bot: I haz the power, 2016-05-03 09:42:49]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2016-05-03 09:43:54]

Description was changed from

==========
[turbofan] Restore basic write barrier elimination.

Restore the basic write barrier elimination that we used to run as part
of the simplified lowering phase (in ChangeLowering actually) before, by
moving the write barrier computation to SimplifiedLowering where we can
still look at types and consider the heap/isolate, and just update the
WriteBarrierKind in the FieldAccess/ElementAccess that we later use when
lowering to a machine Load/Store.

CQ_INCLUDE_TRYBOTS=tryserver.v8:v8_linux64_tsan_rel
R=mstarzinger@chromium.org
BUG=v8:4969,chromium:608636
LOG=n
==========

to

==========
[turbofan] Restore basic write barrier elimination.

Restore the basic write barrier elimination that we used to run as part
of the simplified lowering phase (in ChangeLowering actually) before, by
moving the write barrier computation to SimplifiedLowering where we can
still look at types and consider the heap/isolate, and just update the
WriteBarrierKind in the FieldAccess/ElementAccess that we later use when
lowering to a machine Load/Store.

CQ_INCLUDE_TRYBOTS=tryserver.v8:v8_linux64_tsan_rel
R=mstarzinger@chromium.org
BUG=v8:4969,chromium:608636
LOG=n

Committed: https://crrev.com/7dcb6ad379fbacbc8bdc8e11a6e50d680ffa3f62
Cr-Commit-Position: refs/heads/master@{#35969}
==========

[commit-bot: I haz the power, 2016-05-03 09:43:55]

Patchset 2 (id:??) landed as
https://crrev.com/7dcb6ad379fbacbc8bdc8e11a6e50d680ffa3f62
Cr-Commit-Position: refs/heads/master@{#35969}

[Michael Lippautz, 2016-05-03 13:44:46]

A revert of this CL (patchset #2 id:20001) has been created in
https://codereview.chromium.org/1943743003/ by mlippautz@chromium.org.

The reason for reverting is: Breaks WBs that should be there ;)

https://uberchromegw.corp.google.com/i/client.v8/builders/V8%20Linux%20-%20gc...

Will open repro bug asap..