[llvm-commits] [llvm-gcc-4.0] r42287 - in /llvm-gcc-4.0/trunk/gcc: c-common.h llvm-backend.cpp objc/objc-act.c stub-objc.c

[Bill Wendling]

On 9/25/07, Chris Lattner <clattner at apple.com> wrote:
> On Sep 24, 2007, at 7:51 PM, Bill Wendling wrote:
> > URL: http://llvm.org/viewvc/llvm-project?rev=42287&view=rev
> > Log:
> > During the processing of Objective-C "protocols", the objc frontend
> > creates two
> > decls for the protocol. One for the metadata and another for when it's
> > referenced. However, protocols are "internal global", so when we do
> > a lookup
> > for the reference, it doesn't find the first decl because it's not
> > "external".
> > This will perform a second lookup for objective C protocols if we
> > don't find
> > it among the "external globals".
>
> Hi Bill,
>
> This is a good short-term solution Bill, but I don't think it is what
> we want to keep.
>
I agree.

> Specifically, your new function adds a fairly expensive call to
> strstr to the global variable emission path for all languages.
>
It only does the check if this is objc. And further limits it in the
function call itself to only objc-2. I tried to limit it as much as
possible. :-)

> It would be better to merge the vardecls in objc-act.c where it
> creates them.  I see this code:
>
> /* Build decl = initializer; for each protocol referenced in @protocol
> (MyProtole) expression. */
>
> static void
> build_protocollist_translation_table (void)
> {
>    tree chain;
>    static char string[BUFSIZE];
>
>    for (chain = protocollist_ref_chain; chain; chain = TREE_CHAIN
> (chain))
>      {
>        tree expr = TREE_VALUE (chain);
>        tree decl = TREE_PURPOSE (chain);
>        gcc_assert (TREE_CODE (expr) == PROTOCOL_INTERFACE_TYPE);
>        /* APPLE LOCAL begin radar 4695109 */
>        sprintf (string, "_OBJC_PROTOCOL_$_%s",
>                IDENTIFIER_POINTER (PROTOCOL_NAME (expr)));
>        expr = start_var_decl (objc_v2_protocol_template, string);
>        /* APPLE LOCAL end radar 4695109 */
>        expr = convert (objc_protocol_type, build_fold_addr_expr (expr));
>        finish_var_decl (decl, expr);
>      }
> }
>
> and this:
>
> static void
> build_v2_protocol_reference (tree p)
> {
>    tree decl;
>    const char *proto_name;
>
>    /* static struct protocol_t  _OBJC_PROTOCOL_$<mumble>; */
>
>    proto_name = synth_id_with_class_suffix ("_OBJC_PROTOCOL_$", p);
>    decl = start_var_decl (objc_v2_protocol_template, proto_name);
>    PROTOCOL_V2_FORWARD_DECL (p) = decl;
> }
>
> I'm not sure which happens first, but this is creating two DECL nodes
> with the same name.

I believe that "build_protocollist_translation_table ()" is called last.

>  Can you please investigate using lookup_name to
> only create the new var_decl node if it doesn't already exist?
>
> It might just be a matter of changing the code to looks like this:
>
>    proto_name = synth_id_with_class_suffix ("_OBJC_PROTOCOL_$", p);
>    if ((decl = lookup_name(proto_name)) == 0)
>      decl = start_var_decl (objc_v2_protocol_template, proto_name);
>    PROTOCOL_V2_FORWARD_DECL (p) = decl;
>
> Or something.
>
You read my mind! :-) I'm testing something akin to this right now,
actually. I'll let you know how it goes.

-bw