Remove old/unused atomicops code

Remove old/unused atomicops code

None of these gcc versions of atomicopts are used in
chromium anymore.  The portable versions were added in:
https://codereview.chromium.org/636783002 and all our
GCC-based targets use those now.

This change mostly just removes dead code.

Some files were added to base/allocator/BUILD.gn in order
to have it match allocator.gyp.   These were not required
before but are now (specifically tcmalloc's
base/atomicops-internals-x86.cc).  We got away without it
previously since base/atomicops_internals_x86_gcc.cc
provided the same symbol, but the symbol collision was
arguably a bug.

BUG=423074
Committed: https://crrev.com/b030801fcf447602931b3e4b8f94aae8053c61fb
Cr-Commit-Position: refs/heads/master@{#337123}

[Sam Clegg, 2015-06-26 21:53:54]

sbc@chromium.org changed reviewers:
+ jfb@chromium.org

[Sam Clegg, 2015-06-26 23:18:32]

sbc@chromium.org changed reviewers:
+ thestig@chromium.org

[JF, 2015-06-29 16:51:30]

jfb@chromium.org changed reviewers:
+ danalbert@google.com, rmcilroy@chromium.org

[JF, 2015-06-29 16:51:30]

I think atomicops_internals_gcc.h is only there for the NaCl toolchain (and
could be removed in favor of the portable version).

I don't think we can remove the ISA-specific versions because Android has
STLPort which doesn't have <atomic>. Android also relies on kernel user helpers
[1] to support older cores, and I don't think we can deprecate these at the
moment because some C++ compilers assume more recent cores (so even with
<atomic> support the generated code needs to be properly vetted on supported
compilers).

rmcilroy/danalbert, could you confirm the Android state of things?


  [1]: https://www.kernel.org/doc/Documentation/arm/kernel_user_helpers.txt

[rmcilroy, 2015-06-29 16:58:52]

On 2015/06/29 16:51:30, JF wrote:
> I think atomicops_internals_gcc.h is only there for the NaCl toolchain (and
> could be removed in favor of the portable version).
> 
> I don't think we can remove the ISA-specific versions because Android has
> STLPort which doesn't have <atomic>. Android also relies on kernel user
helpers
> [1] to support older cores, and I don't think we can deprecate these at the
> moment because some C++ compilers assume more recent cores (so even with
> <atomic> support the generated code needs to be properly vetted on supported
> compilers).
> 
> rmcilroy/danalbert, could you confirm the Android state of things?
> 
> 
>   [1]: https://www.kernel.org/doc/Documentation/arm/kernel_user_helpers.txt

I don't think we are using stlport on Android any longer, we have moved to
libc++ as far as I know. Saying that, I'm not sure on the state of <atomic> in
the Android libc++ (I assume it works, but I don't know if it uses the
kernel-user helper functions on older devices, so I'm not sure on performance).
danalbert - can you comment on this?

[JF, 2015-06-29 17:10:02]

> I don't think we are using stlport on Android any longer, we have moved to
> libc++ as far as I know. Saying that, I'm not sure on the state of <atomic> in
> the Android libc++ (I assume it works, but I don't know if it uses the
> kernel-user helper functions on older devices, so I'm not sure on
performance).
> danalbert - can you comment on this?

Could you also help figure out which versions of Android that Chrome still needs
to build for? We don't want to do a migration that works for recent Android
versions' toolchains but not older toolchains. Or can we use a newer toolchain
to build Chrome for an older Android version?

[rmcilroy, 2015-06-29 17:29:49]

On 2015/06/29 17:10:02, JF wrote:
> > I don't think we are using stlport on Android any longer, we have moved to
> > libc++ as far as I know. Saying that, I'm not sure on the state of <atomic>
in
> > the Android libc++ (I assume it works, but I don't know if it uses the
> > kernel-user helper functions on older devices, so I'm not sure on
> performance).
> > danalbert - can you comment on this?
> 
> Could you also help figure out which versions of Android that Chrome still
needs
> to build for? We don't want to do a migration that works for recent Android
> versions' toolchains but not older toolchains. Or can we use a newer toolchain
> to build Chrome for an older Android version?

We build Chrome for Jellybean and above. We only build a single APK which runs
on all versions so we use the same (newer) NDK toolchain for all Android
versions. The c++ library is statically linked into the binary.

[Sam Clegg, 2015-06-29 17:44:33]

On 2015/06/29 17:29:49, rmcilroy wrote:
> On 2015/06/29 17:10:02, JF wrote:
> > > I don't think we are using stlport on Android any longer, we have moved to
> > > libc++ as far as I know. Saying that, I'm not sure on the state of
<atomic>
> in
> > > the Android libc++ (I assume it works, but I don't know if it uses the
> > > kernel-user helper functions on older devices, so I'm not sure on
> > performance).
> > > danalbert - can you comment on this?
> > 
> > Could you also help figure out which versions of Android that Chrome still
> needs
> > to build for? We don't want to do a migration that works for recent Android
> > versions' toolchains but not older toolchains. Or can we use a newer
toolchain
> > to build Chrome for an older Android version?
> 
> We build Chrome for Jellybean and above. We only build a single APK which runs
> on all versions so we use the same (newer) NDK toolchain for all Android
> versions. The c++ library is statically linked into the binary.

Indeed if the android build relied on the ABI version, then this change would
fail the android trybots with #error.  But its green, which implies the ABI
versions are completely un-used.

[Sam Clegg, 2015-06-29 17:45:36]

On 2015/06/29 16:51:30, JF wrote:
> I think atomicops_internals_gcc.h is only there for the NaCl toolchain (and
> could be removed in favor of the portable version).

I'm looking into that as a separate change.  For this change I'd simply like to
remove the currently unused versions.

> 
> I don't think we can remove the ISA-specific versions because Android has
> STLPort which doesn't have <atomic>. Android also relies on kernel user
helpers
> [1] to support older cores, and I don't think we can deprecate these at the
> moment because some C++ compilers assume more recent cores (so even with
> <atomic> support the generated code needs to be properly vetted on supported
> compilers).
> 
> rmcilroy/danalbert, could you confirm the Android state of things?
> 
> 
>   [1]: https://www.kernel.org/doc/Documentation/arm/kernel_user_helpers.txt

[Sam Clegg, 2015-06-29 17:46:14]

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

[commit-bot: I haz the power, 2015-06-29 17:47:02]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1211903003/100001

[JF, 2015-06-29 18:27:26]

OK, it sounds like Android doesn't need these headers then. That's great! I
remember trying to remove these headers a while ago but they were still
required.

This lgtm then, assuming danalbert concurs.

[commit-bot: I haz the power, 2015-06-29 19:32:59]

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

[commit-bot: I haz the power, 2015-06-29 19:33:00]

Dry run: This issue passed the CQ dry run.

[Sam Clegg, 2015-06-29 19:36:26]

sbc@chromium.org changed reviewers:
+ thakis@chromium.org
- thestig@chromium.org

[Sam Clegg, 2015-06-29 19:36:26]

+thakis for base/OWNERS and thestig is OOO

[Sam Clegg, 2015-07-01 18:28:26]

Patchset #7 (id:120001) has been deleted

[Sam Clegg, 2015-07-01 18:28:47]

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

[Sam Clegg, 2015-07-01 18:28:47]

The patchset sent to the CQ was uploaded after l-g-t-m from jfb@chromium.org
Link to the patchset: https://codereview.chromium.org/1211903003/#ps140001
(title: "also remove _gcc.h")

[Sam Clegg, 2015-07-01 18:29:24]

On 2015/06/29 17:45:36, Sam Clegg wrote:
> On 2015/06/29 16:51:30, JF wrote:
> > I think atomicops_internals_gcc.h is only there for the NaCl toolchain (and
> > could be removed in favor of the portable version).
> 
> I'm looking into that as a separate change.  For this change I'd simply like
to
> remove the currently unused versions.
>

Actually atomicops_internals_gcc.h is not used anymore either.  Removed that
too.

[commit-bot: I haz the power, 2015-07-01 18:29:38]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1211903003/140001

[JF, 2015-07-01 18:32:41]

Very cool! Still lgtm.

[Nico, 2015-07-01 18:49:08]

lgtm except for the bootstrap.py change

https://codereview.chromium.org/1211903003/diff/140001/base/allocator/BUILD.gn
File base/allocator/BUILD.gn (right):

https://codereview.chromium.org/1211903003/diff/140001/base/allocator/BUILD.g...
base/allocator/BUILD.gn:72: "$tcmalloc_dir/src/base/atomicops-internals-x86.h",
Would the generic atomics work for tcmalloc too?

https://codereview.chromium.org/1211903003/diff/140001/tools/gn/bootstrap/boo...
File tools/gn/bootstrap/bootstrap.py (right):

https://codereview.chromium.org/1211903003/diff/140001/tools/gn/bootstrap/boo...
tools/gn/bootstrap/bootstrap.py:146: 'base/base_paths.cc',
This probably now needs to reference the new atomics file?

[Sam Clegg, 2015-07-01 18:52:59]

https://codereview.chromium.org/1211903003/diff/140001/base/allocator/BUILD.gn
File base/allocator/BUILD.gn (right):

https://codereview.chromium.org/1211903003/diff/140001/base/allocator/BUILD.g...
base/allocator/BUILD.gn:72: "$tcmalloc_dir/src/base/atomicops-internals-x86.h",
On 2015/07/01 18:49:07, Nico wrote:
> Would the generic atomics work for tcmalloc too?

Sure.  Makes sense not to have multiple copies of such things,
 but that should be a separate change.

Also, the tcmalloc version we use here is many years old.  It could use a
refresh, but all the refresh instructions are still svn based.

https://codereview.chromium.org/1211903003/diff/140001/tools/gn/bootstrap/boo...
File tools/gn/bootstrap/bootstrap.py (right):

https://codereview.chromium.org/1211903003/diff/140001/tools/gn/bootstrap/boo...
tools/gn/bootstrap/bootstrap.py:146: 'base/base_paths.cc',
On 2015/07/01 18:49:07, Nico wrote:
> This probably now needs to reference the new atomics file?

There are no .cc files needed for atomics anymore, only headers.  From the looks
of it bootstrap.py doesn't mention headers anywhere.  Will at least make sure it
builds..

[Nico, 2015-07-01 18:55:09]

https://codereview.chromium.org/1211903003/diff/140001/base/allocator/BUILD.gn
File base/allocator/BUILD.gn (right):

https://codereview.chromium.org/1211903003/diff/140001/base/allocator/BUILD.g...
base/allocator/BUILD.gn:72: "$tcmalloc_dir/src/base/atomicops-internals-x86.h",
On 2015/07/01 18:52:58, Sam Clegg wrote:
> On 2015/07/01 18:49:07, Nico wrote:
> > Would the generic atomics work for tcmalloc too?
> 
> Sure.  Makes sense not to have multiple copies of such things,
>  but that should be a separate change.

Oh yeah, totally.

> Also, the tcmalloc version we use here is many years old.  It could use a
> refresh, but all the refresh instructions are still svn based.

It's not just old, it's fairly different. From what I understand, there's now 3
tcmalloc versions, all pretty different:
1. Open-source tcmalloc
2. Google-internal tcmalloc, as far as I know that's not synced with open-source
tcmalloc at all
3. Chromium tcmalloc

:-/

[Nico, 2015-07-01 18:55:42]

https://codereview.chromium.org/1211903003/diff/140001/tools/gn/bootstrap/boo...
File tools/gn/bootstrap/bootstrap.py (right):

https://codereview.chromium.org/1211903003/diff/140001/tools/gn/bootstrap/boo...
tools/gn/bootstrap/bootstrap.py:146: 'base/base_paths.cc',
On 2015/07/01 18:52:58, Sam Clegg wrote:
> On 2015/07/01 18:49:07, Nico wrote:
> > This probably now needs to reference the new atomics file?
> 
> There are no .cc files needed for atomics anymore, only headers.  From the
looks
> of it bootstrap.py doesn't mention headers anywhere.  Will at least make sure
it
> builds..

Oh, and this sounds good. lgtm!

[Sam Clegg, 2015-07-01 19:41:00]

On 2015/07/01 18:55:42, Nico wrote:
>
https://codereview.chromium.org/1211903003/diff/140001/tools/gn/bootstrap/boo...
> File tools/gn/bootstrap/bootstrap.py (right):
> 
>
https://codereview.chromium.org/1211903003/diff/140001/tools/gn/bootstrap/boo...
> tools/gn/bootstrap/bootstrap.py:146: 'base/base_paths.cc',
> On 2015/07/01 18:52:58, Sam Clegg wrote:
> > On 2015/07/01 18:49:07, Nico wrote:
> > > This probably now needs to reference the new atomics file?
> > 
> > There are no .cc files needed for atomics anymore, only headers.  From the
> looks
> > of it bootstrap.py doesn't mention headers anywhere.  Will at least make
sure
> it
> > builds..
> 
> Oh, and this sounds good. lgtm!

gn bootstrap.py change verified (after updating it with other needed fixes:
https://codereview.chromium.org/1216153008/)

[commit-bot: I haz the power, 2015-07-01 20:00:44]

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

[commit-bot: I haz the power, 2015-07-01 20:00:46]

Dry run: This issue passed the CQ dry run.

[Sam Clegg, 2015-07-01 21:52:05]

The CQ bit was checked by sbc@chromium.org

[commit-bot: I haz the power, 2015-07-01 21:52:21]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1211903003/140001

[commit-bot: I haz the power, 2015-07-01 21:57:23]

Committed patchset #7 (id:140001)

[commit-bot: I haz the power, 2015-07-01 21:58:30]

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

[jdduke (slow), 2015-07-21 21:11:17]

A revert of this CL (patchset #7 id:140001) has been created in
https://codereview.chromium.org/1250853002/ by jdduke@chromium.org.

The reason for reverting is: Temporarily reverting until libc++ ships to stable
on Android..