code review 1020043: The comments in dfa indicate that at some point read me...

The comments in dfa indicate that at some point read memory barriers would
be added, as require for lax memory systems such as Alpha.
Here is a patch that adds memory barrier support on Alpha (although not
tested on an alpha).

This patch is used in Debian's re2, as we replace atomicops.h code with
inlined memory barrier functions provided by libatomic-ops. Thus we don't
need to support every architecture in RE2, but we do need read memory
barriers in the lock-free search code.

[Stefano Rivera, 2010-04-30 13:00:52]

Hello rsc (cc: re2-dev@googlegroups.com),

I'd like you to review this change.

[rsc1, 2010-04-30 16:09:07]

Thanks for adding this.
Can we call the read barrier

MaybeReadMemoryBarrier()

instead of ReadMemoryBarrierDepends()?
The Maybe form is already established in other
parts of the code (e.g., parse.cc), and the
Depends sounds like it might have to do with
dependencies.

Also, I want to confirm that on Debian you're
only using the read barrier on Alpha.
I fear that inserting a read barrier on the other
systems will slow things down unnecessarily.

Thanks.
Russ

[stefano_rivera.za.net, 2010-04-30 18:06:28]

Hi rsc (2010.04.30_18:09:07_+0200)
> Can we call the read barrier
> MaybeReadMemoryBarrier()

Sure. Depends came from the Linux kernel function name: rmb_depends()

> Also, I want to confirm that on Debian you're
> only using the read barrier on Alpha.
> I fear that inserting a read barrier on the other
> systems will slow things down unnecessarily.

Yes, here's the patch:
http://bzr.debian.org/loggerhead/collab-maint/re2/trunk/annotate/head:/debian...

SR

-- 
Stefano Rivera
  http://tumbleweed.org.za/
  H: +27 21 465 6908 C: +27 72 419 8559  UCT: x3127

[Stefano Rivera, 2010-04-30 18:10:38]

Hello rsc (cc: re2-dev@googlegroups.com),

Please take another look.

[rsc1, 2010-04-30 18:48:01]

LGTM

Thanks

[rsc, 2010-04-30 19:49:04]

*** Submitted as http://code.google.com/p/re2/source/detail?r=7f0b91b267fe ***

The comments in dfa indicate that at some point read memory barriers would
be added, as require for lax memory systems such as Alpha.
Here is a patch that adds memory barrier support on Alpha (although not
tested on an alpha).

This patch is used in Debian's re2, as we replace atomicops.h code with
inlined memory barrier functions provided by libatomic-ops. Thus we don't
need to support every architecture in RE2, but we do need read memory
barriers in the lock-free search code.

R=rsc
CC=re2-dev
http://codereview.appspot.com/1020043

Committer: Russ Cox <rsc@swtch.com>