[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

[Fariborz Jahanian]

On Apr 19, 2010, at 2:13 PM, Daniel Dunbar wrote:

> On Mon, Apr 19, 2010 at 11:47 AM, Fariborz Jahanian <fjahanian at apple.com 
> > wrote:
>>
>> On Apr 19, 2010, at 10:21 AM, Daniel Dunbar wrote:
>>
>>> 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
>>
>> This moves the burden to the front-end shared by every one. I  
>> prefer to
>> have the clients do the extra lifting if they need to.
>
> I don't understand what you mean, moving this to the front-end means
> that every decl can have a very simple predicate which is used by all
> clients. This *reduces* the burden on clients, not increases it.

I mean to say, that there is no need for a predicate to be set in the  
front-end.
If client needs to check for this condition (as IR gen need to do),  
then they can
call isStaticLocal() on the decl.

>
>
>>>
>>> means we wouldn't be sticking these things in the extra map for the
>>> non-C++ path.
>>
>> I did the insertion c++ specific for performance.
>
> I'm still not happy with this. It still penalizes all local decls,
> when this is a case that almost never happens. It is much better to
> have Sema clearly define the properties of things, and just let IRgen
> handle implementing them.

Not all local decls. Only local statics.  Local static have the proper  
Sema info.
already. It is just that if they are referenced in a method in a local  
class then
there is no Sema Info. Even with a Sema Info. added for this extremely  
radar case, IRgen still need to have
its decl. at Module level (as is done now). I am convinced that any  
potential performance
is extremely minimal.

- fariborz

>
>
> - Daniel
>
>>>
>>>
>>> Some other nits below...
>>
>> In r101794.
>>
>> - Thanks, Fariborz
>>
>>