[cfe-dev] Patch for ContextDecl
Steve Naroff
snaroff at apple.com
Sun Apr 6 18:59:09 PDT 2008
Chris,
From my perspective, both techniques below are tedious (and disclose
implementation details that could otherwise be hidden).
if (II) {
// AST-based linked list...
- New->setNext(II->getFETokenInfo<ScopedDecl>());
- II->setFETokenInfo(New);
// Sema-based map...
+ IdDeclInfos[II].PushShadowed(New);
// Why isn't the following sufficient? Simple, no implementation
details disclosed...
S->AddDeclForIdentifier(New, II);
}
// ...we could then change IdentifierInfo to have an API similar to
Argiris's IdDeclInfo.
llvm::SmallVector<void *, 2> FETokenInfos;
/// Add a decl in the scope chain
void PushShadowed(void *D) { FETokenInfos.push_back(D); }
void PopShadowed() { FETokenInfos.pop_back(); }
This should...
- Allow us to remove the "Next" from ScopedDecl (which cleans up the
AST node).
- Remove the overhead associated with the map.
- Simplifies the API (no need to explicitly manages chains or
maps...). Keeps the low level details at a lower level...
Does this make sense? Am I missing something? (I've had a few glasses
of wine and may be over simplifying:-)
snaroff
On Apr 6, 2008, at 3:16 PM, Chris Lattner wrote:
>
> On Apr 6, 2008, at 1:32 PM, Argiris Kirtzidis wrote:
>
>> Chris Lattner wrote:
>>> What is actually going on here is that getNext is horribly
>>> overloaded for scope decls. When parsing is happening, the "next"
>>> pointer is used to represent the scope chains hanging off the
>>> identifier info. This keeps track of shadowing. However, after
>>> parsing is done, these fields are dead, so they are "repurposed" to
>>> be the linked list of decls within a context (e.g. functiondecl).
>>>
>>> A truly elegant solution would be to get rid of parsing-specific
>>> stuff from the AST, but I'm not sure how to do this while retaining
>>> efficiency at parse time for walking and maintaining the shadow
>>> stacks.
>> That's a good idea. A suggestion for this is the attached patch.
>
> Wow, this is an incredibly clever way to implement this. I like the
> idea in principle.
>
> However, it appears to be a very consistent slowdown for both carbon.h
> (a large C header) and Cocoa.h (a large objc header) with
> ENABLE_OPTIMIZED=1 DISABLE_ASSERTIONS=1:
>
> Before:
>
> $ time ./clang INPUTS/carbon_h.c
> 0.180u 0.058s 0:00.24 95.8% 0+0k 0+0io 0pf+0w
> $ time ./clang INPUTS/Cocoa_h.m
> 0.197u 0.066s 0:00.26 96.1% 0+0k 0+0io 0pf+0w
>
> After:
>
> $ time ~/llvm/Release-Asserts/bin/clang INPUTS/carbon_h.c
> 0.191u 0.061s 0:00.25 100.0% 0+0k 0+0io 0pf+0w
> $ time ~/llvm/Release-Asserts/bin/clang INPUTS/Cocoa_h.m
> 0.205u 0.069s 0:00.28 92.8% 0+0k 0+0io 0pf+0w
>
> That is a 6.1% slowdown on carbon.h and 4% slowdown on Cocoa.h. The
> number on this machine are very stable, three consecutive runs all
> produced the same timings. While I really would like to have the
> cleanup, I don't think the cost is worth it. When coming from a .i
> file (to reduce syscall/preprocessor overhead) the slowdown is 8.4%
> for carbon.h (0.119s -> 0.129s).
>
> What do you think? If you want, I can provide you with a .i file of
> cocoa.h or carbon.h.
>
> -Chris
> _______________________________________________
> cfe-dev mailing list
> cfe-dev at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-dev
More information about the cfe-dev
mailing list