llvm dev - Sep 2007 - [LLVMdev] RFC: Patch

Hi all,

I have a patch that would potentially help Objective-C 2. In the
"make_decl_llvm()" in llvm-backend.cpp, there is code to search
through the already generated global variables. Objective-C goes
through this code twice with the same identifier. The first time, is
to create meta-data for a "property". The second time is to add a
reference to that property into the correct ASM section.

Okay. so, the first time through, the variable's created and all is
good. It has "internal global" linkage, which is correct. The second
time through, it tries to look up that variable in the module, but
because it has "internal global" (and not "external global")
linkage,
the "getGlobalVariable()" method returns null.

This patch uses the "getNamedGlobal" method, which ignores whether
it's internally or externally global.

My question is, is this liable to break something else down the line?
Should it be modified to call the getNamedGlobal method only if it's
an Objective-C property? Is this even the correct method for an
Objective-C property?

Thanks!
-bw

Index: llvm-backend.cpp
==================================================================---
llvm-backend.cpp	(revision 42205)
+++ llvm-backend.cpp	(working copy)
@@ -1059,7 +1059,7 @@
     } else {
       // If the global has a name, prevent multiple vars with the
same name from
       // being created.
-      GlobalVariable *GVE = TheModule->getGlobalVariable(Name);
+      GlobalVariable *GVE = TheModule->getNamedGlobal(Name);

       if (GVE == 0) {
         GV = new GlobalVariable(Ty, false, GlobalValue::ExternalLinkage,0,

Hi Bill, why not just add "true" to the getGlobalVariable call?

  GlobalVariable *getGlobalVariable(const std::string &Name,
                                    bool AllowInternal = false) const;

Ciao,

Duncan.

On Sep 21, 2007, at 11:50 PM, Duncan Sands wrote:
> Hi Bill, why not just add "true" to the getGlobalVariable call?
>
>   GlobalVariable *getGlobalVariable(const std::string &Name,
>                                     bool AllowInternal = false) const;
>
That would work too. The getNamedVariable() is just a wrapper to  
getGlobalVariable() but passing "true" in. :-)

-bw

On Sep 21, 2007, at 5:11 PM, Bill Wendling wrote:
>
> My question is, is this liable to break something else down the line?
> Should it be modified to call the getNamedGlobal method only if it's
> an Objective-C property? Is this even the correct method for an
> Objective-C property?
There is a bigger question here.  One invariant that is useful is  
that there is only a single decl that corresponds to a given global  
variable.  It sounds like the objc front-end is making two GCC decl  
nodes for the same global variable?  If so, is there a way to fix the  
objc front-end to merge them?

-Chris

On Sep 22, 2007, at 10:57 AM, Chris Lattner wrote:
> On Sep 21, 2007, at 5:11 PM, Bill Wendling wrote:
>> My question is, is this liable to break something else down the line?
>> Should it be modified to call the getNamedGlobal method only if
it's
>> an Objective-C property? Is this even the correct method for an
>> Objective-C property?
>
> There is a bigger question here.  One invariant that is useful is
> that there is only a single decl that corresponds to a given global
> variable.  It sounds like the objc front-end is making two GCC decl
> nodes for the same global variable?  If so, is there a way to fix the
> objc front-end to merge them?
>
According to Fariborz, this is the correct behavior. This second time  
through, it seems as if it's simply trying to do a lookup of the  
(static) global variable "Foo". In essence, it's equivalent to
this
code:

	static struct S {
		int i;
	} Foo; // (1)

	static struct S *Ref = &Foo; // (2)

When I run this through, we only get the "Foo" at (2) going through  
the make_decl_llvm function and not the first one. I think that this  
is because there isn't any metadata that needs to be attached to the  
Foo object.

The code in objc/objc-act.c is acting like this: in finish_objc(), it  
has:

   if (protocol_chain)
     {
       if (flag_objc_abi == 2)
         {
           generate_v2_protocols ();    /* (1) */
         }
       else
         generate_protocols ();
     }
   /* APPLE LOCAL begin radar 4533974 - ObjC new protocol */
   if (protocol_list_chain)
     build_protocol_list_address_table ();
   if (protocollist_ref_chain)
     build_protocollist_translation_table (); /* (2) */

The call at (1) creates the metadata. It calls finish_var_decl(),  
which creates the first decl. The call at (2) builds the reference  
and also calls finish_var_decl(), which tries to do the lookup, but  
fails because the first decl was marked "internal global". So it  
tries to create a new decl, and kablooey. ;-)

The amount that I don't understand the obj-c code can fill  
volumes. :-) We could ask Fariborz if merging them is possible. My  
hunch is "no", but I could be wrong.

-bw