On 25 Mar 2008, at 22:03, Devang Patel wrote:


On Mar 25, 2008, at 10:00 AM, David Chisnall wrote:

+  void CollectIvarTypes(ObjCInterfaceDecl *ObjCClass,
+      std::vector<const llvm::Type*> *IvarTypes);

This is Objective-C specific class method, so rename it as
CollectObjCIvarTypes.
Prefer SmallVector instead of std::vector. Pass it as a reference
instead of a pointer.

Fixed passing by pointer.  Kept as std::vector because
StructType::get() takes a std::vector.

/// getLLVMFieldNo - Return llvm::StructType element number
/// that corresponds to the field FD.
unsigned getLLVMFieldNo(const FieldDecl *FD);
+  unsigned getLLVMFieldNo(QualType ObjectTy, const ObjCIvarDecl
*Decl);

Why do you need new method here ?

This gets the field number from an LLVM structure created from an
Objective-C class, rather than one created from a C structure.
Possibly it should have a different name?  Suggestions welcome.

In this method you do a field lookup by iterating over all ObjectTy
ivars. Would not it be more efficient to keep a map just like normal C
struct fields ?

You'd need to generate the map at some point, possibly the first time it's called? Since it works as-is, I'd rather do this later if profiling indicates this is a bottleneck.

+    // Warning: Use of this is strongly discouraged.  Late binding
of instance
+    // variables is supported on some runtimes and so using static
binding can
+    // break code when libraries are updated.  Only use this if
you have
+    // previously checked that the ObjCRuntime subclass in use
does not support
+    // late-bound ivars.

Please add assert() to check late-bound of ivars support.

I'd like to do this, but GodeGenTypes does not have a reference to
anything that has a reference to anything that has a reference to
the ObjCRuntime subclass, so it's non-trivial.  Suggestions welcome.

You may want to collect runtime info while constructing CodeGenTypes.

Doing this would require major changes to the structure of CodeGenModule, since its Types member would have to be initialised after the runtime was constructed. It's also not quite semantically correct - you should still be able to get the static structure of the object using @defs() at compile time, and the compiler should emit a warning that you are introducing stronger constraints on the ABI that the runtime requires (and thus susceptible to the fragile base class problem).

runtime_if.diff contains changes to the runtime interface.  This
probably needs to be committed at the same time as the changes to
the GNU runtime, or there might be some conflicts.  The Étoilé
patch may also conflict with the old definitions if it is
committed before this.

OK

-CGObjCRuntime *CreateObjCRuntime(llvm::Module &M);
+CGObjCRuntime *CreateObjCRuntime(llvm::Module &M,
+                                 const llvm::Type *LLVMIntType,
+                                 const llvm::Type *LLVMLongType);

Is not it possible to derive LLVMIntType and LLVMLongType info
based on Module ?

Maybe?  Can I have a hint please?


IIUC, these are target specific int and long types. In that case,
TargetInfo should provide you this info. I guess this OK for now.

This whole interface will probably be tweaked a few more times before it's finalised, but since it's only called in one place it's easy to fix later.

Thanks! Make sure while(AI) is not an infinite loop.

Well spotted.  It is now not an infinite loop...

David

Attachment: clang.diff
Description: Binary data


_______________________________________________
cfe-dev mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-dev

Reply via email to