Implement sk_atomic_conditional_inc with sk_atomic_cas.

Implement sk_atomic_conditional_inc with sk_atomic_cas.

Now that we have sk_atomic_cas, we can replace all the platform-specific CAS loops with one.

BUG=skia:

Committed: http://code.google.com/p/skia/source/detail?r=14892

[bungeman-skia, 2014-05-27 14:20:19]

I agree that this is a good idea since it's the one non-trivial atomic needed in
SkThread. lgtm

https://codereview.chromium.org/300553003/diff/80001/include/core/SkThread.h
File include/core/SkThread.h (right):

https://codereview.chromium.org/300553003/diff/80001/include/core/SkThread.h#...
include/core/SkThread.h:55: prev = *addr;
Eck, it irks me that we have to do this load on all platforms, when on x86 the
cas tells us the previous value. Linked Load / Store Conditional platforms like
ARM actually don't know, which is why android only reports bool. I suppose on
all platforms the addr has to be loaded as part of the cas, so at least the
value at that point should be close at hand.

[mtklein, 2014-05-27 14:27:26]

https://codereview.chromium.org/300553003/diff/80001/include/core/SkThread.h
File include/core/SkThread.h (right):

https://codereview.chromium.org/300553003/diff/80001/include/core/SkThread.h#...
include/core/SkThread.h:55: prev = *addr;
On 2014/05/27 14:20:20, bungeman1 wrote:
> Eck, it irks me that we have to do this load on all platforms, when on x86 the
> cas tells us the previous value. Linked Load / Store Conditional platforms
like
> ARM actually don't know, which is why android only reports bool. I suppose on
> all platforms the addr has to be loaded as part of the cas, so at least the
> value at that point should be close at hand.

Agreed.  sk_atomic_cas is a bit least-common-denominator, but it seems good
enough.

[mtklein, 2014-05-27 14:27:51]

+reed for include/core.  No real API change.

[reed1, 2014-05-27 14:36:18]

https://codereview.chromium.org/300553003/diff/80001/include/core/SkThread.h
File include/core/SkThread.h (right):

https://codereview.chromium.org/300553003/diff/80001/include/core/SkThread.h#...
include/core/SkThread.h:56: if (0 == prev) {
Why the test/break? Does this warrent a comment?

[mtklein, 2014-05-27 14:38:29]

https://codereview.chromium.org/300553003/diff/80001/include/core/SkThread.h
File include/core/SkThread.h (right):

https://codereview.chromium.org/300553003/diff/80001/include/core/SkThread.h#...
include/core/SkThread.h:56: if (0 == prev) {
On 2014/05/27 14:36:18, reed1 wrote:
> Why the test/break? Does this warrent a comment?

Would return 0 be clearer?  That's the conditional part of the conditional_inc:
don't increment 0.

[reed1, 2014-05-27 14:44:18]

lgtm

https://codereview.chromium.org/300553003/diff/80001/include/core/SkThread.h
File include/core/SkThread.h (right):

https://codereview.chromium.org/300553003/diff/80001/include/core/SkThread.h#...
include/core/SkThread.h:56: if (0 == prev) {
On 2014/05/27 14:38:30, mtklein wrote:
> On 2014/05/27 14:36:18, reed1 wrote:
> > Why the test/break? Does this warrent a comment?
> 
> Would return 0 be clearer?  That's the conditional part of the
conditional_inc:
> don't increment 0.

Ah, no, my bad. I didn't read the bloody name of the (new) function.

Returning 0 explicitly might be clearer, but I'm not asking for that change
per-se.

[mtklein, 2014-05-27 14:49:34]

The CQ bit was checked by mtklein@google.com

[commit-bot: I haz the power, 2014-05-27 14:49:48]

CQ is trying da patch. Follow status at
 https://skia-tree-status.appspot.com/cq/mtklein@chromium.org/300553003/80001

[mtklein, 2014-05-27 14:54:55]

The CQ bit was checked by mtklein@google.com

[commit-bot: I haz the power, 2014-05-27 14:55:02]

CQ is trying da patch. Follow status at
 https://skia-tree-status.appspot.com/cq/mtklein@chromium.org/300553003/100001

[commit-bot: I haz the power, 2014-05-27 15:00:35]

Change committed as 14892