[patch lto/c++/middle-end/windows]: Fix handling of dllexport for LTO as externally_visible

[Kai Tietz]

2010/11/16 Richard Guenther <richard.guenther@gmail.com>:
> On Tue, Nov 16, 2010 at 4:11 PM, Kai Tietz <ktietz70@googlemail.com> wrote:
>> 2010/11/16 Jan Hubicka <hubicka@ucw.cz>:
>>>> On Tue, Nov 16, 2010 at 11:46 AM, Kai Tietz <Kai.Tietz@onevision.com> wrote:
>>>> > Hello,
>>>> >
>>>> > this patch introduces an new flag for dllexport (likewise to the already
>>>> > existing DLLIMPORT flag) to structure tree_decl_with_vis in tree.h header..
>>>> > As some places still dependent on checking explicit for
>>>> > dllimport/dllexport attribute, I didn't omitted here the the attribute
>>>> > node copy. This is maybe something for 4.7 gcc.
>>>> >
>>>> > ChangeLog
>>>> >
>>>> >        * config/i386/winnt-cxx.c (maybe_add_dllexport): Set
>>>> > DECL_DLLEXPORT_P.
>>>> >        * config/i386/winnt.c (i386_pe_determine_dllexport_p): Use
>>>> > DECL_DLLEXPORT_P
>>>> >        instead of doing attribute lookup.
>>>> >        * tree.c (merge_dllimport_decl_attributes): Likewise.
>>>> >        (handle_dll_attribute): Set DECL_DLLEXPORT_P flag.
>>>> >        * lto-streamer-in.c (unpack_ts_decl_with_vis_value_fields):
>>>> > Restore dllexport flag.
>>>> >        * lto-streamer-out.c (pack_ts_decl_with_vis_value_fields): Save
>>>> > dllexport flag.
>>>> >        * tree-emutls.c (new_emutls_decl): Copy DECL_DLLEXPORT_P flag.
>>>> >        * tree.h (tree_decl_with_vis):Add new member dllexport_flag.
>>>> >        (DECL_DLLEXPORT_P): New macro.
>>>> >        * ipa.c (cgraph_externally_visible_p): Check for DECL_DLLEXPORT_P.
>>>> >        (varpool_externally_visible_p): Likewise.
>>>> >        *cp/optimize.c (maybe_clone_body): Copy DECL_DLLEXPORT_P, too.
>>>> >
>>>> > Tested for x86_64-w64-mingw32, i686-w64-mingw32, and i686-pc-cygwin. Ok
>>>> > for apply?
>>>>
>>>> What's the difference between externally_visible and dllexport?
>>>>
>>>> @@ -3121,6 +3125,7 @@ struct GTY(()) tree_decl_with_vis {
>>>>   unsigned in_text_section : 1;
>>>>   unsigned in_constant_pool : 1;
>>>>   unsigned dllimport_flag : 1;
>>>> + unsigned dllexport_flag : 1;
>>>>   /* Don't belong to VAR_DECL exclusively.  */
>>>>   unsigned weak_flag : 1;
>>>>
>>>> please keep 1) flags in 8 bit groups, 2) add it below the right comment
>>>> (it doesn't belong to VAR_DECLs exclusively, right?)
>>>>
>>>> That said, why not simply add externally_visible when seeing dllexport?
>>>
>>> I would not be too offended by doing that.
>>>
>>>> Why not make externally_visible use a flag (or intergrate that with
>>>> visibility support)?
>>>
>>> We already have flag in cgraph node, so we search for it in declaration
>>> constant number of times (I think twice, once for visibility pass and once for
>>> whole_program_visibility), so adding flag seems bit redundant.
>>>
>>> We can also just search for dllexport attribute at the same place, it would
>>> not be terribly expensive either.
>>>
>>> Honza
>>>>
>>>> Richard.
>>>>
>>>> > Regards,
>>>> >  i.A. Kai Tietz
>>>> >
>>>> > |  (\_/)  This is Bunny. Copy and paste Bunny
>>>> > | (='.'=) into your signature to help him gain
>>>> > | (")_(") world domination.
>>>> >
>>>> >
>>>
>>
>> So, I moved the new flag at the end of the structure to keep alignment
>> of 8 bit blocks. Additionally I add a testcase for dllexport and
>> whole-program.
>>
>> Ok for apply?
>
> Can you instead of adding a flag use lookup_attribute like honza
> suggested?
>
> Thanks,
> Richard.
>
>> Regards,
>> Kai
>>
>> --
>> |  (\_/) This is Bunny. Copy and paste
>> | (='.'=) Bunny into your signature to help
>> | (")_(") him gain world domination
>>
>

Well, of course. But is it really something good to add everywhere
checks for those attributes again and again? A check of a flag looks
to me more sane then checking at all places declarations attribute
again and again. Especially as there is already the dllimport flag
case present.

Kai
-- 
|  (\_/) This is Bunny. Copy and paste
| (='.'=) Bunny into your signature to help
| (")_(") him gain world domination