[llvm] r239566 - [Support] Fix a race initializing a static local in MSVC

[David Majnemer]

On Fri, Jun 12, 2015 at 7:56 AM, Mehdi Amini <mehdi.amini at apple.com> wrote:

> Hi Reid,
>
> Can you explain how the change makes it thread safe? It’s not obvious to
> me right now.
>

In the MSVC world, loads marked volatile have acquire semantics and stores
marked volatile have release semantics.  We actually don't need such strong
semantics here, just a monotonic guarantee on the loads and stores which
volatile will more or less give us on GCC (for mingw builds and the like).

It contains a benign race but we are OK with that (two threads might both
compute the allocation granularity but they will both produce the same
result.)


>
> Thanks,
>
> Mehdi
>
> > On Jun 11, 2015, at 3:22 PM, Reid Kleckner <reid at kleckner.net> wrote:
> >
> > Author: rnk
> > Date: Thu Jun 11 17:22:45 2015
> > New Revision: 239566
> >
> > URL: http://llvm.org/viewvc/llvm-project?rev=239566&view=rev
> > Log:
> > [Support] Fix a race initializing a static local in MSVC
> >
> > static local initialization isn't thread safe with MSVC and a race was
> > reported in PR23817. We can't use std::atomic because it's not trivially
> > constructible, so instead do some lame volatile global integer
> > manipulation.
> >
> > Modified:
> >    llvm/trunk/lib/Support/Windows/Memory.inc
> >
> > Modified: llvm/trunk/lib/Support/Windows/Memory.inc
> > URL:
> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Support/Windows/Memory.inc?rev=239566&r1=239565&r2=239566&view=diff
> >
> ==============================================================================
> > --- llvm/trunk/lib/Support/Windows/Memory.inc (original)
> > +++ llvm/trunk/lib/Support/Windows/Memory.inc Thu Jun 11 17:22:45 2015
> > @@ -78,7 +78,15 @@ MemoryBlock Memory::allocateMappedMemory
> >   // While we'd be happy to allocate single pages, the Windows allocation
> >   // granularity may be larger than a single page (in practice, it is
> 64K)
> >   // so mapping less than that will create an unreachable fragment of
> memory.
> > -  static const size_t Granularity = getAllocationGranularity();
> > +  // Avoid using one-time initialization of static locals here, since
> they
> > +  // aren't thread safe with MSVC.
> > +  static volatile size_t GranularityCached;
> > +  size_t Granularity = GranularityCached;
> > +  if (Granularity == 0) {
> > +    Granularity = getAllocationGranularity();
> > +    GranularityCached = Granularity;
> > +  }
> > +
> >   const size_t NumBlocks = (NumBytes+Granularity-1)/Granularity;
> >
> >   uintptr_t Start = NearBlock ?
> reinterpret_cast<uintptr_t>(NearBlock->base()) +
> >
> >
> > _______________________________________________
> > llvm-commits mailing list
> > llvm-commits at cs.uiuc.edu
> > http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20150612/bd1b6bd1/attachment.html>