Update implementation of atomics with latest Chromium version but use compiler builtin atomics

Update implementation of atomics with latest Chromium version but use compiler builtin atomics

Ideally, we would use the standard library. However, when we are compiling against an older version of the standard library the atomic implementation may be slow.

BUG=

Committed: https://crrev.com/343c4ebdd13c086e732e57f0c834556f36473d97
Cr-Commit-Position: refs/heads/master@{#40488}

[Hannes Payer (out of office), 2016-10-18 11:07:25]

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

[commit-bot: I haz the power, 2016-10-18 11:07:34]

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-10-18 11:09:43]

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

[commit-bot: I haz the power, 2016-10-18 11:09:44]

Dry run: Try jobs failed on following builders:
  v8_linux64_asan_rel_ng on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux64_asan_rel_ng/buil...)
  v8_linux_arm_rel_ng on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux_arm_rel_ng/builds/...)
  v8_linux_dbg_ng on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux_dbg_ng/builds/14793)
  v8_linux_gcc_compile_rel on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux_gcc_compile_rel/bu...)
  v8_linux_rel_ng on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux_rel_ng/builds/14736)

[Hannes Payer (out of office), 2016-10-18 11:13:36]

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

[commit-bot: I haz the power, 2016-10-18 11:13:46]

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-10-18 11:15:53]

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

[commit-bot: I haz the power, 2016-10-18 11:15:54]

Dry run: Try jobs failed on following builders:
  v8_linux64_asan_rel_ng on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux64_asan_rel_ng/buil...)
  v8_linux64_gyp_rel_ng on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux64_gyp_rel_ng/build...)
  v8_linux64_rel_ng on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux64_rel_ng/builds/14779)
  v8_linux_arm64_rel_ng on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux_arm64_rel_ng/build...)
  v8_linux_dbg_ng on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux_dbg_ng/builds/14795)
  v8_linux_gcc_compile_rel on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux_gcc_compile_rel/bu...)
  v8_linux_mips64el_compile_rel on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux_mips64el_compile_r...)
  v8_linux_mipsel_compile_rel on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux_mipsel_compile_rel...)
  v8_linux_nodcheck_rel_ng on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux_nodcheck_rel_ng/bu...)

[Hannes Payer (out of office), 2016-10-18 11:22:25]

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

[commit-bot: I haz the power, 2016-10-18 11:22:33]

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-10-18 11:26:23]

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

[commit-bot: I haz the power, 2016-10-18 11:26:24]

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

[Hannes Payer (out of office), 2016-10-18 11:37:30]

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

[commit-bot: I haz the power, 2016-10-18 11:37:40]

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

[Hannes Payer (out of office), 2016-10-18 11:40:55]

Patchset #5 (id:80001) has been deleted

[Hannes Payer (out of office), 2016-10-18 11:47:32]

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

[commit-bot: I haz the power, 2016-10-18 11:47:46]

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-10-18 13:07:58]

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

[commit-bot: I haz the power, 2016-10-18 13:07:59]

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

[Hannes Payer (out of office), 2016-10-19 16:49:44]

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

[commit-bot: I haz the power, 2016-10-19 16:49:53]

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

[Hannes Payer (out of office), 2016-10-19 16:51:50]

Description was changed from

==========
Update implementation of atomics with latest Chromium version.

BUG=
==========

to

==========
Update implementation of atomics with latest Chromium version but use compiler
builtin atomics

BUG=
==========

[commit-bot: I haz the power, 2016-10-19 17:03:19]

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

[commit-bot: I haz the power, 2016-10-19 17:03:20]

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

[Hannes Payer (out of office), 2016-10-19 19:21:02]

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

[commit-bot: I haz the power, 2016-10-19 19:21:13]

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-10-19 19:57:56]

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

[commit-bot: I haz the power, 2016-10-19 19:57:57]

Dry run: This issue passed the CQ dry run.

[Hannes Payer (out of office), 2016-10-20 07:36:10]

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

[commit-bot: I haz the power, 2016-10-20 07:36:21]

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-10-20 08:10:14]

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

[commit-bot: I haz the power, 2016-10-20 08:10:15]

Dry run: This issue passed the CQ dry run.

[Hannes Payer (out of office), 2016-10-20 08:26:14]

hpayer@chromium.org changed reviewers:
+ mlippautz@chromium.org, ulan@chromium.org

[Hannes Payer (out of office), 2016-10-20 08:27:10]

Description was changed from

==========
Update implementation of atomics with latest Chromium version but use compiler
builtin atomics

BUG=
==========

to

==========
Update implementation of atomics with latest Chromium version but use compiler
builtin atomics

Ideally, we would use the standard library. However, when we are compiling
against an older version of the standard library the atomic implementation may
be slow.

BUG=
==========

[Michael Lippautz, 2016-10-20 10:28:24]

lgtm from my side

https://codereview.chromium.org/2425963002/diff/150001/src/base/atomicops_int...
File src/base/atomicops_internals_portable.h (right):

https://codereview.chromium.org/2425963002/diff/150001/src/base/atomicops_int...
src/base/atomicops_internals_portable.h:46: // assertion should detect this
issue, were it to fire then this header
Last sentences of this paragraph do not apply for our implementation as we use
the intrinsics and not std::...

[Jarin, 2016-10-20 11:03:38]

jarin@chromium.org changed reviewers:
+ jarin@chromium.org

[Jarin, 2016-10-20 11:03:39]

https://codereview.chromium.org/2425963002/diff/150001/src/base/atomicops_int...
File src/base/atomicops_internals_portable.h (right):

https://codereview.chromium.org/2425963002/diff/150001/src/base/atomicops_int...
src/base/atomicops_internals_portable.h:26: //    implemented as sequentially
consistent fence followed by a relaxed load.
We do not use acquire-store and release-load. How about removing them?

https://codereview.chromium.org/2425963002/diff/150001/src/base/atomicops_int...
src/base/atomicops_internals_portable.h:111: __atomic_store_n(ptr, value,
__ATOMIC_RELEASE);
This actually makes me a bit worried. Are we sure we never use Release_Store as
sequentially consistent store (which we cannot express otherwise)?

The same for Acquire_Load.

It would be safer to use __ATOMIC_SEQ_CST here and for acquire loads.

[ulan, 2016-10-20 12:09:56]

lgtm!

[Michael Lippautz, 2016-10-20 12:30:15]

https://codereview.chromium.org/2425963002/diff/150001/src/base/atomicops_int...
File src/base/atomicops_internals_portable.h (right):

https://codereview.chromium.org/2425963002/diff/150001/src/base/atomicops_int...
src/base/atomicops_internals_portable.h:46: // assertion should detect this
issue, were it to fire then this header
On 2016/10/20 10:28:24, Michael Lippautz wrote:
> Last sentences of this paragraph do not apply for our implementation as we use
> the intrinsics and not std::...

+1 on removing the weirdos.

[Jarin, 2016-10-20 12:59:25]

lgtm

[Hannes Payer (out of office), 2016-10-20 17:48:02]

https://codereview.chromium.org/2425963002/diff/150001/src/base/atomicops_int...
File src/base/atomicops_internals_portable.h (right):

https://codereview.chromium.org/2425963002/diff/150001/src/base/atomicops_int...
src/base/atomicops_internals_portable.h:26: //    implemented as sequentially
consistent fence followed by a relaxed load.
On 2016/10/20 11:03:39, Jarin wrote:
> We do not use acquire-store and release-load. How about removing them?

Done.

https://codereview.chromium.org/2425963002/diff/150001/src/base/atomicops_int...
src/base/atomicops_internals_portable.h:46: // assertion should detect this
issue, were it to fire then this header
On 2016/10/20 10:28:24, Michael Lippautz wrote:
> Last sentences of this paragraph do not apply for our implementation as we use
> the intrinsics and not std::...

Done.

https://codereview.chromium.org/2425963002/diff/150001/src/base/atomicops_int...
src/base/atomicops_internals_portable.h:111: __atomic_store_n(ptr, value,
__ATOMIC_RELEASE);
On 2016/10/20 11:03:39, Jarin wrote:
> This actually makes me a bit worried. Are we sure we never use Release_Store
as
> sequentially consistent store (which we cannot express otherwise)?
> 
> The same for Acquire_Load.
> 
> It would be safer to use __ATOMIC_SEQ_CST here and for acquire loads.

As discussed offline, we will look at performance impact in a follow-up cl.

[Hannes Payer (out of office), 2016-10-20 17:48:09]

The CQ bit was checked by hpayer@chromium.org

[Hannes Payer (out of office), 2016-10-20 17:48:10]

The patchset sent to the CQ was uploaded after l-g-t-m from
mlippautz@chromium.org, ulan@chromium.org, jarin@chromium.org
Link to the patchset: https://codereview.chromium.org/2425963002/#ps170001
(title: "comments")

[commit-bot: I haz the power, 2016-10-20 17:48:20]

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-10-20 17:54:30]

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

[commit-bot: I haz the power, 2016-10-20 17:54:31]

Try jobs failed on following builders:
  v8_win64_rel_ng on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_win64_rel_ng/builds/16504)
  v8_win_compile_dbg on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_win_compile_dbg/builds/2...)
  v8_win_rel_ng on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_win_rel_ng/builds/16588)

[Hannes Payer (out of office), 2016-10-21 06:38:13]

The CQ bit was checked by hpayer@chromium.org

[Hannes Payer (out of office), 2016-10-21 06:38:13]

The patchset sent to the CQ was uploaded after l-g-t-m from
mlippautz@chromium.org, ulan@chromium.org, jarin@chromium.org
Link to the patchset: https://codereview.chromium.org/2425963002/#ps190001
(title: "fix tests")

[commit-bot: I haz the power, 2016-10-21 06:38:23]

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-10-21 07:32:48]

Description was changed from

==========
Update implementation of atomics with latest Chromium version but use compiler
builtin atomics

Ideally, we would use the standard library. However, when we are compiling
against an older version of the standard library the atomic implementation may
be slow.

BUG=
==========

to

==========
Update implementation of atomics with latest Chromium version but use compiler
builtin atomics

Ideally, we would use the standard library. However, when we are compiling
against an older version of the standard library the atomic implementation may
be slow.

BUG=
==========

[commit-bot: I haz the power, 2016-10-21 07:32:49]

Committed patchset #10 (id:190001)

[Michael Achenbach, 2016-10-21 08:09:34]

A revert of this CL (patchset #10 id:190001) has been created in
https://chromiumcodereview.appspot.com/2438983002/ by machenbach@chromium.org.

The reason for reverting is:
https://build.chromium.org/p/client.v8.ports/builders/V8%20Android%20Arm64%20....

[commit-bot: I haz the power, 2016-11-17 22:09:01]

Description was changed from

==========
Update implementation of atomics with latest Chromium version but use compiler
builtin atomics

Ideally, we would use the standard library. However, when we are compiling
against an older version of the standard library the atomic implementation may
be slow.

BUG=
==========

to

==========
Update implementation of atomics with latest Chromium version but use compiler
builtin atomics

Ideally, we would use the standard library. However, when we are compiling
against an older version of the standard library the atomic implementation may
be slow.

BUG=

Committed: https://crrev.com/343c4ebdd13c086e732e57f0c834556f36473d97
Cr-Commit-Position: refs/heads/master@{#40488}
==========

[commit-bot: I haz the power, 2016-11-17 22:09:02]

Patchset 10 (id:??) landed as
https://crrev.com/343c4ebdd13c086e732e57f0c834556f36473d97
Cr-Commit-Position: refs/heads/master@{#40488}