[llvm-commits] [llvm] r97759 - /llvm/trunk/lib/CodeGen/PseudoSourceValue.cpp

[Chris Lattner]

On Mar 4, 2010, at 2:15 PM, Jeffrey Yasskin wrote:

> Author: jyasskin
> Date: Thu Mar  4 16:15:01 2010
> New Revision: 97759
> 
> URL: http://llvm.org/viewvc/llvm-project?rev=97759&view=rev
> Log:
> Fix memcheck-found leaks: one false positive from using new[], and one true
> positive where pointers would be leaked on llvm_shutdown.

Nice, one question:

> +namespace {
> +struct PSVGlobalsTy {
> +  // PseudoSourceValues are immutable so don't need locking.
> +  const PseudoSourceValue PSVs[4];
> +  sys::Mutex Lock;  // Guards FSValues, but not the values inside it.
> +  std::map<int, const PseudoSourceValue *> FSValues;
> +
> +  PSVGlobalsTy() : PSVs() {}
> +  ~PSVGlobalsTy() {
> +    for (std::map<int, const PseudoSourceValue *>::iterator
> +           I = FSValues.begin(), E = FSValues.end(); I != E; ++I) {
> +      delete I->second;

Why does this need an std::map?  Aren't the keys dense integers?  Why not directly index into the array, allowing elimination of the lock?

-Chris


> +    }
> +  }
> +};
> +
> +static ManagedStatic<PSVGlobalsTy> PSVGlobals;
> +
> +}  // anonymous namespace
> 
> const PseudoSourceValue *PseudoSourceValue::getStack()
> -{ return &(*PSVs)[0]; }
> +{ return &PSVGlobals->PSVs[0]; }
> const PseudoSourceValue *PseudoSourceValue::getGOT()
> -{ return &(*PSVs)[1]; }
> +{ return &PSVGlobals->PSVs[1]; }
> const PseudoSourceValue *PseudoSourceValue::getJumpTable()
> -{ return &(*PSVs)[2]; }
> +{ return &PSVGlobals->PSVs[2]; }
> const PseudoSourceValue *PseudoSourceValue::getConstantPool()
> -{ return &(*PSVs)[3]; }
> +{ return &PSVGlobals->PSVs[3]; }
> 
> static const char *const PSVNames[] = {
>   "Stack",
> @@ -48,13 +67,13 @@
>         Subclass) {}
> 
> void PseudoSourceValue::printCustom(raw_ostream &O) const {
> -  O << PSVNames[this - *PSVs];
> +  O << PSVNames[this - PSVGlobals->PSVs];
> }
> 
> -static ManagedStatic<std::map<int, const PseudoSourceValue *> > FSValues;
> -
> const PseudoSourceValue *PseudoSourceValue::getFixedStack(int FI) {
> -  const PseudoSourceValue *&V = (*FSValues)[FI];
> +  PSVGlobalsTy &PG = *PSVGlobals;
> +  sys::ScopedLock locked(PG.Lock);
> +  const PseudoSourceValue *&V = PG.FSValues[FI];
>   if (!V)
>     V = new FixedStackPseudoSourceValue(FI);
>   return V;
> 
> 
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits