On Fri, Jul 25, 2014 at 12:24 PM, Richard Biener
<richard.guent...@gmail.com> wrote:
> On Thu, Jul 24, 2014 at 7:21 PM, Martin Liška <mli...@suse.cz> wrote:
>>
>> On 07/24/2014 05:53 PM, David Malcolm wrote:
>>>
>>> On Thu, 2014-07-24 at 16:23 +0100, Martin Liška wrote:
>>>>
>>>> Hello,
>>>> I would like to encapsulate all symtab_nodes to a class called
>>>> 'symtab'.
>>>
>>> Thanks!
>>>
>>>> To respect gcc:context, I would like to add the class to global
>>>> context (gcc::context) and I have troubles with GTY and GGC.
>>>>
>>>> design:
>>>>
>>>> class GTY(()) symtab
>>>> {
>>>> public:
>>>> symtab_node* GTY(()) nodes;
>>>> };
>>>>
>>>> class GTY(()) context
>>>> {
>>>> public:
>>>> context () {}
>>>> void init ();
>>>>
>>>> /* Pass-management. */
>>>> pass_manager *get_passes () { gcc_assert (m_passes); return m_passes;
>>>> }
>>>>
>>>> /* Handling dump files. */
>>>> dump_manager *get_dumps () {gcc_assert (m_dumps); return m_dumps; }
>>>>
>>>> /* Symbol table. */
>>>> symtab *get_symtab () { gcc_assert (symbol_table); return
>>>> symbol_table; }
>>>>
>>>> /* Symbol table. */
>>>> symtab * GTY(()) symbol_table;
>>>>
>>>> private:
>>>> /* Pass-management. */
>>>> pass_manager * GTY((skip)) m_passes;
>>>>
>>>> /* Dump files. */
>>>> dump_manager * GTY((skip)) m_dumps;
>>>> }; // class context
>>>>
>>>> } // namespace gcc
>>>>
>>>> /* The global singleton context aka "g".
>>>> (the name is chosen to be easy to type in a debugger). */
>>>> extern GTY(()) gcc::context *g;
>>>>
>>>>
>>>>
>>>> gcc:context in created by g = ggc_cleared_alloc<gcc::context> (); and
>>>> g->symbol_table too.
>>>> My problem is that during pg_pch_restore:
>>>> #3 0x0000000000850043 in gt_pch_restore (f=0x1ac9100) at
>>>> ../../gcc/ggc-common.c:691
>>>>
>>>> When I add watch to *(&g), gcc::context *g is restored ^ and
>>>> gcc:context::m_passes and gcc:context::m_dumps are set to an inaccessible
>>>> address.
>>>>
>>>> Is my GGC construct correct?
>>>
>>> Back when we introduced the context and the pass_manager (called the
>>> "pipeline" in the initial patch submissions), those two classes were
>>> GTY-marked, but I reverted the change in r201887 (aka
>>> 5d068519b66a37ab391c427d0aac13f66a9b5c4e):
>>>
>>> Revert my last two changes, r201865 and r201864
>>> 2013-08-20 David Malcolm <dmalc...@redhat.com>
>>> Revert my last two changes, r201865 and r201864:
>>> Revert r201865:
>>> 2013-08-20 David Malcolm <dmalc...@redhat.com>
>>> Make opt_pass and gcc::pass_manager be GC-managed, so that
>>> pass
>>> instances can own GC refs.
>>> * Makefile.in (GTFILES): Add pass_manager.h and tree-pass.h.
>>> * context.c (gcc::context::gt_ggc_mx): Traverse passes_.
>>> (gcc::context::gt_pch_nx): Likewise.
>>> (gcc::context::gt_pch_nx): Likewise.
>>> * ggc.h (gt_ggc_mx <T>): New.
>>> (gt_pch_nx_with_op <T>): New.
>>> (gt_pch_nx <T>): New.
>>> * passes.c (opt_pass::gt_ggc_mx): New.
>>> (opt_pass::gt_pch_nx): New.
>>> (opt_pass::gt_pch_nx_with_op): New.
>>> (pass_manager::gt_ggc_mx): New.
>>> (pass_manager::gt_pch_nx): New.
>>> (pass_manager::gt_pch_nx_with_op): New.
>>> (pass_manager::operator new): Use
>>> ggc_internal_cleared_alloc_stat rather than xcalloc.
>>> * pass_manager.h (class pass_manager): Add GTY((user)) marking.
>>> (pass_manager::gt_ggc_mx): New.
>>> (pass_manager::gt_pch_nx): New.
>>> (pass_manager::gt_pch_nx_with_op): New.
>>> * tree-pass.h (class opt_pass): Add GTY((user)) marking.
>>> (opt_pass::operator new): New.
>>> (opt_pass::gt_ggc_mx): New.
>>> (opt_pass::gt_pch_nx): New.
>>> (opt_pass::gt_pch_nx_with_op): New.
>>> Revert r201864:
>>> 2013-08-20 David Malcolm <dmalc...@redhat.com>
>>> * Makefile.in (GTFILES): Add context.h.
>>> * context.c (gcc::context::operator new): New.
>>> (gcc::context::gt_ggc_mx): New.
>>> (gcc::context::gt_pch_nx): New.
>>> (gcc::context::gt_pch_nx): New.
>>> * context.h (gcc::context): Add GTY((user)) marking.
>>> (gcc::context::operator new): New.
>>> (gcc::context::gt_ggc_mx): New.
>>> (gcc::context::gt_pch_nx): New.
>>> (gcc::context::gt_pch_nx): New.
>>> (g): Add GTY marking.
>>> (gt_ggc_mx (gcc::context *)): New.
>>> (gt_pch_nx (gcc::context *)): New.
>>> (gt_pch_nx (gcc::context *ctxt, gt_pointer_operator op,
>>> void *cookie)): New.
>>> * gengtype.c (open_base_files) <ifiles>: Add context.h.
>>>
>>> Looking over the list archives, I found this reason for the revert:
>>> https://gcc.gnu.org/ml/gcc/2013-08/msg00214.html
>>> the integration of GTY into context and pass_manager wasn't fully-baked,
>>> and was causing PCH crashes. Looks like I didn't go back and fully
>>> debug the issue; sorry.
>>>
>>> As for the issue you're seeing... I'm guessing you've put the "skip"
>>> clauses on m_passes and m_dumps as a workaround for the issue from last
>>> year, but they don't seem in themselves to be valid: the context is
>>> GTY-marked, and hence saved to/restored from PCH (albeit buggily,
>>> presumably), but the skip clauses mean that those fields aren't
>>> interacting with PCH, merely copied as raw bytes to/from disk, and so
>>> those pointers are going to be pointing to the address they pointed to
>>> in the process that created the PCH, which will likely be nonsensical in
>>> the process reading the PCH file, which would explain the symptoms
>>> you're seeing.
>>>
>>> For now, maybe keep the symtab ptr as a GTY-marked global, outside of
>>> the context? We can at least get a big reduction of the global state
>>> that way, and we can revisit GTY-enabling the context as a separate
>>> issue, moving the symtab ptr into the context at that time?
>>
>> Hello,
>> thank you for you advice. It really looks that I face the same issue as
>> you seen. As you suggested, I will start with a global pointer to symtab and
>> we'll see further integration into context class (where it should reside
>> according to me).
>
> Note that I would suggest an explicit marking function for this singleton
> (thus GTY((user))). Most pointers of the symtab stuff doesn't need following
> but everything is held together by the single array/map?
Which also would make it possible to move the symtab objects out-of-GC
space (but still follow the ->decl edges).
Richard.
> Richard.
>
>> Thanks,
>> Martin
>>
>>
>>>
>>> Hope this is helpful
>>> Dave
>>>
>>
