On 30 Jun 2011, at 11:27, Richard Guenther wrote:

On Wed, Jun 29, 2011 at 7:47 PM, Iain Sandoe
<develo...@sandoe-acoustics.co.uk> wrote:
The bug arises because of the use, by the ObjC FE, of two old target macros that emit efficient representations of class definitions and references.

This 'works fine' (however wrong it might be conceptually), until LTO is
engaged, whereupon the definitions vanish without trace (since no
corresponding real variable is ever created).

---

The patch creates appropriate variables in the FE and tags them as ObjC
meta-data (in the same manner as is done for other objc meta-data).

We then intercept them in varasm.c with a hook that allows the target to declare that it has completely handled the output of a variable - allowing
us to handle them in the required special manner.

FAOD, It is necessary to preserve this mechanism for emitting the
definitions and references to permit linkage with existing system libraries.

----

If the patch is acceptable, then I would expect to follow up with a patch to remove ASM_DECLARE_CLASS_REFERENCE and ASM_DECLARE_UNRESOLVED_REFERENCE from
the tree - since this is their only use.

----

bootstrapped on i686-linux, i686-darwin9, x86_64-darwin10, (checked to do
the Right Thing on darwin).

Mike has already given this a 'seems reasonable' in the PR thread, however,
I need an approver for the varasm.c and target hook changes.

OK for trunk & 4.6?

Where does the target handle output of the variable? If it is in the hook
then that hook is misnamed - it should then probably be called
assemble_variable.

It is in the hook,
hence named "handled_assemble_variable_p"
.. allowing us to test if the target has completed the work and finish assemble_variable () early.

Does that explain it better? - would an expanded comment help?

(I am happy to use any name generally agreed, but IMO the name should convey the idea that the result should be tested to determine if the target did the work, or it will be confusing to maintain).

Not sure if this is the most reasonable piece of
assemble_variable to split out to a hook though.

It is only used by darwin at present - so nothing is split out for any other target (the default hook is simply 'false').

Initially, I thought about simply moving the ASM_DECLARE_CLASS_REFERENCE and ASM_DECLARE_UNRESOLVED_REFERENCE to varasm.c

However, there seems to be a general move to use hooks instead of macros, and this seemed like a good time to make the change (and pave the way to remove those two macros from the tree).

If the consensus is a preference to retain the macros - I could re- work the patch to do that.

I'm not sure we want to backport this though (there isn't even a bugreport
about it?).

PR48109, reported initially against 4.6.0 (before it forked);
... I guess it's also wrong code under LTO.

thanks
Iain


Thanks,
Richard.

Iain

===

gcc/

       * target.def (handled_assemble_variable_p): New hook.
* varasm.c (assemble_variable): Allow target to handle variable
output
       in some special manner.
       * doc/tm.texi: Regenerate.
* config/darwin.c (darwin_objc1_section): Handle class defs/ refs.
       (darwin_handled_assemble_variable_p): New.
* config/darwin-protos.h (darwin_handled_assemble_variable_p): New. * config/darwin.h (TARGET_ASM_HANDLED_ASSEMBLE_VARIABLE_P): New.

gcc/objc/

*objc-next-runtime-abi-01.c (handle_next_class_ref): Don't emit lazy
refs. for
cases where the class is local. Declare a real, meta-data item
tagged as
       a class reference.
(handle_next_impent): Declare a real, meta-data item tagged as a
class def.

===

Index: gcc/target.def
===================================================================
--- gcc/target.def      (revision 175628)
+++ gcc/target.def      (working copy)
@@ -449,6 +449,13 @@ DEFHOOK
 bool, (FILE *file, rtx x),
 default_asm_output_addr_const_extra)

+DEFHOOK
+(handled_assemble_variable_p,
+ "Returns @code{true} iff the target has handled the assembly of the\
+  variable @var{var_decl}",
+ bool, (tree var_decl),
+ hook_bool_tree_false)
+
/* ??? The TARGET_PRINT_OPERAND* hooks are part of the asm_out struct, even though that is not reflected in the macro name to override their
   initializers.  */
Index: gcc/objc/objc-next-runtime-abi-01.c
===================================================================
--- gcc/objc/objc-next-runtime-abi-01.c (revision 175628)
+++ gcc/objc/objc-next-runtime-abi-01.c (working copy)
@@ -2267,27 +2267,51 @@ generate_objc_symtab_decl (void)
                  init_objc_symtab (TREE_TYPE (UOBJC_SYMBOLS_decl)));
 }

-
 static void
 handle_next_class_ref (tree chain)
 {
+  tree decl, exp;
+  struct imp_entry *impent;
  const char *name = IDENTIFIER_POINTER (TREE_VALUE (chain));
  char *string = (char *) alloca (strlen (name) + 30);

+  for (impent = imp_list; impent; impent = impent->next)
+    if (TREE_CODE (impent->imp_context) == CLASS_IMPLEMENTATION_TYPE
+        && IDENTIFIER_POINTER (CLASS_NAME (impent->imp_context))
+           == IDENTIFIER_POINTER (TREE_VALUE (chain)))
+      return; /* we declare this, no need for a lazy ref.  */
+
  sprintf (string, ".objc_class_name_%s", name);

-#ifdef ASM_DECLARE_UNRESOLVED_REFERENCE
-  ASM_DECLARE_UNRESOLVED_REFERENCE (asm_out_file, string);
-#else
-  return ; /* NULL build for targets other than Darwin.  */
-#endif
+  decl = build_decl (UNKNOWN_LOCATION,
+ VAR_DECL, get_identifier (string), char_type_node);
+  TREE_PUBLIC (decl) = 1;
+  DECL_EXTERNAL (decl) = 1;
+  DECL_CONTEXT (decl) = NULL_TREE;
+  finish_var_decl (decl, NULL);
+
+ /* We build a variable to signal the reference. This will be intercepted
+     and output as a lazy reference.  */
+  sprintf (string, "_OBJC_class_ref_%s", name);
+  exp = build1 (ADDR_EXPR, string_type_node, decl);
+  decl = build_decl (input_location,
+ VAR_DECL, get_identifier (string), string_type_node);
+  TREE_STATIC (decl) = 1;
+  DECL_ARTIFICIAL (decl) = 1;
+  DECL_INITIAL (decl) = error_mark_node;
+
+  /* We must force the reference.  */
+  DECL_PRESERVE_P (decl) = 1;
+  OBJCMETA (decl, objc_meta, get_identifier ("V1_CREF"));
+  DECL_CONTEXT (decl) = NULL_TREE;
+  finish_var_decl (decl, exp);
 }

 static void
 handle_next_impent (struct imp_entry *impent)
 {
  char buf[BUFSIZE];
-
+  tree decl;
  switch (TREE_CODE (impent->imp_context))
    {
    case CLASS_IMPLEMENTATION_TYPE:
@@ -2303,11 +2327,16 @@ handle_next_impent (struct imp_entry *impent)
      return;
    }

-#ifdef ASM_DECLARE_CLASS_REFERENCE
-  ASM_DECLARE_CLASS_REFERENCE (asm_out_file, buf);
-#else
-  return ; /* NULL build for targets other than Darwin.  */
-#endif
+  /* We build a variable to signal that this TU contains this class
metadata.  */
+  decl = build_decl (UNKNOWN_LOCATION,
+                    VAR_DECL, get_identifier (buf), char_type_node);
+  TREE_PUBLIC (decl) = 1;
+  DECL_CONTEXT (decl) = NULL_TREE;
+  OBJCMETA (decl, objc_meta, get_identifier ("V1_CDEF"));
+  TREE_STATIC (decl) = 1;
+  DECL_ARTIFICIAL (decl) = 1;
+  DECL_INITIAL (decl) = error_mark_node;
+  finish_var_decl (decl,  NULL);
 }

 static void
Index: gcc/varasm.c
===================================================================
--- gcc/varasm.c        (revision 175628)
+++ gcc/varasm.c        (working copy)
@@ -2009,6 +2009,10 @@ assemble_variable (tree decl, int top_level ATTRIB
  align_variable (decl, dont_output_data);
  set_mem_align (decl_rtl, DECL_ALIGN (decl));

+ /* Allow the target to handle the variable output in some special manner.
 */
+  if (targetm.asm_out.handled_assemble_variable_p (decl))
+    return;
+
  if (TREE_PUBLIC (decl))
    maybe_assemble_visibility (decl);

Index: gcc/config/darwin-protos.h
===================================================================
--- gcc/config/darwin-protos.h  (revision 175628)
+++ gcc/config/darwin-protos.h  (working copy)
@@ -106,6 +106,7 @@ extern void darwin_asm_output_aligned_decl_local ( extern void darwin_asm_output_aligned_decl_common (FILE *, tree, const char
*,
unsigned HOST_WIDE_INT,
                                                  unsigned int);
+extern bool darwin_handled_assemble_variable_p (tree);

 extern bool darwin_binds_local_p (const_tree);
 extern void darwin_cpp_builtins (struct cpp_reader *);
Index: gcc/config/darwin.c
===================================================================
--- gcc/config/darwin.c (revision 175628)
+++ gcc/config/darwin.c (working copy)
@@ -1439,6 +1439,11 @@ darwin_objc1_section (tree decl ATTRIBUTE_UNUSED,
  else if (!strncmp (p, "V2_CSTR", 7))
    return darwin_sections[objc_constant_string_object_section];

+  else if (!strncmp (p, "V1_CREF", 7))
+    return darwin_sections[objc_cls_refs_section];
+  else if (!strncmp (p, "V1_CDEF", 7))
+    return data_section;
+
  return base;
 }

@@ -2594,6 +2599,53 @@ darwin_assemble_visibility (tree decl, int vis)
            "not supported in this configuration; ignored");
 }

+/* For certain Objective-C metadata we handle the assembly of the variables + here (it must be here, rather than in darwin-c.c, to cater for LTO).
 When
+ we reference a class we emit a lazy ref, when we have class metadata + (local to a specific object), we emit a tag so that linkage will be
+   satisfied for the class.  */
+
+bool
+darwin_handled_assemble_variable_p (tree decl)
+{
+  tree meta;
+  if (DECL_ATTRIBUTES (decl)
+ && (meta = lookup_attribute ("OBJC1META", DECL_ATTRIBUTES (decl))))
+    {
+      const char *name;
+      rtx decl_rtl, symbol;
+
+      if (TREE_VALUE (meta) == get_identifier ("V1_CREF"))
+       {
+         tree exp = DECL_INITIAL (decl);
+         gcc_assert (exp
+                     && exp != error_mark_node
+                     && TREE_CODE (exp) == ADDR_EXPR);
+         exp = TREE_OPERAND (exp, 0);
+         decl_rtl = DECL_RTL (exp);
+         symbol = XEXP (decl_rtl, 0);
+         name = XSTR (symbol, 0);
+         targetm.asm_out.globalize_decl_name (asm_out_file, exp);
+         fputs ("\t.lazy_reference\t",asm_out_file);
+         assemble_name (asm_out_file, name);
+         fputs ("\n",asm_out_file);
+         return true;
+       }
+      else if (TREE_VALUE (meta) == get_identifier ("V1_CDEF"))
+       {
+         decl_rtl = DECL_RTL (decl);
+         symbol = XEXP (decl_rtl, 0);
+         name = XSTR (symbol, 0);
+         targetm.asm_out.globalize_decl_name (asm_out_file, decl);
+         fputs ("\t",asm_out_file);
+         assemble_name (asm_out_file, name);
+         fputs (" = 0\n",asm_out_file);
+         return true;
+       }
+    }
+  return false;
+}
+
 /* VEC Used by darwin_asm_dwarf_section.
Maybe a hash tab would be better here - but the intention is that this is a very short list (fewer than 16 items) and each entry should (ideally,
Index: gcc/config/darwin.h
===================================================================
--- gcc/config/darwin.h (revision 175628)
+++ gcc/config/darwin.h (working copy)
@@ -678,6 +678,9 @@ extern GTY(()) section * darwin_sections[NUM_DARWI
 #undef TARGET_ASM_SELECT_SECTION
 #define TARGET_ASM_SELECT_SECTION machopic_select_section

+#undef TARGET_ASM_HANDLED_ASSEMBLE_VARIABLE_P
+#define TARGET_ASM_HANDLED_ASSEMBLE_VARIABLE_P
darwin_handled_assemble_variable_p
+
 #undef TARGET_ASM_FUNCTION_SECTION
 #define TARGET_ASM_FUNCTION_SECTION darwin_function_section





Reply via email to