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