This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [patch lto/c++/middle-end/windows]: Fix handling of dllexport for LTO as externally_visible
On Tue, Nov 16, 2010 at 5:08 PM, Kai Tietz <ktietz70@googlemail.com> wrote:
> 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.
The patch will be smaller w/o the flag, especially you don't need to
worry about copying it everywhere.
Richard.
> Kai
> --
> |? (\_/) This is Bunny. Copy and paste
> | (='.'=) Bunny into your signature to help
> | (")_(") him gain world domination
>