On Thu, Jun 30, 2011 at 12:50 PM, Iain Sandoe
<develo...@sandoe-acoustics.co.uk> wrote:
>
> 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).
Well, as it certainly isn't a predicate but is supposed to do the actual
work simply calling it assemble_variable is better.
>> 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').
Yes, I've seen that. Still a hook like this should be generally useful,
and you still process through some pieces of assemble_variable
while you choose to skip some other piece - it should probably be
documented which part of assemble_variable is supposed to be handled
by the hook.
I'll defer to Richard for this (and the approval).
> 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.
No, a hook is definitely better.
>> 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.
So the ChangeLog should mention this PR.
Richard.
> 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
>>>
>>>
>>>
>>>
>
>
