This new patch is much better. Thank you very much.
2011/9/21 Stephen Clarke <stephen.cla...@st.com>
> Yes, you are correct.
> Sorry for the mistake,
> Steve.
>
> Wu Yongchong wrote:
> > in the file wgen-diffs-2.txt line 62 and line 71
> > - Maybe_Emit_Cleanup (j, TRUE);
> > + Maybe_Handler_Emit_Cleanup (j);
> >
> > you replay Maybe_Emit_Cleanup with Maybe_Handler_Emit_Cleanup. Should
> > it be Maybe_Emit_Handler_cleanup ?
> >
> >
> > On Fri, Sep 9, 2011 at 6:08 PM, Stephen Clarke <stephen.cla...@st.com>
> wrote:
> >> We have also made a fix for this problem in ST. The idea of the fix is
> >> exactly the same, but some implementation details are different. I
> >> reported it to Wu Yongchong, who suggested I submit it for general
> >> review and approval.
> >>
> >> Apologies: I am reporting a patch which I have only validated in the
> context
> >> of ST's sources, which have quite some modifications compared to the
> >> open64.net sources.
> >>
> >> Basic fix (wgen-diffs-1.txt): We added a new function
> >> Maybe_Emit_Cleanups(), which does the work of maintaining the
> >> scope_cleanup_stack and emitting the cleanup. This function avoids
> >> duplicate code in WGEN_Expand_Goto and WGEN_Expand_Return. Also we used
> an
> >> STL stack to save the scope info information, rather than defining a new
> >> type.
> >>
> >> We made another change at the same time (wgen-diffs-2.txt): since the
> only
> >> remaining calls to Maybe_Emit_Cleanup() are made when generating
> handlers,
> >> the from_handler parameter is no longer required, so we
> >> removed it and renamed the function.
> >>
> >> Regards,
> >> Steve.
> >>
> >> Wu Yongchong wrote:
> >>> Hi, all
> >>> Can a gatekeeper help review this patch
> >>>
> >>> It is fix of open64.net bug 828.
> >>>
> >>> The change of version 3360 is introduced to resolve the issue about
> >>> cleanup code of GOTO to outer block and RETURN statements. It
> >>> manipulate the scope-cleanup-stack, so that cleanup code of each
> >>> cleanup would only contain cleanups not finished yet.
> >>>
> >>> The issue of this patch is that the cleanup code generation process
> >>> may push and pop some items on the scope-clean-up stack, so that the
> >>> elements beyond the top of stack would be destroyed. To avoid this
> >>> issue, the new change save and restore these elements on the stack.
> >>>
> >>>
> >>>
> >>
> >> --- wgen_stmt.cxx.orig 2011-09-07 16:36:30.378726000 +0100
> >> +++ wgen_stmt.cxx.1 2011-09-07 16:53:56.868216000 +0100
> >> @@ -354,6 +354,43 @@
> >> }
> >> }
> >>
> >> +static INT32
> >> +Maybe_Emit_Cleanups (gs_t sentinel)
> >> +{
> >> + // Emit all the cleanups required at a transfer of control out of
> >> + // the current scope, e.g. a goto or return. Note, after this
> function,
> >> + // we continue in the same scope, so we should not actually pop the
> >> + // cleanups, but for correct processing we must temporarily pop them
> as
> >> we
> >> + // emit them.
> >> + // Therefore, pop them to a temporary stack, then restore them again
> >> + // afterwards.
> >> + INT32 result;
> >> + std::stack<SCOPE_CLEANUP_INFO> saved_cleanups;
> >> +
> >> + while (scope_cleanup_i >= 0 &&
> >> + scope_cleanup_stack[scope_cleanup_i].stmt != sentinel) {
> >> + SCOPE_CLEANUP_INFO scope_cleanup = scope_cleanup_stack
> >> [scope_cleanup_i];
> >> + --scope_cleanup_i;
> >> + saved_cleanups.push(scope_cleanup);
> >> + gs_t stmt = scope_cleanup.stmt;
> >> + if (gs_tree_code(stmt) == GS_CLEANUP_STMT &&
> >> + !scope_cleanup.cleanup_eh_only)
> >> + WGEN_One_Stmt_Cleanup (gs_cleanup_expr(stmt));
> >> + else if (gs_tree_code(stmt) == GS_TRY_FINALLY_EXPR)
> >> + WGEN_One_Stmt_Cleanup (gs_tree_operand(stmt, 1));
> >> + }
> >> +
> >> + result = scope_cleanup_i;
> >> +
> >> + while (! saved_cleanups.empty ()) {
> >> + scope_cleanup_stack[++scope_cleanup_i] = saved_cleanups.top ();
> >> + saved_cleanups.pop ();
> >> + }
> >> +
> >> + return result;
> >> +}
> >> +
> >> +
> >> static void
> >> Emit_Cleanup(gs_t cleanup)
> >> {
> >> @@ -2133,16 +2170,7 @@
> >> if (*li != *ci) break;
> >> if (ci!=Current_scope_nest.rend())
> >> {
> >> - int scope_cleanup_i_save = scope_cleanup_i;
> >> - i = scope_cleanup_i;
> >> - Is_True(i != -1, ("WGEN_Expand_Goto: scope_cleanup_stack
> empty"));
> >> - while ((i >= 0) && (scope_cleanup_stack [i].stmt != *ci))
> >> - {
> >> - scope_cleanup_i --;
> >> - Maybe_Emit_Cleanup (i, FALSE);
> >> - --i;
> >> - }
> >> - scope_cleanup_i = scope_cleanup_i_save;
> >> + i = Maybe_Emit_Cleanups (*ci);
> >> if (i == -1)
> >> {
> >> #ifdef FE_GNU_4_2_0
> >> @@ -2293,20 +2321,8 @@
> >> }
> >> #endif
> >>
> >> - int i = scope_cleanup_i;
> >> - int scope_cleanup_i_save = scope_cleanup_i;
> >> - while (i != -1) {
> >> -#ifdef KEY
> >> - scope_cleanup_i--;
> >> - Maybe_Emit_Cleanup (i, FALSE);
> >> -#else
> >> - if (gs_tree_code(scope_cleanup_stack [i].stmt) ==
> GS_CLEANUP_STMT)
> >> - WGEN_One_Stmt_Cleanup (gs_cleanup_expr(scope_cleanup_stack
> >> [i].stmt));
> >> -#endif
> >> - --i;
> >> - }
> >> + Maybe_Emit_Cleanups (NULL);
> >> #ifdef KEY
> >> - scope_cleanup_i = scope_cleanup_i_save;
> >> if (emit_exceptions && processing_handler) {
> >> HANDLER_INFO hi = handler_stack.top();
> >> FmtAssert (hi.scope, ("NULL scope"));
> >> @@ -2402,20 +2418,8 @@
> >> }
> >> #endif
> >>
> >> - int i = scope_cleanup_i;
> >> - int scope_cleanup_i_save = scope_cleanup_i;
> >> - while (i != -1) {
> >> -#ifdef KEY
> >> - scope_cleanup_i--;
> >> - Maybe_Emit_Cleanup (i, FALSE);
> >> -#else
> >> - if (gs_tree_code(scope_cleanup_stack [i].stmt) ==
> GS_CLEANUP_STMT)
> >> - WGEN_One_Stmt_Cleanup (gs_cleanup_expr(scope_cleanup_stack
> >> [i].stmt));
> >> -#endif
> >> - --i;
> >> - }
> >> + Maybe_Emit_Cleanups (NULL);
> >> #ifdef KEY
> >> - scope_cleanup_i = scope_cleanup_i_save;
> >> if (emit_exceptions && processing_handler) {
> >> HANDLER_INFO hi = handler_stack.top();
> >> FmtAssert (hi.scope, ("NULL scope"));
> >>
> >> --- wgen_stmt.cxx.1 2011-09-07 16:53:56.868216000 +0100
> >> +++ wgen_stmt.cxx.2 2011-09-07 17:04:36.141813000 +0100
> >> @@ -324,34 +324,22 @@
> >> }
> >>
> >> // May be emit cleanup in I-th entry of stack depending on the
> statement
> >> -// in the entry. FROM_HANDLER determines which stack to use. This
> function
> >> +// in the entry. Called when we are emitting a handler. This function
> >> // is called when the decision whether to emit a cleanup and how to
> emit it
> >> // depends on the type of statement.
> >> // Wgen TODO: This function probably needs to be called from a few
> other
> >> // places also, they will be added as the need arises.
> >> static inline void
> >> -Maybe_Emit_Cleanup(INT i, BOOL from_handler)
> >> +Maybe_Emit_Handler_Cleanup(INT i)
> >> {
> >> - if (!from_handler)
> >> - {
> >> - gs_t stmt = scope_cleanup_stack [i].stmt;
> >> - if (gs_tree_code(stmt) == GS_CLEANUP_STMT &&
> >> - !scope_cleanup_stack[i].cleanup_eh_only)
> >> - WGEN_One_Stmt_Cleanup (gs_cleanup_expr(stmt));
> >> - else if (gs_tree_code(stmt) == GS_TRY_FINALLY_EXPR)
> >> - WGEN_One_Stmt_Cleanup (gs_tree_operand(stmt, 1));
> >> - }
> >> - else
> >> - {
> >> - HANDLER_INFO hi = handler_stack.top();
> >> - gs_t stmt = (*hi.scope) [i].stmt;
> >> -
> >> - if (gs_tree_code(stmt) == GS_CLEANUP_STMT &&
> >> - !(*hi.scope) [i].cleanup_eh_only)
> >> - WGEN_One_Stmt_Cleanup (gs_cleanup_expr(stmt));
> >> - else if (gs_tree_code(stmt) == GS_TRY_FINALLY_EXPR)
> >> - WGEN_One_Stmt_Cleanup (gs_tree_operand(stmt, 1));
> >> - }
> >> + HANDLER_INFO hi = handler_stack.top();
> >> + gs_t stmt = (*hi.scope) [i].stmt;
> >> +
> >> + if (gs_tree_code(stmt) == GS_CLEANUP_STMT &&
> >> + !(*hi.scope) [i].cleanup_eh_only)
> >> + WGEN_One_Stmt_Cleanup (gs_cleanup_expr(stmt));
> >> + else if (gs_tree_code(stmt) == GS_TRY_FINALLY_EXPR)
> >> + WGEN_One_Stmt_Cleanup (gs_tree_operand(stmt, 1));
> >> }
> >>
> >> static INT32
> >> @@ -2195,7 +2183,7 @@
> >> Is_True(i != -1, ("WGEN_Expand_Goto: scope_cleanup_stack empty
> inside
> >> handler"));
> >> while ((i >= 0) && ((*hi.scope) [i].stmt != *ci))
> >> {
> >> - Maybe_Emit_Cleanup (i, TRUE);
> >> + Maybe_Emit_Handler_Cleanup (i);
> >> --i;
> >> }
> >> }
> >> @@ -2328,7 +2316,7 @@
> >> FmtAssert (hi.scope, ("NULL scope"));
> >> int j = hi.scope->size()-1;
> >> while (j != -1) {
> >> - Maybe_Emit_Cleanup (j, TRUE);
> >> + Maybe_Handler_Emit_Cleanup (j);
> >> --j;
> >> }
> >> }
> >> @@ -2425,7 +2413,7 @@
> >> FmtAssert (hi.scope, ("NULL scope"));
> >> int j = hi.scope->size()-1;
> >> while (j != -1) {
> >> - Maybe_Emit_Cleanup (j, TRUE);
> >> + Maybe_Handler_Emit_Cleanup (j);
> >> --j;
> >> }
> >> }
> >>
> >>
> >
> >
> >
>
>
>
> ------------------------------------------------------------------------------
> All the data continuously generated in your IT infrastructure contains a
> definitive record of customers, application performance, security
> threats, fraudulent activity and more. Splunk takes this data and makes
> sense of it. Business sense. IT sense. Common sense.
> http://p.sf.net/sfu/splunk-d2dcopy1
> _______________________________________________
> Open64-devel mailing list
> Open64-devel@lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/open64-devel
>
--
Regards,
Lai Jian-Xin
------------------------------------------------------------------------------
All the data continuously generated in your IT infrastructure contains a
definitive record of customers, application performance, security
threats, fraudulent activity and more. Splunk takes this data and makes
sense of it. Business sense. IT sense. Common sense.
http://p.sf.net/sfu/splunk-d2dcopy1
_______________________________________________
Open64-devel mailing list
Open64-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/open64-devel
