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

[Mehdi Amini]

Thanks for the answer.

> On Jun 12, 2015, at 9:42 AM, David Majnemer <david.majnemer at gmail.com> wrote:
> 
> 
> 
> On Fri, Jun 12, 2015 at 7:56 AM, Mehdi Amini <mehdi.amini at apple.com <mailto: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.)

I see, we’re fine with a race on the initialization in the sense that it can happen multiple times (since I guess getAllocationGranularity() is pure) but it has to be visible in each thread. 

Also I read online that MSVC 2013 should support thread-safe local static initialization (see: http://herbsutter.com/2013/09/09/visual-studio-2013-rc-is-now-available/ <http://herbsutter.com/2013/09/09/visual-studio-2013-rc-is-now-available/> )
Isn’t MSVC2013 the current minimal version for LLVM?

Thanks,

— 
Mehdi


>  
> 
> Thanks,
> 
> Mehdi
> 
> > On Jun 11, 2015, at 3:22 PM, Reid Kleckner <reid at kleckner.net <mailto: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 <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 <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 <mailto:llvm-commits at cs.uiuc.edu>
> > http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits <http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits>
> 
> 
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu <mailto:llvm-commits at cs.uiuc.edu>
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits <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/61ddcac5/attachment.html>