[cfe-commits] r101756 - in /cfe/trunk: include/clang/AST/Decl.h lib/CodeGen/CGDecl.cpp lib/CodeGen/CGExpr.cpp lib/CodeGen/CodeGenModule.h test/CodeGenCXX/static-local-in-local-class.cpp

[Daniel Dunbar]

Hi Fariborz,

On Sun, Apr 18, 2010 at 2:01 PM, Fariborz Jahanian <fjahanian at apple.com> wrote:
> Author: fjahanian
> Date: Sun Apr 18 16:01:23 2010
> New Revision: 101756
>
> URL: http://llvm.org/viewvc/llvm-project?rev=101756&view=rev
> Log:
> Local static variables must be available module-wise
> as they are accessible in static methods in a class
> local to the same function. Fixes PR6769.

Whee, I didn't even know that was legal. This patch has a couple
problems, the biggest one is that we are inserting *very* local decl
into an extra map now. This is bad from a performance perspective.

I don't see a very elegant solution to this yet, but perhaps it makes
sense for Sema to mark Decl's where the static function-local storage
must be promoted to the translation unit. Among other things this
means we wouldn't be sticking these things in the extra map for the
non-C++ path.

Some other nits below...

> Added:
>    cfe/trunk/test/CodeGenCXX/static-local-in-local-class.cpp
> Modified:
>    cfe/trunk/include/clang/AST/Decl.h
>    cfe/trunk/lib/CodeGen/CGDecl.cpp
>    cfe/trunk/lib/CodeGen/CGExpr.cpp
>    cfe/trunk/lib/CodeGen/CodeGenModule.h
>
> Modified: cfe/trunk/include/clang/AST/Decl.h
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/Decl.h?rev=101756&r1=101755&r2=101756&view=diff
> ==============================================================================
> --- cfe/trunk/include/clang/AST/Decl.h (original)
> +++ cfe/trunk/include/clang/AST/Decl.h Sun Apr 18 16:01:23 2010
> @@ -548,6 +548,12 @@
>     return getStorageClass() <= Register;
>   }
>
> +  /// isStaticLocal - Returns tru if a variable with function scope is a
> +  /// static local variable.

tru -> true

> +  bool isStaticLocal() const {
> +    return getStorageClass() == Static && !isFileVarDecl();
> +  }
> +
>   /// hasExternStorage - Returns true if a variable has extern or
>   /// __private_extern__ storage.
>   bool hasExternalStorage() const {
>
> Modified: cfe/trunk/lib/CodeGen/CGDecl.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGDecl.cpp?rev=101756&r1=101755&r2=101756&view=diff
> ==============================================================================
> --- cfe/trunk/lib/CodeGen/CGDecl.cpp (original)
> +++ cfe/trunk/lib/CodeGen/CGDecl.cpp Sun Apr 18 16:01:23 2010
> @@ -205,6 +205,7 @@
>   // Store into LocalDeclMap before generating initializer to handle
>   // circular references.
>   DMEntry = GV;
> +  CGM.setStaticLocalDeclMap(&D, GV);
>
>   // Make sure to evaluate VLA bounds now so that we have them for later.
>   //
>
> Modified: cfe/trunk/lib/CodeGen/CGExpr.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGExpr.cpp?rev=101756&r1=101755&r2=101756&view=diff
> ==============================================================================
> --- cfe/trunk/lib/CodeGen/CGExpr.cpp (original)
> +++ cfe/trunk/lib/CodeGen/CGExpr.cpp Sun Apr 18 16:01:23 2010
> @@ -1106,6 +1106,8 @@
>     bool NonGCable = VD->hasLocalStorage() && !VD->hasAttr<BlocksAttr>();
>
>     llvm::Value *V = LocalDeclMap[VD];
> +    if (!V && VD->isStaticLocal())
> +      V = CGM.getStaticLocalDeclMap(VD);
>     assert(V && "DeclRefExpr not entered in LocalDeclMap?");
>
>     Qualifiers Quals = MakeQualifiers(E->getType());
>
> Modified: cfe/trunk/lib/CodeGen/CodeGenModule.h
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CodeGenModule.h?rev=101756&r1=101755&r2=101756&view=diff
> ==============================================================================
> --- cfe/trunk/lib/CodeGen/CodeGenModule.h (original)
> +++ cfe/trunk/lib/CodeGen/CodeGenModule.h Sun Apr 18 16:01:23 2010
> @@ -133,6 +133,7 @@
>
>   llvm::StringMap<llvm::Constant*> CFConstantStringMap;
>   llvm::StringMap<llvm::Constant*> ConstantStringMap;
> +  llvm::DenseMap<const Decl*, llvm::Value*> StaticLocalDeclMap;
>
>   /// CXXGlobalInits - Global variables with initializers that need to run
>   /// before main.
> @@ -170,6 +171,14 @@
>   /// been configured.
>   bool hasObjCRuntime() { return !!Runtime; }
>
> +  llvm::Value *getStaticLocalDeclMap(const VarDecl *VD) {
> +    return StaticLocalDeclMap[VD];
> +  }
> +  void setStaticLocalDeclMap(const VarDecl *D,
> +                             llvm::GlobalVariable *GV) {
> +    StaticLocalDeclMap[D] = GV;
> +  }

These functions are not named well. A function named
'getStaticLocalDeclMap' should return a map, not a map entry. Same for
set...

 - Daniel

> +
>   CGDebugInfo *getDebugInfo() { return DebugInfo; }
>   ASTContext &getContext() const { return Context; }
>   const CodeGenOptions &getCodeGenOpts() const { return CodeGenOpts; }
>
> Added: cfe/trunk/test/CodeGenCXX/static-local-in-local-class.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenCXX/static-local-in-local-class.cpp?rev=101756&view=auto
> ==============================================================================
> --- cfe/trunk/test/CodeGenCXX/static-local-in-local-class.cpp (added)
> +++ cfe/trunk/test/CodeGenCXX/static-local-in-local-class.cpp Sun Apr 18 16:01:23 2010
> @@ -0,0 +1,21 @@
> +// RUN: %clang_cc1 -emit-llvm -o %t %s
> +// PR6769
> +
> +struct X {
> +  static void f();
> +};
> +
> +void X::f() {
> +  static int *i;
> +  {
> +    struct Y {
> +      static void g() {
> +        i = new int();
> +       *i = 100;
> +       (*i) = (*i) +1;
> +      }
> +    };
> +    (void)Y::g();
> +  }
> +  (void)i;
> +}
>
>
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
>